From b5c2120d085d2a4d5d6896b9638c14e06c905e45 Mon Sep 17 00:00:00 2001 From: BehindTheMath Date: Mon, 22 Jan 2018 10:55:29 -0500 Subject: [PATCH] Fix async switches (#110) If any switches are async, the subsequent code will execute before the switches are finished. This PR moves all that code to a new function, and debounces the calls to onSwitch() so it only executes once, after all the switches finish. Fizes #72. --- index.js | 125 ++++++++++++++++++++-------------- lib/switches-selectors.js | 20 ++++-- package.json | 2 +- tests/lib/switch-selectors.js | 3 +- tests/{index.js => setup.js} | 0 5 files changed, 92 insertions(+), 58 deletions(-) rename tests/{index.js => setup.js} (100%) diff --git a/index.js b/index.js index da4aa8f..eb2f853 100644 --- a/index.js +++ b/index.js @@ -14,6 +14,11 @@ var defaultSwitches = require("./lib/switches") var Pjax = function(options) { this.firstrun = true + this.state = { + numPendingSwitches: 0, + href: null, + options: null + } var parseOptions = require("./lib/proto/parse-options.js"); parseOptions.apply(this,[options]) @@ -83,8 +88,12 @@ Pjax.prototype = { }, onSwitch: function() { - this.parseDOM(document) - trigger(window, "resize scroll") + this.state.numPendingSwitches-- + + // debounce calls, so we only run this once after all switches are finished. + if (this.state.numPendingSwitches === 0) { + this.afterAllSwitches() + } }, loadContent: function(html, options) { @@ -125,22 +134,6 @@ Pjax.prototype = { // try { this.switchSelectors(this.options.selectors, tmpEl, document, options) - // FF bug: Won’t autofocus fields that are inserted via JS. - // This behavior is incorrect. So if theres no current focus, autofocus - // the last field. - // - // http://www.w3.org/html/wg/drafts/html/master/forms.html - var autofocusEl = Array.prototype.slice.call(document.querySelectorAll("[autofocus]")).pop() - if (autofocusEl && document.activeElement !== autofocusEl) { - autofocusEl.focus(); - } - - // execute scripts when DOM have been completely updated - this.options.selectors.forEach(function(selector) { - forEachEls(document.querySelectorAll(selector), function(el) { - executeScripts(el) - }) - }) // } // catch(e) { // if (this.options.debug) { @@ -181,6 +174,8 @@ Pjax.prototype = { else if (request.getResponseHeader("X-XHR-Redirected-To")) { href = request.getResponseHeader("X-XHR-Redirected-To") } + this.state.href = href + this.state.options = clone(options) try { this.loadContent(html, options) @@ -197,49 +192,79 @@ Pjax.prototype = { throw e } } + }.bind(this)) + }, - if (options.history) { - if (this.firstrun) { - this.lastUid = this.maxUid = newUid() - this.firstrun = false - window.history.replaceState({ + afterAllSwitches: function() { + trigger(window, "resize scroll") + + // FF bug: Won’t autofocus fields that are inserted via JS. + // This behavior is incorrect. So if theres no current focus, autofocus + // the last field. + // + // http://www.w3.org/html/wg/drafts/html/master/forms.html + var autofocusEl = Array.prototype.slice.call(document.querySelectorAll("[autofocus]")).pop() + if (autofocusEl && document.activeElement !== autofocusEl) { + autofocusEl.focus(); + } + + // execute scripts when DOM have been completely updated + this.options.selectors.forEach(function(selector) { + forEachEls(document.querySelectorAll(selector), function(el) { + executeScripts(el) + }) + }) + + var state = this.state + + if (state.options.history) { + if (this.firstrun) { + this.lastUid = this.maxUid = newUid() + this.firstrun = false + window.history.replaceState({ url: window.location.href, title: document.title, uid: this.maxUid }, document.title) - } + } - // Update browser history - this.lastUid = this.maxUid = newUid() - window.history.pushState({ - url: href, - title: options.title, + // Update browser history + this.lastUid = this.maxUid = newUid() + + window.history.pushState({ + url: state.href, + title: state.options.title, uid: this.maxUid }, - options.title, - href) + state.options.title, + state.href) + } + + this.forEachSelectors(function(el) { + this.parseDOM(el) + }, this) + + // Fire Events + trigger(document,"pjax:complete pjax:success", state.options) + + state.options.analytics() + + // Scroll page to top on new page load + if (state.options.scrollTo !== false) { + if (state.options.scrollTo.length > 1) { + window.scrollTo(state.options.scrollTo[0], state.options.scrollTo[1]) } - - this.forEachSelectors(function(el) { - this.parseDOM(el) - }, this) - - // Fire Events - trigger(document,"pjax:complete pjax:success", options) - - options.analytics() - - // Scroll page to top on new page load - if (options.scrollTo !== false) { - if (options.scrollTo.length > 1) { - window.scrollTo(options.scrollTo[0], options.scrollTo[1]) - } - else { - window.scrollTo(0, options.scrollTo) - } + else { + window.scrollTo(0, state.options.scrollTo) } - }.bind(this)) + } + + this.state = { + numPendingSwitches: 0, + href: null, + options: null + } } } diff --git a/lib/switches-selectors.js b/lib/switches-selectors.js index 5ae919f..e55c205 100644 --- a/lib/switches-selectors.js +++ b/lib/switches-selectors.js @@ -3,6 +3,8 @@ var forEachEls = require("./foreach-els") var defaultSwitches = require("./switches") module.exports = function(switches, switchesOptions, selectors, fromEl, toEl, options) { + var switchesQueue = []; + selectors.forEach(function(selector) { var newEls = fromEl.querySelectorAll(selector) var oldEls = toEl.querySelectorAll(selector) @@ -24,12 +26,18 @@ module.exports = function(switches, switchesOptions, selectors, fromEl, toEl, op if (this.log) { this.log("newEl", newEl, "oldEl", oldEl) } - if (switches[selector]) { - switches[selector].bind(this)(oldEl, newEl, options, switchesOptions[selector]) - } - else { - defaultSwitches.outerHTML.bind(this)(oldEl, newEl, options) - } + + var callback = (switches[selector]) ? + switches[selector].bind(this, oldEl, newEl, options, switchesOptions[selector]) : + defaultSwitches.outerHTML.bind(this, oldEl, newEl, options) + + switchesQueue.push(callback) }, this) }, this) + + this.state.numPendingSwitches = switchesQueue.length + + switchesQueue.forEach(function(queuedSwitch) { + queuedSwitch() + }) } diff --git a/package.json b/package.json index 7bea2a5..5d2949c 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "lint": "jscs . && jshint . --exclude-path .gitignore", "standalone": "browserify index.js --standalone Pjax > pjax.js", "build-debug": "browserify index.js --debug --standalone Pjax > pjax.js", - "tests": "tape -r ./tests/index.js ./tests/**/*.js", + "tests": "tape -r ./tests/setup.js ./tests/**/*.js", "test": "npm run lint && npm run tests | tap-spec", "coverage-tests": "npm run tests | tap-nyc", "coverage": "nyc -x \"tests/**\" npm run coverage-tests", diff --git a/tests/lib/switch-selectors.js b/tests/lib/switch-selectors.js index 8653227..1e32e04 100644 --- a/tests/lib/switch-selectors.js +++ b/tests/lib/switch-selectors.js @@ -9,7 +9,8 @@ tape("test switchesSelectors", function(t) { var pjax = { onSwitch: function() { console.log("Switched") - } + }, + state: {} } var tmpEl = document.implementation.createHTMLDocument() diff --git a/tests/index.js b/tests/setup.js similarity index 100% rename from tests/index.js rename to tests/setup.js