Handle XHR response error #137

Merged
BehindTheMath merged 6 commits from feature/handle-response-error into master 2018-03-15 15:12:32 -05:00
5 changed files with 310 additions and 65 deletions

View File

@@ -174,6 +174,34 @@ pjax.loadUrl("/your-url")
pjax.loadUrl("/your-other-url", {timeout: 10}) pjax.loadUrl("/your-other-url", {timeout: 10})
``` ```
#### `handleResponse(responseText, request, href)`
This method takes the raw response, processes the URL, then calls `pjax.loadContent()` to actually load it into the DOM.
It is passed the following arguments:
* **responseText** (string): This is the raw response text. This is equivalent to `request.responseText`.
* **request** (XMLHttpRequest): This is the XHR object.
* **href** (string): This is the URL that was passed to `loadUrl()`.
You can override this if you want to process the data before, or instead of, it being loaded into the DOM.
For example, if you want to check for a JSON response, you could do the following:
```js
var pjax = new Pjax();
pjax._handleResponse = pjax.handleResponse;
pjax.handleResponse = function(responseText, request, href) {
if (request.responseText.match("<html")) {
pjax._handleResponse(responseText, request, href);
} else {
// handle response here
}
}
```
### Options ### Options
##### `elements` (String, default: `"a[href], form[action]"`) ##### `elements` (String, default: `"a[href], form[action]"`)
@@ -453,7 +481,7 @@ All events are fired from the _document_, not the link that was clicked.
* `pjax:send` - Fired after the Pjax request begins. * `pjax:send` - Fired after the Pjax request begins.
* `pjax:complete` - Fired after the Pjax request finishes. * `pjax:complete` - Fired after the Pjax request finishes.
* `pjax:success` - Fired after the Pjax request succeeds. * `pjax:success` - Fired after the Pjax request succeeds.
* `pjax:error` - Fired after the Pjax request fails. * `pjax:error` - Fired after the Pjax request fails. The request object will be passed along as `event.options.request`.
`send` and `complete` are a good pair of events to use if you are implementing a loading indicator (eg: [topbar](http://buunguyen.github.io/topbar/)) `send` and `complete` are a good pair of events to use if you are implementing a loading indicator (eg: [topbar](http://buunguyen.github.io/topbar/))

View File

@@ -142,6 +142,8 @@ Pjax.prototype = {
doRequest: require("./lib/send-request.js"), doRequest: require("./lib/send-request.js"),
handleResponse: require("./lib/proto/handle-response.js"),
loadUrl: function(href, options) { loadUrl: function(href, options) {
options = typeof options === "object" ? options = typeof options === "object" ?
extend({}, this.options, options) : extend({}, this.options, options) :
@@ -155,66 +157,7 @@ Pjax.prototype = {
trigger(document, "pjax:send", options) trigger(document, "pjax:send", options)
// Do the request // Do the request
this.request = this.doRequest(href, options, function(html, request) { this.request = this.doRequest(href, options, this.handleResponse.bind(this))
// Fail if unable to load HTML via AJAX
if (html === false) {
trigger(document, "pjax:complete pjax:error", options)
return
}
// push scroll position to history
var currentState = window.history.state || {}
window.history.replaceState({
url: currentState.url || window.location.href,
title: currentState.title || document.title,
uid: currentState.uid || newUid(),
scrollPos: [document.documentElement.scrollLeft || document.body.scrollLeft,
document.documentElement.scrollTop || document.body.scrollTop]
},
document.title, window.location)
var oldHref = href
if (request.responseURL) {
if (href !== request.responseURL) {
href = request.responseURL
}
}
else if (request.getResponseHeader("X-PJAX-URL")) {
href = request.getResponseHeader("X-PJAX-URL")
}
else if (request.getResponseHeader("X-XHR-Redirected-To")) {
href = request.getResponseHeader("X-XHR-Redirected-To")
}
// Add back the hash if it was removed
var a = document.createElement("a")
a.href = oldHref
var oldHash = a.hash
a.href = href
if (oldHash && !a.hash) {
a.hash = oldHash
href = a.href
}
this.state.href = href
this.state.options = clone(options)
try {
this.loadContent(html, options)
}
catch (e) {
if (!this.options.debug) {
if (console && console.error) {
console.error("Pjax switch fail: ", e)
}
return this.latestChance(href)
}
else {
throw e
}
}
}.bind(this))
}, },
afterAllSwitches: function() { afterAllSwitches: function() {

View File

@@ -0,0 +1,70 @@
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var clone = require("../util/clone.js")
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var newUid = require("../uniqueid.js")
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var trigger = require("../events/trigger.js")
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
module.exports = function(responseText, request, href) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var tempOptions = clone(this.options);
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
tempOptions.request = request
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
// Fail if unable to load HTML via AJAX
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
if (responseText === false) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
trigger(document, "pjax:complete pjax:error", tempOptions)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
return
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
// push scroll position to history
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var currentState = window.history.state || {}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
window.history.replaceState({
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
url: currentState.url || window.location.href,
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
title: currentState.title || document.title,
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
uid: currentState.uid || newUid(),
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
scrollPos: [document.documentElement.scrollLeft || document.body.scrollLeft,
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
document.documentElement.scrollTop || document.body.scrollTop]
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
},
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
document.title, window.location)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
// Check for redirects
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var oldHref = href
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
if (request.responseURL) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
if (href !== request.responseURL) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
href = request.responseURL
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
else if (request.getResponseHeader("X-PJAX-URL")) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
href = request.getResponseHeader("X-PJAX-URL")
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
else if (request.getResponseHeader("X-XHR-Redirected-To")) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
href = request.getResponseHeader("X-XHR-Redirected-To")
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
// Add back the hash if it was removed
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var a = document.createElement("a")
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
a.href = oldHref
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
var oldHash = a.hash
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
a.href = href
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
if (oldHash && !a.hash) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
a.hash = oldHash
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
href = a.href
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
this.state.href = href
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
this.state.options = clone(this.options)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
try {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
this.loadContent(responseText, this.options)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
catch (e) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
trigger(document, "pjax:error", tempOptions)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
if (!this.options.debug) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
if (console && console.error) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
console.error("Pjax switch fail: ", e)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
return this.latestChance(href)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
else {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
throw e
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
}
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.

View File

@@ -13,21 +13,21 @@ module.exports = function(location, options, callback) {
request.onreadystatechange = function() { request.onreadystatechange = function() {
if (request.readyState === 4) { if (request.readyState === 4) {
if (request.status === 200) { if (request.status === 200) {
callback(request.responseText, request) callback(request.responseText, request, location)
} }
else { else {
callback(null, request) callback(null, request, location)
} }
} }
} }
request.onerror = function(e) { request.onerror = function(e) {
console.log(e) console.log(e)
callback(null, request) callback(null, request, location)
} }
request.ontimeout = function() { request.ontimeout = function() {
callback(null, request) callback(null, request, location)
} }
// Prepare the request payload for forms, if available // Prepare the request payload for forms, if available

View File

@@ -0,0 +1,204 @@
var tape = require("tape")
var handleReponse = require("../../../lib/proto/handle-response")
var noop = require("../../../lib/util/noop")
var href = "https://example.org/"
var storeEventHandler
var pjaxErrorEventTriggerTest
tape("test events triggered when handleResponse(false) is called", function(t) {
t.plan(3)
var pjax = {
options: {
test: 1
}
}
var events = []
storeEventHandler = function(e) {
events.push(e.type)
t.equal(e.test, 1)
}
document.addEventListener("pjax:complete", storeEventHandler)
document.addEventListener("pjax:error", storeEventHandler)
handleReponse.bind(pjax)(false, null)
t.same(events, ["pjax:complete", "pjax:error"], "calling handleResponse(false) triggers 'pjax:complete' and 'pjax:error'")
t.end()
})
tape("test when handleResponse() is called normally", function(t) {
var pjax = {
options: {},
loadContent: noop,
state: {}
}
var request = {
getResponseHeader: noop
}
handleReponse.bind(pjax)("", request, "href")
delete window.history.state.uid
t.same(window.history.state, {
url: href,
title: "",
scrollPos: [0, 0]
}, "window.history.state is set correctly")
t.equals(pjax.state.href, "href", "this.state.href is set correctly")
t.same(pjax.state.options, pjax.options, "this.state.options is set correctly")
t.end()
})
tape("test when handleResponse() is called normally with request.responseURL", function(t) {
var pjax = {
options: {},
loadContent: noop,
state: {}
}
var request = {
responseURL: href + "1",
getResponseHeader: noop
}
handleReponse.bind(pjax)("", request, "")
t.equals(pjax.state.href, request.responseURL, "this.state.href is set correctly")
t.end()
})
tape("test when handleResponse() is called normally with X-PJAX-URL", function(t) {
var pjax = {
options: {},
loadContent: noop,
state: {}
}
var request = {
getResponseHeader: function(header) {
if (header === "X-PJAX-URL") {
return href + "2"
}
}
}
handleReponse.bind(pjax)("", request, "")
t.equals(pjax.state.href, href + "2", "this.state.href is set correctly")
t.end()
})
tape("test when handleResponse() is called normally with X-XHR-Redirected-To", function(t) {
var pjax = {
options: {},
loadContent: noop,
state: {}
}
var request = {
getResponseHeader: function(header) {
if (header === "X-XHR-Redirected-To") {
return href + "3"
}
}
}
handleReponse.bind(pjax)("", request, "")
t.equals(pjax.state.href, href + "3", "this.state.href is set correctly")
t.end()
})
tape("test when handleResponse() is called normally with a hash", function(t) {
var pjax = {
options: {},
loadContent: noop,
state: {}
}
var request = {
responseURL: href + "2",
getResponseHeader: noop
}
handleReponse.bind(pjax)("", request, href + "1#test")
t.equals(pjax.state.href, href + "2#test", "this.state.href is set correctly")
t.end()
})
tape("test try...catch for loadContent() when options.debug is true", function(t) {
t.plan(2)
var pjax = {
options: {},
loadContent: noop,
state: {}
}
var request = {
getResponseHeader: noop
}
pjax.loadContent = function() {
throw new Error()
}
pjax.options.debug = true
document.removeEventListener("pjax:error", storeEventHandler)
pjaxErrorEventTriggerTest = function() {
t.pass("pjax:error event triggered")
}
document.addEventListener("pjax:error", pjaxErrorEventTriggerTest)
t.throws(function() {
handleReponse.bind(pjax)("", request, "")
}, Error, "error is rethrown")
t.end()
})
tape("test try...catch for loadContent()", function(t) {
t.plan(2)
var pjax = {
options: {},
loadContent: noop,
state: {}
}
var request = {
getResponseHeader: noop
}
pjax.loadContent = function() {
throw new Error()
}
pjax.latestChance = function() {
return true
}
pjax.options.debug = false
document.removeEventListener("pjax:error", pjaxErrorEventTriggerTest)
t.doesNotThrow(function() {
t.equals(handleReponse.bind(pjax)("", request, ""), true, "this.latestChance() is called")
}, Error, "error is not thrown")
t.end()
})