Abort previous pending XHR when navigating #114

Merged
robinnorth merged 4 commits from fix/abort-pending-xhr into master 2018-01-24 18:54:34 -05:00
8 changed files with 95 additions and 8 deletions

View File

@@ -10,7 +10,7 @@
<div class='body'>
<h1>Index</h1>
Hello.
Go to <a href='page2.html' class="js-Pjax">Page 2</a> and view your console to see Pjax events.
Go to <a href='page2.html' class="js-Pjax">Page 2</a> or <a href='page3.html' class="js-Pjax">Page 3</a> and view your console to see Pjax events.
Clicking on <a href='index.html'>this page</a> will just reload the page entirely.
</div>
</body>

15
example/page3.html Normal file
View File

@@ -0,0 +1,15 @@
<!doctype html>
<html>
<head>
<meta charset='utf-8'>
<title>Hello</title>
<script src='../pjax.js'></script>
<script src='example.js'></script>
</head>
<body>
<div class='body'>
<h1>Page 3</h1>
Hello. Go to <a href='index.html' class="js-Pjax">Index</a>.
</div>
</body>
</html>

View File

@@ -5,6 +5,8 @@ var forEachEls = require("./lib/foreach-els.js")
var newUid = require("./lib/uniqueid.js")
var noop = require("./lib/util/noop")
BehindTheMath commented 2018-01-24 18:25:25 -05:00 (Migrated from github.com)
Review

Can you change this as well to noop.js?

Can you change this as well to `noop.js`?
var on = require("./lib/events/on.js")
// var off = require("./lib/events/on.js")
var trigger = require("./lib/events/trigger.js")
@@ -145,16 +147,21 @@ Pjax.prototype = {
// }
},
doRequest: require("./lib/request.js"),
abortRequest: require("./lib/abort-request.js"),
doRequest: require("./lib/send-request.js"),
loadUrl: function(href, options) {
this.log("load href", href, options)
// Abort any previous request
this.abortRequest(this.request)
trigger(document, "pjax:send", options);
// Do the request
options.requestOptions.timeout = this.options.timeout
this.doRequest(href, options.requestOptions, function(html, request) {
this.request = this.doRequest(href, options.requestOptions, function(html, request) {
// Fail if unable to load HTML via AJAX
if (html === false) {
trigger(document,"pjax:complete pjax:error", options)
@@ -326,10 +333,10 @@ if (Pjax.isSupported()) {
}
// if there isnt required browser functions, returning stupid api
else {
var stupidPjax = function() {}
var stupidPjax = noop
for (var key in Pjax.prototype) {
if (Pjax.prototype.hasOwnProperty(key) && typeof Pjax.prototype[key] === "function") {
stupidPjax[key] = stupidPjax
stupidPjax[key] = noop
}
}

8
lib/abort-request.js Normal file
View File

@@ -0,0 +1,8 @@
var noop = require("./util/noop")
BehindTheMath commented 2018-01-24 18:25:38 -05:00 (Migrated from github.com)
Review

Can you change this as well to noop.js?

Can you change this as well to `noop.js`?
robinnorth commented 2018-01-24 18:31:24 -05:00 (Migrated from github.com)
Review

I can, although half the library uses this Node convention of not appending the module file extension to the require call -- we should probably standardise this one way or the other -- something for #97?

I can, although half the library uses this Node convention of not appending the module file extension to the require call -- we should probably standardise this one way or the other -- something for #97?
BehindTheMath commented 2018-01-24 18:53:58 -05:00 (Migrated from github.com)
Review

You're right; I didn't notice that.

I guess leave it for now, and make a comment in #97.

You're right; I didn't notice that. I guess leave it for now, and make a comment in #97.
module.exports = function(request) {
if (request && request.readyState < 4) {
request.onreadystatechange = noop
request.abort()
}
}

1
lib/util/noop.js Normal file
View File

@@ -0,0 +1 @@
module.exports = function() {}

View File

@@ -0,0 +1,56 @@
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var tape = require("tape")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var abortRequest = require("../../lib/abort-request.js")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var sendRequest = require("../../lib/send-request.js")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
// Polyfill responseURL property into XMLHttpRequest if it doesn't exist,
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
// just for the purposes of this test
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
// This polyfill is not complete; it won't show the updated location if a
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
// redirection occurred, but it's fine for our purposes.
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
if (!("responseURL" in XMLHttpRequest.prototype)) {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var nativeOpen = XMLHttpRequest.prototype.open
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
XMLHttpRequest.prototype.open = function(method, url) {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
this.responseURL = url
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
return nativeOpen.apply(this, arguments)
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
}
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
}
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
tape("test aborting xhr request", function(t) {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var requestCacheBust = sendRequest.bind({
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
options: {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
cacheBust: true,
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
},
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
})
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.test("- pending request is aborted", function(t) {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var r = requestCacheBust("https://httpbin.org/delay/10", {}, function() {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.fail("xhr was not aborted")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
})
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.equal(r.readyState, 1, "xhr readyState is '1' (SENT)")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
abortRequest(r)
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.equal(r.status, 0, "xhr HTTP status is '0' (ABORTED)")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.equal(r.responseText, "", "xhr response is empty")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.end()
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
})
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.test("- request is not aborted if it has already completed", function(t) {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var r = requestCacheBust("https://httpbin.org/get", {}, function() {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
abortRequest(r)
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.equal(r.readyState, 4, "xhr readyState is '4' (DONE)")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.equal(r.status, 200, "xhr HTTP status is '200' (OK)")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.end()
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
})
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
})
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.test("- request is not aborted if it is undefined", function(t) {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
var r
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
try {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
abortRequest(r)
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
}
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
catch (e) {
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.fail("aborting an undefined request threw an error")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
}
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.equal(typeof r, "undefined", "undefined xhr was ignored")
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.end()
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
})
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
t.end()
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.
})
BehindTheMath commented 2018-01-24 09:30:37 -05:00 (Migrated from github.com)
Review

Is this callback ever called? Do you want to add a test that fails if it is?

Is this callback ever called? Do you want to add a test that fails if it is?
robinnorth commented 2018-01-24 09:44:01 -05:00 (Migrated from github.com)
Review

Good idea, will do that.

Good idea, will do that.
robinnorth commented 2018-01-24 09:50:26 -05:00 (Migrated from github.com)
Review

Actually, that callback is never called, because if abortRequest(r) doesn't abort the request, the next assert (t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)")) immediately fails, so I don't think we do need to add anything else in.

Actually, that callback is never called, because if `abortRequest(r)` doesn't abort the request, the next assert (`t.equal(r.readyState, 0, "xhr readyState is '0' (ABORTED)"))` immediately fails, so I don't think we do need to add anything else in.
BehindTheMath commented 2018-01-24 17:40:31 -05:00 (Migrated from github.com)
Review

Tape has a t.fail() function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?

Tape has a [`t.fail()`](https://github.com/substack/tape#tfailmsg) function. I think adding that will make it more apparent that the callback is not supposed to be called. What do you think?
robinnorth commented 2018-01-24 18:18:10 -05:00 (Migrated from github.com)
Review

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback shouldn't fire, will add that back in.

I did add that and then remove it again because the callback would never fire, but I take the point about making it explicit that the callback *shouldn't* fire, will add that back in.

View File

@@ -1,6 +1,6 @@
var tape = require("tape")
var request = require("../../lib/request.js")
var sendRequest = require("../../lib/send-request.js")
// Polyfill responseURL property into XMLHttpRequest if it doesn't exist,
// just for the purposes of this test
@@ -18,7 +18,7 @@ tape("test xhr request", function(t) {
var url = "https://httpbin.org/get"
t.test("- request is made, gets a result, and is cache-busted", function(t) {
var requestCacheBust = request.bind({
var requestCacheBust = sendRequest.bind({
options: {
cacheBust: true,
},
@@ -36,7 +36,7 @@ tape("test xhr request", function(t) {
})
})
t.test("- request is not cache-busted when configured not to be", function(t) {
var requestNoCacheBust = request.bind({
var requestNoCacheBust = sendRequest.bind({
options: {
cacheBust: false,
},