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
Showing only changes of commit 1f0e015b3c - Show all commits

View File

@@ -3,11 +3,11 @@ 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: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 trigger = require("../events/trigger.js")
module.exports = function(responseText, request, href) {
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
if (responseText === false) {
var tempOptions = 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.
trigger(document, "pjax:complete pjax:error", tempOptions)
return
@@ -55,9 +55,7 @@ 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: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.
this.loadContent(responseText, this.options)
}
catch (e) {
var tempOptions2 = 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.
tempOptions2.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.
trigger(document, "pjax:error", tempOptions2)
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.
if (!this.options.debug) {
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: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.