From 6dce050fb6bd4b9a9040405cdf9a6e0d53ed0d42 Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Tue, 30 Aug 2016 14:54:11 -0400 Subject: [PATCH 01/11] simplify graph reorder logic and switch out order of removal / insertion --- .../javascripts/turbograft/turbohead.coffee | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index aa13dfa1..fe08773b 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -157,11 +157,11 @@ reorderActiveLinks = (activeLinks, upstreamLinks) -> markReorderAsFinished(linkToMove, currentLink, linksToPassBy) if linksToPassBy.length == 0 + linkClone = linkToMove.cloneNode() + document.head.insertBefore(linkClone, currentLink) removeLink(linkToMove, startIndex) - - document.head.insertBefore(linkToMove, activeLinksCopy[i]) - activeLinksCopy.splice(i, 0, linkToMove) - triggerEvent('page:after-link-inserted', linkToMove) + activeLinksCopy.splice(i, 0, linkClone) + triggerEvent('page:after-link-inserted', linkClone) return else addNewReorder(linkToMove, currentLink, pendingReorders) @@ -172,15 +172,10 @@ reorderActiveLinks = (activeLinks, upstreamLinks) -> markReorderAsFinished(linkToMove, currentLink, linksToPassBy) if linksToPassBy.length == 0 + linkClone = linkToMove.cloneNode() + document.head.insertBefore(linkClone, currentLink.nextSibling) removeLink(linkToMove, startIndex) - - targetIndex = i - 1 - if targetIndex == activeLinksCopy.length - 1 - document.head.appendChild(linkToMove) - activeLinksCopy.push(linkToMove) - else - document.head.insertBefore(linkToMove, activeLinksCopy[targetIndex + 1]) - activeLinksCopy.splice(targetIndex + 1, 0, linkToMove) + activeLinksCopy.splice(i, 0, linkToMove) triggerEvent('page:after-link-inserted', linkToMove) return else From 11def969a03c68b44f9ec6c0a76c26992891f925 Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Tue, 30 Aug 2016 14:56:02 -0400 Subject: [PATCH 02/11] emit cloned node --- lib/assets/javascripts/turbograft/turbohead.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index fe08773b..1d9a319f 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -176,7 +176,7 @@ reorderActiveLinks = (activeLinks, upstreamLinks) -> document.head.insertBefore(linkClone, currentLink.nextSibling) removeLink(linkToMove, startIndex) activeLinksCopy.splice(i, 0, linkToMove) - triggerEvent('page:after-link-inserted', linkToMove) + triggerEvent('page:after-link-inserted', linkClone) return else addNewReorder(linkToMove, currentLink, pendingReorders) From dd5d992e4f3288215271743307257734d99de308 Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Tue, 30 Aug 2016 23:34:59 -0400 Subject: [PATCH 03/11] revert turbohead to simpler solution, asynchronously load links before loading scripts --- .../javascripts/turbograft/turbohead.coffee | 238 +++++------------- test/javascripts/fixtures/js/routes.coffee | 78 ++---- test/javascripts/turbolinks_test.coffee | 95 +------ 3 files changed, 88 insertions(+), 323 deletions(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index 1d9a319f..1935b094 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -4,26 +4,24 @@ class window.TurboHead update: (successCallback, failureCallback) -> activeAssets = extractTrackedAssets(@activeDocument) upstreamAssets = extractTrackedAssets(@upstreamDocument) - {activeScripts, newScripts} = processScripts(activeAssets, upstreamAssets) - if hasScriptConflict(activeScripts, newScripts) - return failureCallback() + newScripts = upstreamAssets + .filter(filterForNodeType('SCRIPT')) + .filter(noMatchFor({attribute: 'src', inCollection: activeAssets})) - updateLinkTags(activeAssets, upstreamAssets) - updateScriptTags(@activeDocument, newScripts, successCallback) + newLinks = upstreamAssets + .filter(filterForNodeType('LINK')) + .filter(noMatchFor({attribute: 'href', inCollection: activeAssets})) -updateLinkTags = (activeAssets, upstreamAssets) -> - activeLinks = activeAssets.filter(filterForNodeType('LINK')) - upstreamLinks = upstreamAssets.filter(filterForNodeType('LINK')) - remainingActiveLinks = removeStaleLinks(activeLinks, upstreamLinks) - reorderedActiveLinks = reorderActiveLinks(remainingActiveLinks, upstreamLinks) - insertNewLinks(reorderedActiveLinks, upstreamLinks) + if newScripts.concat(newLinks).some(hasAssetConflicts(activeAssets)) + return failureCallback() -updateScriptTags = (activeDocument, newScripts, callback) -> - asyncSeries( - newScripts.map((scriptNode) -> insertScriptTask(activeDocument, scriptNode)), - callback - ) + updateTasks = [ + (done) => updateLinkTags(@activeDocument, newLinks, done), + (done) => updateScriptTags(@activeDocument, newScripts, done) + ] + + asyncSeries(updateTasks, successCallback) extractTrackedAssets = (doc) -> [].slice.call(doc.querySelectorAll('[data-turbolinks-track]')) @@ -31,179 +29,65 @@ extractTrackedAssets = (doc) -> filterForNodeType = (nodeType) -> (node) -> node.nodeName == nodeType -hasScriptConflict = (activeScripts, newScripts) -> - hasExistingScriptAssetName = (upstreamNode) -> - activeScripts.some (activeNode) -> - upstreamNode.dataset.turbolinksTrackScriptAs == activeNode.dataset.turbolinksTrackScriptAs +noMatchFor = ({attribute, inCollection}) -> + (node) -> + !inCollection.some((nodeFromCollection) -> node[attribute] != nodeFromCollection[attribute]) + +hasAssetConflicts = (activeAssets) -> + (newNode) -> + activeAssets.some((activeNode) -> + trackName = newNode.dataset.turbolinksTrack + trackName == activeNode.dataset.turbolinksTrack && + trackName != 'true' + ) + +updateLinkTags = (activeDocument, newLinks, callback) -> + asyncParallel( + newLinks.map((linkNode) -> insertLinkTask(activeDocument, linkNode)), + callback + ) - newScripts.some(hasExistingScriptAssetName) +updateScriptTags = (activeDocument, newScripts, callback) -> + asyncSeries( + newScripts.map((scriptNode) -> insertScriptTask(activeDocument, scriptNode)), + callback + ) asyncSeries = (tasks, callback) -> return callback() if tasks.length == 0 task = tasks.shift() task(-> asyncSeries(tasks, callback)) + +asyncParallel = (tasks, callback) -> + tasksRemaining = tasks.length + return callback() if tasksRemaining == 0 + + done = () -> + tasksRemaining-- + if (tasksRemaining == 0) + callback() + + tasks.forEach((task) -> task(done)) + insertScriptTask = (activeDocument, scriptNode) -> # We need to clone script tags in order to ensure that the browser executes them. newNode = activeDocument.createElement('SCRIPT') newNode.setAttribute(attr.name, attr.value) for attr in scriptNode.attributes newNode.appendChild(activeDocument.createTextNode(scriptNode.innerHTML)) + insertAssetTask(activeDocument, newNode, 'script') - return (done) -> - onScriptEvent = (event) -> - triggerEvent('page:script-error', event) if event.type == 'error' - newNode.removeEventListener('load', onScriptEvent) - newNode.removeEventListener('error', onScriptEvent) +insertLinkTask = (activeDocument, node) -> + insertAssetTask(activeDocument, node.cloneNode(), 'link') + +insertAssetTask = (activeDocument, newNode, name) -> + (done) -> + onAssetEvent = (event) -> + triggerEvent("page:#{name}-error", event) if event.type == 'error' + newNode.removeEventListener('load', onAssetEvent) + newNode.removeEventListener('error', onAssetEvent) done() - newNode.addEventListener('load', onScriptEvent) - newNode.addEventListener('error', onScriptEvent) + newNode.addEventListener('load', onAssetEvent) + newNode.addEventListener('error', onAssetEvent) activeDocument.head.appendChild(newNode) - triggerEvent('page:after-script-inserted', newNode) - -processScripts = (activeAssets, upstreamAssets) -> - activeScripts = activeAssets.filter(filterForNodeType('SCRIPT')) - upstreamScripts = upstreamAssets.filter(filterForNodeType('SCRIPT')) - hasNewSrc = (upstreamNode) -> - activeScripts.every (activeNode) -> - upstreamNode.src != activeNode.src - - newScripts = upstreamScripts.filter(hasNewSrc) - - {activeScripts, newScripts} - -removeStaleLinks = (activeLinks, upstreamLinks) -> - isStaleLink = (link) -> - upstreamLinks.every (upstreamLink) -> - upstreamLink.href != link.href - - staleLinks = activeLinks.filter(isStaleLink) - - for staleLink in staleLinks - removedLink = document.head.removeChild(staleLink) - triggerEvent('page:after-link-removed', removedLink) - - activeLinks.filter((link) -> !isStaleLink(link)) - -reorderAlreadyExists = (link1, link2, reorders) -> - reorders.some (reorderPair) -> - link1 in reorderPair && link2 in reorderPair - -generateReorderGraph = (activeLinks, upstreamLinks) -> - reorders = [] - for activeLink1 in activeLinks - for activeLink2 in activeLinks - continue if activeLink1.href == activeLink2.href - continue if reorderAlreadyExists(activeLink1, activeLink2, reorders) - - upstreamLink1 = upstreamLinks.filter((link) -> link.href == activeLink1.href)[0] - upstreamLink2 = upstreamLinks.filter((link) -> link.href == activeLink2.href)[0] - - orderHasChanged = - (activeLinks.indexOf(activeLink1) < activeLinks.indexOf(activeLink2)) != - (upstreamLinks.indexOf(upstreamLink1) < upstreamLinks.indexOf(upstreamLink2)) - - reorders.push([activeLink1, activeLink2]) if orderHasChanged - reorders - -nextMove = (activeLinks, reorders) -> - changesAssociatedTo = (link) -> - reorders.filter (reorderPair) -> - link in reorderPair - - linksSortedByMovePriority = activeLinks - .slice() - .sort (link1, link2) -> - changesAssociatedTo(link2).length - changesAssociatedTo(link1).length - - linkToMove = linksSortedByMovePriority[0] - - linksToPassBy = changesAssociatedTo(linkToMove).map (reorderPair) -> - (reorderPair.filter (link) -> link.href != linkToMove.href)[0] - - {linkToMove, linksToPassBy} - -reorderActiveLinks = (activeLinks, upstreamLinks) -> - activeLinksCopy = activeLinks.slice() - pendingReorders = generateReorderGraph(activeLinksCopy, upstreamLinks) - - removeReorder = (link1, link2) -> - reorderToRemove = (pendingReorders.filter (reorderPair) -> - link1 in reorderPair && link2 in reorderPair)[0] - indexToRemove = pendingReorders.indexOf(reorderToRemove) - pendingReorders.splice(indexToRemove, 1) - - addNewReorder = (link1, link2) -> - pendingReorders.push [link1, link2] - - markReorderAsFinished = (linkToMove, linkToPass, remainingLinksToPass) -> - removeReorder(linkToMove, linkToPass) - removalIndex = remainingLinksToPass.indexOf(linkToPass) - remainingLinksToPass.splice(removalIndex, 1) - - removeLink = (linkToRemove, indexOfLink) -> - removedLink = document.head.removeChild(linkToRemove) - triggerEvent('page:after-link-removed', removedLink) - activeLinksCopy.splice(indexOfLink, 1) - - performMove = (linkToMove, linksToPassBy) -> - moveDirection = if activeLinksCopy.indexOf(linkToMove) > activeLinksCopy.indexOf(linksToPassBy[0]) then 'UP' else 'DOWN' - startIndex = activeLinksCopy.indexOf(linkToMove) - - switch moveDirection - when 'UP' - for i in [(startIndex - 1)..0] - currentLink = activeLinksCopy[i] - if currentLink in linksToPassBy - markReorderAsFinished(linkToMove, currentLink, linksToPassBy) - - if linksToPassBy.length == 0 - linkClone = linkToMove.cloneNode() - document.head.insertBefore(linkClone, currentLink) - removeLink(linkToMove, startIndex) - activeLinksCopy.splice(i, 0, linkClone) - triggerEvent('page:after-link-inserted', linkClone) - return - else - addNewReorder(linkToMove, currentLink, pendingReorders) - when 'DOWN' - for i in [(startIndex + 1)...activeLinksCopy.length] - currentLink = activeLinksCopy[i] - if currentLink in linksToPassBy - markReorderAsFinished(linkToMove, currentLink, linksToPassBy) - - if linksToPassBy.length == 0 - linkClone = linkToMove.cloneNode() - document.head.insertBefore(linkClone, currentLink.nextSibling) - removeLink(linkToMove, startIndex) - activeLinksCopy.splice(i, 0, linkToMove) - triggerEvent('page:after-link-inserted', linkClone) - return - else - addNewReorder(linkToMove, currentLink, pendingReorders) - - while pendingReorders.length > 0 - {linkToMove, linksToPassBy} = nextMove(activeLinksCopy, pendingReorders) - performMove(linkToMove, linksToPassBy) - - activeLinksCopy - -insertNewLinks = (activeLinks, upstreamLinks) -> - isNewLink = (link) -> - activeLinks.every (activeLink) -> - activeLink.href != link.href - - upstreamLinks - .filter(isNewLink) - .reverse() # This is because we can't insert before a sibling that hasn't been inserted yet. - .forEach (newUpstreamLink) -> - index = upstreamLinks.indexOf(newUpstreamLink) - newActiveLink = newUpstreamLink.cloneNode() - if index == upstreamLinks.length - 1 - document.head.appendChild(newActiveLink) - activeLinks.push(newActiveLink) - else - targetIndex = activeLinks.indexOf((activeLinks.filter (link) -> - link.href == upstreamLinks[index + 1].href)[0]) - document.head.insertBefore(newActiveLink, activeLinks[targetIndex]) - activeLinks.splice(targetIndex, 0, newActiveLink) - triggerEvent('page:after-link-inserted', newActiveLink) + triggerEvent("page:after-#{name}-inserted", newNode) diff --git a/test/javascripts/fixtures/js/routes.coffee b/test/javascripts/fixtures/js/routes.coffee index 1a90d1d4..348d66da 100644 --- a/test/javascripts/fixtures/js/routes.coffee +++ b/test/javascripts/fixtures/js/routes.coffee @@ -24,8 +24,7 @@ window.ROUTES = { Hi there! @@ -44,12 +43,10 @@ window.ROUTES = { Hi there! @@ -70,12 +67,10 @@ window.ROUTES = { Hi there! @@ -96,8 +91,7 @@ window.ROUTES = { Hi there! @@ -116,8 +110,7 @@ window.ROUTES = { Hi there! @@ -136,8 +129,7 @@ window.ROUTES = { @@ -155,16 +147,13 @@ window.ROUTES = { Hi there! @@ -183,16 +172,13 @@ window.ROUTES = { Hi there! @@ -211,16 +197,13 @@ window.ROUTES = { Hi there! @@ -239,16 +222,13 @@ window.ROUTES = { Hi there! @@ -267,16 +247,13 @@ window.ROUTES = { Hi there! @@ -295,16 +272,13 @@ window.ROUTES = { Hi there! diff --git a/test/javascripts/turbolinks_test.coffee b/test/javascripts/turbolinks_test.coffee index 6fbe1d81..075bdc96 100644 --- a/test/javascripts/turbolinks_test.coffee +++ b/test/javascripts/turbolinks_test.coffee @@ -134,15 +134,6 @@ describe 'Turbolinks', -> assert.equal(linkTagInserted.callCount, 1) done() - it 'dispatches page:after-link-removed event when removing a link on navigation', (done) -> - linkTagRemoved = sinon.spy() - $(document).on 'page:after-link-removed', linkTagRemoved - - visit url: 'singleLinkInHead', -> - visit url: 'noScriptsOrLinkInHead', -> - assert.equal(linkTagRemoved.callCount, 1) - done() - it 'inserts link with a new href into empty head on navigation', (done) -> visit url: 'noScriptsOrLinkInHead', -> assertLinks([]) @@ -157,13 +148,6 @@ describe 'Turbolinks', -> assertLinks(['foo.css', 'bar.css']) done() - it 'inserts link with a new href before exiting scripts in head on navigation', (done) -> - visit url: 'singleLinkInHead', -> - assertLinks(['foo.css']) - visit url: 'twoLinksInHeadReverseOrder', -> - assertLinks(['bar.css', 'foo.css']) - done() - it 'does not reinsert link with existing href into identical head on navigation', (done) -> visit url: 'singleLinkInHead', -> assertLinks(['foo.css']) @@ -171,83 +155,6 @@ describe 'Turbolinks', -> assertLinks(['foo.css']) done() - it 'removes link when navigating to a page with an empty head', (done) -> - visit url: 'singleLinkInHead', -> - assertLinks(['foo.css']) - visit url: 'noScriptsOrLinkInHead', -> - assertLinks([]) - done() - - it 'removes link when navigating to a page with the href missing', (done) -> - visit url: 'twoLinksInHead', -> - assertLinks(['foo.css', 'bar.css']) - visit url: 'singleLinkInHead', -> - assertLinks(['foo.css']) - done() - - describe 'transforms the current head to have the same links in the same order as the upstream document with minimal moves', -> - it 'maintains order when moving from an empty head to a page with link nodes.', (done) -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted - visit url: 'fourLinksInHeadABCD', -> - assertLinks(['a.css', 'b.css', 'c.css', 'd.css']) - assert.equal(linkTagInserted.callCount, 4) - done() - - it 'performs the minimal number of moves for an upwards transform ABCD to BCDA.', (done) -> - visit url: 'fourLinksInHeadABCD', -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted - visit url: 'fourLinksInHeadBCDA', -> - assertLinks(['b.css', 'c.css', 'd.css', 'a.css']) - assert.equal(linkTagInserted.callCount, 1) # Move A - done() - - it 'performs the minimal number of moves for a downwards transform BCDA to ABCD.', (done) -> - visit url: 'fourLinksInHeadBCDA', -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted - visit url: 'fourLinksInHeadABCD', -> - assertLinks(['a.css', 'b.css', 'c.css', 'd.css']) - assert.equal(linkTagInserted.callCount, 1) # Move A - done() - - it 'maintains order when moving from a head with links ABCD to links CDAB.', (done) -> - visit url: 'fourLinksInHeadABCD', -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted - visit url: 'fourLinksInHeadCDAB', -> - assertLinks(['c.css', 'd.css', 'a.css', 'b.css']) - assert.equal(linkTagInserted.callCount, 2) # Move A,B or C,D - done() - - it 'maintains order when moving from a head with links ABCD to links CDBA.', (done) -> - visit url: 'fourLinksInHeadABCD', -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted - visit url: 'fourLinksInHeadCDBA', -> - assertLinks(['c.css', 'd.css', 'b.css', 'a.css']) - assert.equal(linkTagInserted.callCount, 2) # Move A,B - done() - - it 'maintains order when moving from a head with links ABCD to links CBDA.', (done) -> - visit url: 'fourLinksInHeadABCD', -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted - visit url: 'fourLinksInHeadCBDA', -> - assertLinks(['c.css', 'b.css', 'd.css', 'a.css']) - assert.equal(linkTagInserted.callCount, 2) # Move A,B or A,C - done() - - it 'maintains order when moving from a head with links ABCD to links DCBA.', (done) -> - visit url: 'fourLinksInHeadABCD', -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted - visit url: 'fourLinksInHeadDCBA', -> - assertLinks(['d.css', 'c.css', 'b.css', 'a.css']) - assert.equal(linkTagInserted.callCount, 3) # Move any 3 - done() - describe 'head script tag tracking', -> it 'dispatches page:after-script-inserted event when inserting a script on navigation', (done) -> scriptTagInserted = sinon.spy() @@ -348,7 +255,7 @@ describe 'Turbolinks', -> assertScripts(['foo.js', 'bar.js']) done() - it 'refreshes page when a data-turbolinks-track-script-as src changes', (done) -> + it 'refreshes page when a data-turbolinks-track value matches but src changes', (done) -> visit url: 'singleScriptInHead', -> visit url: 'singleScriptInHeadWithDifferentSourceButSameName', -> assert(Turbolinks.fullPageNavigate.called, 'Should perform a full page refresh.') From 1a2be368419c04b8e3a21349914ee1772b37a6ab Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Tue, 30 Aug 2016 23:46:12 -0400 Subject: [PATCH 04/11] wallow in shame at the state of browser support for link load events --- lib/assets/javascripts/turbograft/turbohead.coffee | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index 1935b094..4ab8e7ae 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -31,7 +31,7 @@ filterForNodeType = (nodeType) -> noMatchFor = ({attribute, inCollection}) -> (node) -> - !inCollection.some((nodeFromCollection) -> node[attribute] != nodeFromCollection[attribute]) + !inCollection.some((nodeFromCollection) -> node[attribute] == nodeFromCollection[attribute]) hasAssetConflicts = (activeAssets) -> (newNode) -> @@ -42,10 +42,10 @@ hasAssetConflicts = (activeAssets) -> ) updateLinkTags = (activeDocument, newLinks, callback) -> - asyncParallel( - newLinks.map((linkNode) -> insertLinkTask(activeDocument, linkNode)), - callback - ) + # style tag load events don't work in all browsers + # as such we just hope they load ¯\_(ツ)_/¯ + newLinks.forEach((linkNode) -> insertLinkTask(activeDocument, linkNode)()) + callback() updateScriptTags = (activeDocument, newScripts, callback) -> asyncSeries( From bd66d218af5b438baeb3f73ff1ae07f26d342ace Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Wed, 31 Aug 2016 11:36:48 -0400 Subject: [PATCH 05/11] noOp --- lib/assets/javascripts/turbograft/turbohead.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index 4ab8e7ae..3e0e04e5 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -44,7 +44,7 @@ hasAssetConflicts = (activeAssets) -> updateLinkTags = (activeDocument, newLinks, callback) -> # style tag load events don't work in all browsers # as such we just hope they load ¯\_(ツ)_/¯ - newLinks.forEach((linkNode) -> insertLinkTask(activeDocument, linkNode)()) + newLinks.forEach((linkNode) -> insertLinkTask(activeDocument, linkNode)(noOp)) callback() updateScriptTags = (activeDocument, newScripts, callback) -> @@ -53,6 +53,8 @@ updateScriptTags = (activeDocument, newScripts, callback) -> callback ) +noOp = -> null + asyncSeries = (tasks, callback) -> return callback() if tasks.length == 0 task = tasks.shift() From bca20abbca62db74faa6315f4e766627aed544d7 Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Wed, 31 Aug 2016 13:50:24 -0400 Subject: [PATCH 06/11] remove unused function --- lib/assets/javascripts/turbograft/turbohead.coffee | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index 3e0e04e5..67a68765 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -60,18 +60,6 @@ asyncSeries = (tasks, callback) -> task = tasks.shift() task(-> asyncSeries(tasks, callback)) - -asyncParallel = (tasks, callback) -> - tasksRemaining = tasks.length - return callback() if tasksRemaining == 0 - - done = () -> - tasksRemaining-- - if (tasksRemaining == 0) - callback() - - tasks.forEach((task) -> task(done)) - insertScriptTask = (activeDocument, scriptNode) -> # We need to clone script tags in order to ensure that the browser executes them. newNode = activeDocument.createElement('SCRIPT') From fded8a3a012315bb58d784627fce17a92546ace1 Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Thu, 1 Sep 2016 10:34:04 -0400 Subject: [PATCH 07/11] no more async updateLink --- .../javascripts/turbograft/turbohead.coffee | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index 67a68765..e9557fc9 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -16,12 +16,8 @@ class window.TurboHead if newScripts.concat(newLinks).some(hasAssetConflicts(activeAssets)) return failureCallback() - updateTasks = [ - (done) => updateLinkTags(@activeDocument, newLinks, done), - (done) => updateScriptTags(@activeDocument, newScripts, done) - ] - - asyncSeries(updateTasks, successCallback) + updateLinkTags(@activeDocument, newLinks) + updateScriptTags(@activeDocument, newScripts, successCallback) extractTrackedAssets = (doc) -> [].slice.call(doc.querySelectorAll('[data-turbolinks-track]')) @@ -41,11 +37,10 @@ hasAssetConflicts = (activeAssets) -> trackName != 'true' ) -updateLinkTags = (activeDocument, newLinks, callback) -> +updateLinkTags = (activeDocument, newLinks) -> # style tag load events don't work in all browsers # as such we just hope they load ¯\_(ツ)_/¯ - newLinks.forEach((linkNode) -> insertLinkTask(activeDocument, linkNode)(noOp)) - callback() + newLinks.forEach((linkNode) -> insertLinkTask(activeDocument, linkNode)()) updateScriptTags = (activeDocument, newScripts, callback) -> asyncSeries( @@ -76,7 +71,7 @@ insertAssetTask = (activeDocument, newNode, name) -> triggerEvent("page:#{name}-error", event) if event.type == 'error' newNode.removeEventListener('load', onAssetEvent) newNode.removeEventListener('error', onAssetEvent) - done() + done() if typeof done == 'function' newNode.addEventListener('load', onAssetEvent) newNode.addEventListener('error', onAssetEvent) activeDocument.head.appendChild(newNode) From b0d8949980646eabb95d8947d509d0670373cfaf Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Thu, 1 Sep 2016 10:44:21 -0400 Subject: [PATCH 08/11] refactor assetConflict logic to it's own function, make turbohead memoize newScripts and stuff like that --- .../javascripts/turbograft/turbohead.coffee | 26 +++++++++---------- .../javascripts/turbograft/turbolinks.coffee | 10 +++---- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index e9557fc9..dbb377a8 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -1,23 +1,23 @@ class window.TurboHead constructor: (@activeDocument, @upstreamDocument) -> - - update: (successCallback, failureCallback) -> - activeAssets = extractTrackedAssets(@activeDocument) - upstreamAssets = extractTrackedAssets(@upstreamDocument) - - newScripts = upstreamAssets + @activeAssets = extractTrackedAssets(@activeDocument) + @upstreamAssets = extractTrackedAssets(@upstreamDocument) + @newScripts = @upstreamAssets .filter(filterForNodeType('SCRIPT')) - .filter(noMatchFor({attribute: 'src', inCollection: activeAssets})) + .filter(noMatchFor({attribute: 'src', inCollection: @activeAssets})) - newLinks = upstreamAssets + @newLinks = @upstreamAssets .filter(filterForNodeType('LINK')) - .filter(noMatchFor({attribute: 'href', inCollection: activeAssets})) + .filter(noMatchFor({attribute: 'href', inCollection: @activeAssets})) - if newScripts.concat(newLinks).some(hasAssetConflicts(activeAssets)) - return failureCallback() + hasAssetConflicts: () -> + @newScripts + .concat(@newLinks) + .some(hasAssetConflicts(@activeAssets)) - updateLinkTags(@activeDocument, newLinks) - updateScriptTags(@activeDocument, newScripts, successCallback) + insertNewAssets: (callback) -> + updateLinkTags(@activeDocument, @newLinks) + updateScriptTags(@activeDocument, @newScripts, callback) extractTrackedAssets = (doc) -> [].slice.call(doc.querySelectorAll('[data-turbolinks-track]')) diff --git a/lib/assets/javascripts/turbograft/turbolinks.coffee b/lib/assets/javascripts/turbograft/turbolinks.coffee index d8874120..11306b64 100644 --- a/lib/assets/javascripts/turbograft/turbolinks.coffee +++ b/lib/assets/javascripts/turbograft/turbolinks.coffee @@ -135,13 +135,9 @@ class window.Turbolinks if options.partialReplace updateBody(upstreamDocument, xhr, options) else - new TurboHead(document, upstreamDocument).update( - onHeadUpdateSuccess = -> - updateBody(upstreamDocument, xhr, options) - , - onHeadUpdateError = -> - Turbolinks.fullPageNavigate(url.absolute) - ) + turbohead = new TurboHead(document, upstreamDocument) + return Turbolinks.fullPageNavigate(url.absolute) if turbohead.hasAssetConflicts() + turbohead.insertNewAssets(-> updateBody(upstreamDocument, xhr, options)) else triggerEvent 'page:error', xhr Turbolinks.fullPageNavigate(url.absolute) if url? From 58ccac98d6b3c2241df38d01ce873b2b72e048f5 Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Thu, 1 Sep 2016 14:43:31 -0400 Subject: [PATCH 09/11] add checks for anonymous assets, supporting tests, refactor common filter logic into higher level functions make hasChangedAnonymousAssets work test improvements fix silly tests fix pushState bugs test naming --- .../javascripts/turbograft/turbohead.coffee | 73 +++-- .../javascripts/turbograft/turbolinks.coffee | 7 +- test/javascripts/fixtures/js/routes.coffee | 193 +++++--------- test/javascripts/turbolinks_test.coffee | 251 ++++++++++-------- 4 files changed, 263 insertions(+), 261 deletions(-) diff --git a/lib/assets/javascripts/turbograft/turbohead.coffee b/lib/assets/javascripts/turbograft/turbohead.coffee index dbb377a8..060d2320 100644 --- a/lib/assets/javascripts/turbograft/turbohead.coffee +++ b/lib/assets/javascripts/turbograft/turbohead.coffee @@ -1,41 +1,74 @@ +TRACKED_ASSET_SELECTOR = '[data-turbolinks-track]' +TRACKED_ATTRIBUTE_NAME = 'turbolinksTrack' +ANONYMOUS_TRACK_VALUE = 'true' + class window.TurboHead constructor: (@activeDocument, @upstreamDocument) -> @activeAssets = extractTrackedAssets(@activeDocument) @upstreamAssets = extractTrackedAssets(@upstreamDocument) @newScripts = @upstreamAssets - .filter(filterForNodeType('SCRIPT')) - .filter(noMatchFor({attribute: 'src', inCollection: @activeAssets})) + .filter(attributeMatches('nodeName', 'SCRIPT')) + .filter(noAttributeMatchesIn('src', @activeAssets)) @newLinks = @upstreamAssets - .filter(filterForNodeType('LINK')) - .filter(noMatchFor({attribute: 'href', inCollection: @activeAssets})) + .filter(attributeMatches('nodeName', 'LINK')) + .filter(noAttributeMatchesIn('href', @activeAssets)) - hasAssetConflicts: () -> + hasChangedAnonymousAssets: () -> + anonymousUpstreamAssets = @upstreamAssets + .filter(datasetMatches(TRACKED_ATTRIBUTE_NAME, ANONYMOUS_TRACK_VALUE)) + anonymousActiveAssets = @activeAssets + .filter(datasetMatches(TRACKED_ATTRIBUTE_NAME, ANONYMOUS_TRACK_VALUE)) + + if anonymousActiveAssets.length != anonymousUpstreamAssets.length + return true + + anonymousActiveAssets.some( + noAttributeMatchesIn(TRACKED_ATTRIBUTE_NAME, anonymousUpstreamAssets) + ) + + hasNamedAssetConflicts: () -> @newScripts .concat(@newLinks) - .some(hasAssetConflicts(@activeAssets)) + .filter(noDatasetMatches(TRACKED_ATTRIBUTE_NAME, ANONYMOUS_TRACK_VALUE)) + .some(datasetMatchesIn(TRACKED_ATTRIBUTE_NAME, @activeAssets)) + + hasAssetConflicts: () -> + @hasNamedAssetConflicts() || @hasChangedAnonymousAssets() insertNewAssets: (callback) -> updateLinkTags(@activeDocument, @newLinks) updateScriptTags(@activeDocument, @newScripts, callback) extractTrackedAssets = (doc) -> - [].slice.call(doc.querySelectorAll('[data-turbolinks-track]')) + [].slice.call(doc.querySelectorAll(TRACKED_ASSET_SELECTOR)) -filterForNodeType = (nodeType) -> - (node) -> node.nodeName == nodeType +attributeMatches = (attribute, value) -> + (node) -> node[attribute] == value -noMatchFor = ({attribute, inCollection}) -> +attributeMatchesIn = (attribute, collection) -> (node) -> - !inCollection.some((nodeFromCollection) -> node[attribute] == nodeFromCollection[attribute]) - -hasAssetConflicts = (activeAssets) -> - (newNode) -> - activeAssets.some((activeNode) -> - trackName = newNode.dataset.turbolinksTrack - trackName == activeNode.dataset.turbolinksTrack && - trackName != 'true' - ) + collection.some((nodeFromCollection) -> node[attribute] == nodeFromCollection[attribute]) + +noAttributeMatchesIn = (attribute, collection) -> + (node) -> + !collection.some((nodeFromCollection) -> node[attribute] == nodeFromCollection[attribute]) + +datasetMatches = (attribute, value) -> + (node) -> node.dataset[attribute] == value + +noDatasetMatches = (attribute, value) -> + (node) -> node.dataset[attribute] != value + +datasetMatchesIn = (attribute, collection) -> + (node) -> + value = node.dataset[attribute] + collection.some(datasetMatches(attribute, value)) + +noDatasetMatchesIn = (attribute, collection) -> + (node) -> + value = node.dataset[attribute] + !collection.some(datasetMatches(attribute, value)) updateLinkTags = (activeDocument, newLinks) -> # style tag load events don't work in all browsers @@ -48,8 +81,6 @@ updateScriptTags = (activeDocument, newScripts, callback) -> callback ) -noOp = -> null - asyncSeries = (tasks, callback) -> return callback() if tasks.length == 0 task = tasks.shift() diff --git a/lib/assets/javascripts/turbograft/turbolinks.coffee b/lib/assets/javascripts/turbograft/turbolinks.coffee index 11306b64..6e9ba442 100644 --- a/lib/assets/javascripts/turbograft/turbolinks.coffee +++ b/lib/assets/javascripts/turbograft/turbolinks.coffee @@ -131,12 +131,15 @@ class window.Turbolinks triggerEvent 'page:receive' options.updatePushState ?= true if upstreamDocument = processResponse(xhr) - reflectNewUrl url if options.updatePushState if options.partialReplace + reflectNewUrl url if options.updatePushState updateBody(upstreamDocument, xhr, options) else turbohead = new TurboHead(document, upstreamDocument) - return Turbolinks.fullPageNavigate(url.absolute) if turbohead.hasAssetConflicts() + if turbohead.hasAssetConflicts() + return Turbolinks.fullPageNavigate(url.absolute) + + reflectNewUrl url if options.updatePushState turbohead.insertNewAssets(-> updateBody(upstreamDocument, xhr, options)) else triggerEvent 'page:error', xhr diff --git a/test/javascripts/fixtures/js/routes.coffee b/test/javascripts/fixtures/js/routes.coffee index 348d66da..75225b14 100644 --- a/test/javascripts/fixtures/js/routes.coffee +++ b/test/javascripts/fixtures/js/routes.coffee @@ -35,6 +35,25 @@ window.ROUTES = { """ ], + singleScriptInHeadTrackTrue: [ + 200, + {'Content-Type':'text/html'}, + """ + + + + + Hi there! + + +
+ + + """ + ], + twoScriptsInHead: [ 200, {'Content-Type':'text/html'}, @@ -57,6 +76,50 @@ window.ROUTES = { """ ], + twoScriptsInHeadTrackTrue: [ + 200, + {'Content-Type':'text/html'}, + """ + + + + + + Hi there! + + +
+ + + """ + ], + + twoScriptsInHeadTrackTrueAgain: [ + 200, + {'Content-Type':'text/html'}, + """ + + + + + + Hi there! + + +
Merp
+ + + """ + ], + twoScriptsInHeadOneBroken: [ 200, {'Content-Type':'text/html'}, @@ -296,7 +359,7 @@ window.ROUTES = { - + Hi there! @@ -313,8 +376,8 @@ window.ROUTES = { - - + + Hi there! @@ -331,128 +394,8 @@ window.ROUTES = { - - - Hi there! - - -
- - - """ - ], - - fourLinksInHeadABCD: [ - 200, - {'Content-Type':'text/html'}, - """ - - - - - - - - Hi there! - - -
- - - """ - ], - - fourLinksInHeadBCDA: [ - 200, - {'Content-Type':'text/html'}, - """ - - - - - - - - Hi there! - - -
- - - """ - ], - - fourLinksInHeadCDAB: [ - 200, - {'Content-Type':'text/html'}, - """ - - - - - - - - Hi there! - - -
- - - """ - ], - - fourLinksInHeadCDBA: [ - 200, - {'Content-Type':'text/html'}, - """ - - - - - - - - Hi there! - - -
- - - """ - ], - - fourLinksInHeadCBDA: [ - 200, - {'Content-Type':'text/html'}, - """ - - - - - - - - Hi there! - - -
- - - """ - ], - - fourLinksInHeadDCBA: [ - 200, - {'Content-Type':'text/html'}, - """ - - - - - - - + + Hi there! diff --git a/test/javascripts/turbolinks_test.coffee b/test/javascripts/turbolinks_test.coffee index 075bdc96..d96765e6 100644 --- a/test/javascripts/turbolinks_test.coffee +++ b/test/javascripts/turbolinks_test.coffee @@ -122,150 +122,175 @@ describe 'Turbolinks', -> assert(yourCallback.calledOnce, 'Callback was not called.') done() - describe 'head link tag tracking', -> - it 'dispatches page:after-link-inserted event when inserting a link on navigation', (done) -> - linkTagInserted = sinon.spy() - $(document).on 'page:after-link-inserted', linkTagInserted + describe 'head asset tracking', -> + it 'refreshes page when a data-turbolinks-track value matches but src changes', (done) -> + visit url: 'singleScriptInHeadTwo', -> + visit url: 'singleScriptInHeadWithDifferentSourceButSameName', -> + assert(Turbolinks.fullPageNavigate.called, 'Should perform a full page refresh.') + done() - visit url: 'noScriptsOrLinkInHead', -> - assertLinks([]) - visit url: 'singleLinkInHead', -> - assertLinks(['foo.css']) - assert.equal(linkTagInserted.callCount, 1) + it 'does not refresh page when new data-turbolinks-track values encountered', (done) -> + visit url: 'singleScriptInHead', -> + visit url: 'twoScriptsInHead', -> + assert(Turbolinks.fullPageNavigate.notCalled, 'Should not perform a full page refresh.') done() - it 'inserts link with a new href into empty head on navigation', (done) -> - visit url: 'noScriptsOrLinkInHead', -> - assertLinks([]) - visit url: 'singleLinkInHead', -> - assertLinks(['foo.css']) + describe 'using data-turbolinks-track="true"', -> + startFromFixture = (route) -> + fixtureHTML = ROUTES[route][2] + $fixture = $(fixtureHTML) + document.body.innerHTML = $fixture.find('body').html() + document.head.innerHTML = $fixture.find('head').html() + + it 'refreshes page when a new tracked node is present', (done) -> + visit url: 'singleScriptInHeadTrackTrue', -> + assert(Turbolinks.fullPageNavigate.called, 'Should perform a full page refresh.') done() - it 'inserts link with a new href into existing head on navigation', (done) -> - visit url: 'singleLinkInHead', -> - assertLinks(['foo.css']) - visit url: 'twoLinksInHead', -> - assertLinks(['foo.css', 'bar.css']) + it 'refreshes page when an extant tracked node is missing', (done) -> + startFromFixture('twoScriptsInHeadTrackTrue') + visit url: 'singleScriptInHeadTrackTrue', -> + assert(Turbolinks.fullPageNavigate.callCount, 2, 'Should perform two full page refreshes.') done() - it 'does not reinsert link with existing href into identical head on navigation', (done) -> - visit url: 'singleLinkInHead', -> - assertLinks(['foo.css']) + it 'does not refresh page when tracked nodes have matching sources', (done) -> + startFromFixture('singleScriptInHeadTrackTrue') + visit url: 'singleScriptInHeadTrackTrue', -> + assert.equal(Turbolinks.fullPageNavigate.callCount, 1, 'Should not perform a full page refresh.') + done() + + describe 'link tags', -> + it 'dispatches page:after-link-inserted event when inserting a link on navigation', (done) -> + linkTagInserted = sinon.spy() + $(document).on 'page:after-link-inserted', linkTagInserted + + visit url: 'noScriptsOrLinkInHead', -> + assertLinks([]) + visit url: 'singleLinkInHead', -> + assertLinks(['foo.css']) + assert.equal(linkTagInserted.callCount, 1) + done() + + it 'inserts link with a new href into empty head on navigation', (done) -> + visit url: 'noScriptsOrLinkInHead', -> + assertLinks([]) + visit url: 'singleLinkInHead', -> + assertLinks(['foo.css']) + done() + + it 'inserts link with a new href into existing head on navigation', (done) -> visit url: 'singleLinkInHead', -> assertLinks(['foo.css']) - done() + visit url: 'twoLinksInHead', -> + assertLinks(['foo.css', 'bar.css']) + done() - describe 'head script tag tracking', -> - it 'dispatches page:after-script-inserted event when inserting a script on navigation', (done) -> - scriptTagInserted = sinon.spy() - $(document).on 'page:after-script-inserted', scriptTagInserted + it 'does not reinsert link with existing href into identical head on navigation', (done) -> + visit url: 'singleLinkInHead', -> + assertLinks(['foo.css']) + visit url: 'singleLinkInHead', -> + assertLinks(['foo.css']) + done() - visit url: 'noScriptsOrLinkInHead', -> + describe 'script tags', -> + it 'dispatches page:after-script-inserted event when inserting a script on navigation', (done) -> + scriptTagInserted = sinon.spy() + $(document).on 'page:after-script-inserted', scriptTagInserted + + visit url: 'noScriptsOrLinkInHead', -> + visit url: 'singleScriptInHead', -> + assert.equal(scriptTagInserted.callCount, 1) + done() + + it 'inserts script with a new src into empty head on navigation', (done) -> + visit url: 'noScriptsOrLinkInHead', -> + assertScripts([]) + visit url: 'singleScriptInHead', -> + assertScripts(['foo.js']) + done() + + it 'inserts script with a new src into existing head on navigation', (done) -> visit url: 'singleScriptInHead', -> - assert.equal(scriptTagInserted.callCount, 1) - done() + assertScripts(['foo.js']) + visit url: 'twoScriptsInHead', -> + assertScripts(['foo.js', 'bar.js']) + done() - it 'inserts script with a new src into empty head on navigation', (done) -> - visit url: 'noScriptsOrLinkInHead', -> - assertScripts([]) + it 'does not insert duplicate script tag on navigation into identical upstream head', (done) -> visit url: 'singleScriptInHead', -> assertScripts(['foo.js']) - done() + visit url: 'singleScriptInHead', -> + assertScripts(['foo.js']) + done() - it 'inserts script with a new src into existing head on navigation', (done) -> - visit url: 'singleScriptInHead', -> - assertScripts(['foo.js']) - visit url: 'twoScriptsInHead', -> - assertScripts(['foo.js', 'bar.js']) - done() + it 'does not insert duplicate script tag on navigation into superset upstream head', (done) -> + visit url: 'singleScriptInHead', -> + assertScripts(['foo.js']) + visit url: 'twoScriptsInHead', -> + assertScripts(['foo.js', 'bar.js']) + done() - it 'does not insert duplicate script tag on navigation into identical upstream head', (done) -> - visit url: 'singleScriptInHead', -> - assertScripts(['foo.js']) + it 'does not remove script when navigating to a page with an empty head', (done) -> visit url: 'singleScriptInHead', -> assertScripts(['foo.js']) - done() + visit url: 'noScriptsOrLinkInHead', -> + assertScripts(['foo.js']) + done() - it 'does not insert duplicate script tag on navigation into superset upstream head', (done) -> - visit url: 'singleScriptInHead', -> - assertScripts(['foo.js']) + it 'does not remove script nodes when navigating to a page with less script tags', (done) -> visit url: 'twoScriptsInHead', -> assertScripts(['foo.js', 'bar.js']) - done() - - it 'does not remove script when navigating to a page with an empty head', (done) -> - visit url: 'singleScriptInHead', -> - assertScripts(['foo.js']) - visit url: 'noScriptsOrLinkInHead', -> - assertScripts(['foo.js']) - done() - - describe 'executes scripts in the order they are present in the dom of the upstream document', -> - beforeEach -> window.actualExecutionOrder = [] - afterEach -> delete window.actualExecutionOrder - - it 'works in order ABC', (done) -> - expectedScriptOrder = ['a', 'b', 'c'] - visit url: 'threeScriptsInHeadABC', -> - assert.deepEqual(actualExecutionOrder, expectedScriptOrder) - done() - - it 'works in order ACB', (done) -> - visit url: 'threeScriptsInHeadACB', -> - expectedScriptOrder = ['a', 'c', 'b'] - assert.deepEqual(actualExecutionOrder, expectedScriptOrder) - done() + visit url: 'singleScriptInHead', -> + assertScripts(['foo.js', 'bar.js']) + done() - it 'works in order BAC', (done) -> - visit url: 'threeScriptsInHeadBAC', -> - expectedScriptOrder = ['b', 'a', 'c'] - assert.deepEqual(actualExecutionOrder, expectedScriptOrder) - done() + describe 'executes scripts in the order they are present in the dom of the upstream document', -> + beforeEach -> window.actualExecutionOrder = [] + afterEach -> delete window.actualExecutionOrder - it 'works in order BCA', (done) -> - visit url: 'threeScriptsInHeadBCA', -> - expectedScriptOrder = ['b', 'c', 'a'] - assert.deepEqual(actualExecutionOrder, expectedScriptOrder) - done() + it 'works in order ABC', (done) -> + expectedScriptOrder = ['a', 'b', 'c'] + visit url: 'threeScriptsInHeadABC', -> + assert.deepEqual(actualExecutionOrder, expectedScriptOrder) + done() - it 'works in order CAB', (done) -> - visit url: 'threeScriptsInHeadCAB', -> - expectedScriptOrder = ['c', 'a', 'b'] - assert.deepEqual(actualExecutionOrder, expectedScriptOrder) - done() + it 'works in order ACB', (done) -> + visit url: 'threeScriptsInHeadACB', -> + expectedScriptOrder = ['a', 'c', 'b'] + assert.deepEqual(actualExecutionOrder, expectedScriptOrder) + done() - it 'works in order CBA', (done) -> - visit url: 'threeScriptsInHeadCBA', -> - expectedScriptOrder = ['c', 'b', 'a'] - assert.deepEqual(actualExecutionOrder, expectedScriptOrder) - done() + it 'works in order BAC', (done) -> + visit url: 'threeScriptsInHeadBAC', -> + expectedScriptOrder = ['b', 'a', 'c'] + assert.deepEqual(actualExecutionOrder, expectedScriptOrder) + done() - it 'executes new scripts in the relative order they are present in the dom of the upstream document', (done) -> - visit url: 'secondLibraryOnly', -> - assert.equal(actualExecutionOrder[0], 'b') - visit url: 'threeScriptsInHeadABC', -> - assert.equal(actualExecutionOrder[1], 'a') - assert.equal(actualExecutionOrder[2], 'c') + it 'works in order BCA', (done) -> + visit url: 'threeScriptsInHeadBCA', -> + expectedScriptOrder = ['b', 'c', 'a'] + assert.deepEqual(actualExecutionOrder, expectedScriptOrder) done() - it 'does not remove script nodes when navigating to a page with less script tags', (done) -> - visit url: 'twoScriptsInHead', -> - assertScripts(['foo.js', 'bar.js']) - visit url: 'singleScriptInHead', -> - assertScripts(['foo.js', 'bar.js']) - done() + it 'works in order CAB', (done) -> + visit url: 'threeScriptsInHeadCAB', -> + expectedScriptOrder = ['c', 'a', 'b'] + assert.deepEqual(actualExecutionOrder, expectedScriptOrder) + done() - it 'refreshes page when a data-turbolinks-track value matches but src changes', (done) -> - visit url: 'singleScriptInHead', -> - visit url: 'singleScriptInHeadWithDifferentSourceButSameName', -> - assert(Turbolinks.fullPageNavigate.called, 'Should perform a full page refresh.') - done() + it 'works in order CBA', (done) -> + visit url: 'threeScriptsInHeadCBA', -> + expectedScriptOrder = ['c', 'b', 'a'] + assert.deepEqual(actualExecutionOrder, expectedScriptOrder) + done() - it 'it refreshes inline when a new data-turbolinks-track-script-as is added', (done) -> - visit url: 'singleScriptInHead', -> - visit url: 'differentSingleScriptInHead', -> - assert(Turbolinks.fullPageNavigate.notCalled, 'Should not perform a full page refresh.') - done() + it 'executes new scripts in the relative order they are present in the dom of the upstream document', (done) -> + visit url: 'secondLibraryOnly', -> + assert.equal(actualExecutionOrder[0], 'b') + visit url: 'threeScriptsInHeadABC', -> + assert.equal(actualExecutionOrder[1], 'a') + assert.equal(actualExecutionOrder[2], 'c') + done() describe 'with partial page replacement', -> beforeEach -> window.globalStub = stub() From f55434a0c9c3a2105b27fc09229a1906302be269 Mon Sep 17 00:00:00 2001 From: Mathew Allen Date: Fri, 2 Sep 2016 11:34:40 -0400 Subject: [PATCH 10/11] readme modifications --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index 58df30ba..323b14f5 100644 --- a/README.md +++ b/README.md @@ -9,8 +9,12 @@ In botany, one can take parts of a tree and splice it onto another tree. The DO ## One render path Turbograft gives you the ability to maintain a single, canonical render path for views. Your ERB views are the single definition of what will be rendered, without the worry of conditionally fetching snippets of HTML from elsewhere. This approach leads to clear, simplified code. + ## Client-side performance Partial page refreshes mean that CSS and JavaScript are only reloaded when you need them to be. Turbograft improves on the native, single-page application feel for the user while keeping these benefits inherited from Turbolinks. + +Head asset tracking means that you can split your large CSS and Javascript bundles into smaller area bundles, decreasing your page weight and further increasing the responsiveness of your app. + ## Simplicity Turbograft was built with simplicity in mind. It intends to offer the smallest amount of overhead required on top of a traditional Rails stack to solve the problem of making a Rails app feel native to the browser. @@ -160,6 +164,16 @@ and The `data-tg-remote-noserialize` is useful in scenarios where a whole section of the page should be editable, i.e. not `disabled`, but should only conditionally be submitted to the server. +## Head Asset Tracking +### NOTE: This functionality is experimental, has changed significantly since 0.3.0, and may change again in the future before 1.0 +The Turbohead module allows you to track css and javascript assets in the head of the document and change them intelligently. This can be useful in large applications which want to lighten their asset weight by splitting their script and style bundles by area. + +When a ` - Hi there! @@ -387,6 +387,42 @@ window.ROUTES = { """ ], + twoLinksInHeadTrackTrue: [ + 200, + {'Content-Type':'text/html'}, + """ + + + + + + Hi there! + + +
+ + + """ + ], + + twoLinksInHeadTrackTrueOneChanged: [ + 200, + {'Content-Type':'text/html'}, + """ + + + + + + Hi there! + + +
+ + + """ + ], + twoLinksInHeadReverseOrder: [ 200, {'Content-Type':'text/html'}, diff --git a/test/javascripts/turbolinks_test.coffee b/test/javascripts/turbolinks_test.coffee index d96765e6..85397f61 100644 --- a/test/javascripts/turbolinks_test.coffee +++ b/test/javascripts/turbolinks_test.coffee @@ -138,9 +138,7 @@ describe 'Turbolinks', -> describe 'using data-turbolinks-track="true"', -> startFromFixture = (route) -> fixtureHTML = ROUTES[route][2] - $fixture = $(fixtureHTML) - document.body.innerHTML = $fixture.find('body').html() - document.head.innerHTML = $fixture.find('head').html() + document.documentElement.innerHTML = fixtureHTML it 'refreshes page when a new tracked node is present', (done) -> visit url: 'singleScriptInHeadTrackTrue', -> @@ -150,13 +148,25 @@ describe 'Turbolinks', -> it 'refreshes page when an extant tracked node is missing', (done) -> startFromFixture('twoScriptsInHeadTrackTrue') visit url: 'singleScriptInHeadTrackTrue', -> - assert(Turbolinks.fullPageNavigate.callCount, 2, 'Should perform two full page refreshes.') + assert(Turbolinks.fullPageNavigate.called, 'Should perform a full page refresh.') + done() + + it 'refreshes page when upstream and active tracked scripts lengths are equal but one\'s source changes', (done) -> + startFromFixture('twoScriptsInHeadTrackTrue') + visit url: 'twoScriptsInHeadTrackTrueOneChanged', -> + assert(Turbolinks.fullPageNavigate.called, 'Should perform a full page refresh.') + done() + + it 'refreshes page when upstream and active tracked links lengths are equal but one\'s source changes', (done) -> + startFromFixture('twoLinksInHeadTrackTrue') + visit url: 'twoLinksInHeadTrackTrueOneChanged', -> + assert(Turbolinks.fullPageNavigate.called, 'Should perform a full page refresh.') done() it 'does not refresh page when tracked nodes have matching sources', (done) -> startFromFixture('singleScriptInHeadTrackTrue') visit url: 'singleScriptInHeadTrackTrue', -> - assert.equal(Turbolinks.fullPageNavigate.callCount, 1, 'Should not perform a full page refresh.') + assert(Turbolinks.fullPageNavigate.notCalled, 'Should not perform a full page refresh.') done() describe 'link tags', ->