Your ajax()
function gives me a good first impression. The structure seems sound and the event handling well done - the done()
event handler is called exactly once (someone else might want to confirm, though).
A few suggestions:
var opts = Object.assign({...}, options);
opts
vs.options
is inconsistent and carries no semantics. Reassigning tooptions
would mutate the argument which could cause issues for the user. jQuery names the merged options and defaultssettings
, which might be a little bit more clear.if (this.status >= 200 && this.status < 300) {
This line seems good for now. You might want to check 304 in a future version when allowing users to supply caching headers. However, such requests don't respond with data and are probably not within the intended use of your library right now.
xhr.send(null);
The
send
method accepts an optional parameter which is ignored and set tonull
for GET requests. Therefore, I would recommend the simplerxhr.send()
.if (this.readyState === 4) {
Add more semantic by replacing
4
withXMLHttpRequest.DONE
- not available in old browsers such as IE 8not available in old browsers such as IE 8.cb('error');
This seems like a design flaw to me. A user cannot distinguish between an
'error'
string received from the server vs. supplied as a result of an invalid HTTP status code. You already supplyfalse
on multiple occasions - when a network level error or timeout occurs - which is easily distinguished from a valid response by type checking. Why not supplyfalse
in the above case, too? If that's not an option, supply an error status flag or object or introduceerror
callback(s).