From 050fe38127f2cc3734751febabb284adf5d55d4c Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Thu, 14 Sep 2023 19:56:32 +0100 Subject: [PATCH 1/9] Add direction detail to turbo:visit events --- src/core/drive/history.js | 12 +++++++++--- src/core/drive/visit.js | 10 +++++++++- src/core/session.js | 11 ++++++----- src/tests/fixtures/visit.html | 1 + src/tests/functional/visit_tests.js | 23 +++++++++++++++++++++++ 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/core/drive/history.js b/src/core/drive/history.js index 215f56eca..45015b0cc 100644 --- a/src/core/drive/history.js +++ b/src/core/drive/history.js @@ -6,6 +6,7 @@ export class History { restorationData = {} started = false pageLoaded = false + currentIndex = 0 constructor(delegate) { this.delegate = delegate @@ -15,6 +16,7 @@ export class History { if (!this.started) { addEventListener("popstate", this.onPopState, false) addEventListener("load", this.onPageLoad, false) + this.currentIndex = history.state?.turbo?.restorationIndex || 0 this.started = true this.replace(new URL(window.location.href)) } @@ -37,7 +39,9 @@ export class History { } update(method, location, restorationIdentifier = uuid()) { - const state = { turbo: { restorationIdentifier } } + if (method === history.pushState) ++this.currentIndex + + const state = { turbo: { restorationIdentifier, restorationIndex: this.currentIndex } } method.call(history, state, "", location.href) this.location = location this.restorationIdentifier = restorationIdentifier @@ -81,9 +85,11 @@ export class History { const { turbo } = event.state || {} if (turbo) { this.location = new URL(window.location.href) - const { restorationIdentifier } = turbo + const { restorationIdentifier, restorationIndex } = turbo this.restorationIdentifier = restorationIdentifier - this.delegate.historyPoppedToLocationWithRestorationIdentifier(this.location, restorationIdentifier) + const direction = restorationIndex > this.currentIndex ? "forward" : "back" + this.delegate.historyPoppedToLocationWithRestorationIdentifierAndDirection(this.location, restorationIdentifier, direction) + this.currentIndex = restorationIndex } } } diff --git a/src/core/drive/visit.js b/src/core/drive/visit.js index 7fc494c19..a29c96692 100644 --- a/src/core/drive/visit.js +++ b/src/core/drive/visit.js @@ -36,6 +36,12 @@ export const SystemStatusCode = { contentTypeMismatch: -2 } +export const Direction = { + advance: "forward", + restore: "back", + replace: "none" +} + export class Visit { identifier = uuid() // Required by turbo-ios timingMetrics = {} @@ -65,7 +71,8 @@ export class Visit { willRender, updateHistory, shouldCacheSnapshot, - acceptsStreamResponse + acceptsStreamResponse, + direction } = { ...defaultOptions, ...options @@ -83,6 +90,7 @@ export class Visit { this.scrolled = !willRender this.shouldCacheSnapshot = shouldCacheSnapshot this.acceptsStreamResponse = acceptsStreamResponse + this.direction = direction || Direction[action] } get adapter() { diff --git a/src/core/session.js b/src/core/session.js index 44edc7856..868804c59 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -125,11 +125,12 @@ export class Session { // History delegate - historyPoppedToLocationWithRestorationIdentifier(location, restorationIdentifier) { + historyPoppedToLocationWithRestorationIdentifierAndDirection(location, restorationIdentifier, direction) { if (this.enabled) { this.navigator.startVisit(location, restorationIdentifier, { action: "restore", - historyChanged: true + historyChanged: true, + direction }) } else { this.adapter.pageInvalidated({ @@ -188,7 +189,7 @@ export class Session { } extendURLWithDeprecatedProperties(visit.location) if (!visit.silent) { - this.notifyApplicationAfterVisitingLocation(visit.location, visit.action) + this.notifyApplicationAfterVisitingLocation(visit.location, visit.action, visit.direction) } } @@ -313,8 +314,8 @@ export class Session { }) } - notifyApplicationAfterVisitingLocation(location, action) { - return dispatch("turbo:visit", { detail: { url: location.href, action } }) + notifyApplicationAfterVisitingLocation(location, action, direction) { + return dispatch("turbo:visit", { detail: { url: location.href, action, direction } }) } notifyApplicationBeforeCachingSnapshot() { diff --git a/src/tests/fixtures/visit.html b/src/tests/fixtures/visit.html index f4cef2457..19e40e5bb 100644 --- a/src/tests/fixtures/visit.html +++ b/src/tests/fixtures/visit.html @@ -13,6 +13,7 @@

Visit

Same-origin link

+

Same-origin replace link

Same-origin link with ?key=value

Sample response

Same page link

diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index 102cc8a2b..eb44110ec 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -220,6 +220,29 @@ test("test Visit with network error", async ({ page }) => { await nextEventNamed(page, "turbo:fetch-request-error") }) +test("test turbo:visit direction details", async ({ page }) => { + await page.click("#same-origin-link") + let details = await nextEventNamed(page, "turbo:visit") + assert.equal(details.direction, "forward") + + await nextEventNamed(page, "turbo:load") + await page.goBack() + details = await nextEventNamed(page, "turbo:visit") + assert.equal(details.direction, "back") + + await nextEventNamed(page, "turbo:load") + await page.goForward() + details = await nextEventNamed(page, "turbo:visit") + assert.equal(details.direction, "forward") + + await nextEventNamed(page, "turbo:load") + await page.goBack() + await nextEventNamed(page, "turbo:load") + await page.click("#same-origin-replace-link") + details = await nextEventNamed(page, "turbo:visit") + assert.equal(details.direction, "none") +}) + async function visitLocation(page, location) { return page.evaluate((location) => window.Turbo.visit(location), location) } From 31114162ea8f2d2b80812ac55404eb980e1b04ff Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Thu, 14 Sep 2023 20:21:57 +0100 Subject: [PATCH 2/9] Test restorationIndex is persisted between reloads --- src/tests/functional/visit_tests.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index eb44110ec..e3bac99c7 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -243,6 +243,17 @@ test("test turbo:visit direction details", async ({ page }) => { assert.equal(details.direction, "none") }) +test("test turbo:visit direction details after a reload", async ({ page }) => { + await page.click("#same-origin-link") + await nextEventNamed(page, "turbo:load") + await page.reload() + assert.equal( + await page.evaluate(() => window.history.state.turbo.restorationIndex), + 1, + "restorationIndex is persisted between reloads" + ) +}) + async function visitLocation(page, location) { return page.evaluate((location) => window.Turbo.visit(location), location) } From c39c2236d186cf7fc7f90f55b70155137975ca54 Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Thu, 28 Sep 2023 18:42:32 +0100 Subject: [PATCH 3/9] Add data-turbo-visit-direction to html element --- src/core/session.js | 2 ++ src/core/view.js | 8 ++++++++ src/tests/functional/visit_tests.js | 4 +++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/core/session.js b/src/core/session.js index 868804c59..39fe8377c 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -186,6 +186,7 @@ export class Session { visitStarted(visit) { if (!visit.acceptsStreamResponse) { markAsBusy(document.documentElement) + this.view.markVisitDirection(visit.direction) } extendURLWithDeprecatedProperties(visit.location) if (!visit.silent) { @@ -194,6 +195,7 @@ export class Session { } visitCompleted(visit) { + this.view.unmarkVisitDirection() clearBusyState(document.documentElement) this.notifyApplicationAfterPageLoad(visit.getTimingMetrics()) } diff --git a/src/core/view.js b/src/core/view.js index ca81e8bdb..5c62259f1 100644 --- a/src/core/view.js +++ b/src/core/view.js @@ -99,6 +99,14 @@ export class View { } } + markVisitDirection(direction) { + this.element.setAttribute("data-turbo-visit-direction", direction) + } + + unmarkVisitDirection() { + this.element.removeAttribute("data-turbo-visit-direction") + } + async renderSnapshot(renderer) { await renderer.render() } diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index e3bac99c7..2ab405965 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -13,6 +13,7 @@ import { readEventLogs, scrollToSelector, visitAction, + waitUntilSelector, willChangeBody } from "../helpers/page" @@ -221,7 +222,8 @@ test("test Visit with network error", async ({ page }) => { }) test("test turbo:visit direction details", async ({ page }) => { - await page.click("#same-origin-link") + page.click("#same-origin-link") + await waitUntilSelector(page, "[data-turbo-visit-direction='forward']") let details = await nextEventNamed(page, "turbo:visit") assert.equal(details.direction, "forward") From badfd694c19614ea47a108503b155ed85d2c0685 Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Tue, 3 Oct 2023 18:44:58 +0100 Subject: [PATCH 4/9] Versatile Visit direction API. Use html[data-turbo-visit-direction] attribute to communicate visit direction rather than using an event detail --- src/core/session.js | 6 ++-- src/tests/functional/visit_tests.js | 48 +++++++++++++++++++---------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/core/session.js b/src/core/session.js index 39fe8377c..a94bf1c91 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -190,7 +190,7 @@ export class Session { } extendURLWithDeprecatedProperties(visit.location) if (!visit.silent) { - this.notifyApplicationAfterVisitingLocation(visit.location, visit.action, visit.direction) + this.notifyApplicationAfterVisitingLocation(visit.location, visit.action) } } @@ -316,8 +316,8 @@ export class Session { }) } - notifyApplicationAfterVisitingLocation(location, action, direction) { - return dispatch("turbo:visit", { detail: { url: location.href, action, direction } }) + notifyApplicationAfterVisitingLocation(location, action) { + return dispatch("turbo:visit", { detail: { url: location.href, action } }) } notifyApplicationBeforeCachingSnapshot() { diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index 2ab405965..58da4efa4 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -221,31 +221,45 @@ test("test Visit with network error", async ({ page }) => { await nextEventNamed(page, "turbo:fetch-request-error") }) -test("test turbo:visit direction details", async ({ page }) => { - page.click("#same-origin-link") - await waitUntilSelector(page, "[data-turbo-visit-direction='forward']") - let details = await nextEventNamed(page, "turbo:visit") - assert.equal(details.direction, "forward") - - await nextEventNamed(page, "turbo:load") - await page.goBack() - details = await nextEventNamed(page, "turbo:visit") - assert.equal(details.direction, "back") +test("test Visit direction data attribute when clicking a link", async ({ page }) => { + await Promise.all([ + waitUntilSelector(page, "[data-turbo-visit-direction='forward']") + .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), + page.click("#same-origin-link") + ]) +}) +test("test Visit direction detdata attribute when navigating back", async ({ page }) => { + await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") - await page.goForward() - details = await nextEventNamed(page, "turbo:visit") - assert.equal(details.direction, "forward") + await Promise.all([ + waitUntilSelector(page, "[data-turbo-visit-direction='back']") + .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), + page.goBack() + ]) +}) +test("test Visit direction attribute when navigating forward", async ({ page }) => { + await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") await page.goBack() await nextEventNamed(page, "turbo:load") - await page.click("#same-origin-replace-link") - details = await nextEventNamed(page, "turbo:visit") - assert.equal(details.direction, "none") + await Promise.all([ + waitUntilSelector(page, "[data-turbo-visit-direction='forward']") + .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), + page.goForward() + ]) +}) + +test("test Visit direction attribute on a replace visit", async ({ page }) => { + await Promise.all([ + waitUntilSelector(page, "[data-turbo-visit-direction='none']") + .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), + page.click("#same-origin-replace-link") + ]) }) -test("test turbo:visit direction details after a reload", async ({ page }) => { +test("test Turbo history state after a reload", async ({ page }) => { await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") await page.reload() From 319adf2ccd7c5460e90c9889372851439f736be4 Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Fri, 17 Nov 2023 12:10:14 +0000 Subject: [PATCH 5/9] Remove "test" prefix from test description --- src/tests/functional/visit_tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index 58da4efa4..b4a0f3c92 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -221,7 +221,7 @@ test("test Visit with network error", async ({ page }) => { await nextEventNamed(page, "turbo:fetch-request-error") }) -test("test Visit direction data attribute when clicking a link", async ({ page }) => { +test("Visit direction data attribute when clicking a link", async ({ page }) => { await Promise.all([ waitUntilSelector(page, "[data-turbo-visit-direction='forward']") .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), From 6912e8891cbfd816843dbbe486935c120c7e7d87 Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Fri, 17 Nov 2023 12:11:59 +0000 Subject: [PATCH 6/9] Fix typos --- src/tests/functional/visit_tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index b4a0f3c92..17f97b647 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -229,7 +229,7 @@ test("Visit direction data attribute when clicking a link", async ({ page }) => ]) }) -test("test Visit direction detdata attribute when navigating back", async ({ page }) => { +test("Visit direction data attribute when navigating back", async ({ page }) => { await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") await Promise.all([ From 2bc7c13c66b2a31c4c46c63819d4f7f052538e6f Mon Sep 17 00:00:00 2001 From: Dom Christie Date: Fri, 17 Nov 2023 12:14:01 +0000 Subject: [PATCH 7/9] Remove "test" prefix from test description --- src/tests/functional/visit_tests.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index 17f97b647..f54fc5e44 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -239,7 +239,7 @@ test("Visit direction data attribute when navigating back", async ({ page }) => ]) }) -test("test Visit direction attribute when navigating forward", async ({ page }) => { +test("Visit direction attribute when navigating forward", async ({ page }) => { await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") await page.goBack() @@ -251,7 +251,7 @@ test("test Visit direction attribute when navigating forward", async ({ page }) ]) }) -test("test Visit direction attribute on a replace visit", async ({ page }) => { +test("Visit direction attribute on a replace visit", async ({ page }) => { await Promise.all([ waitUntilSelector(page, "[data-turbo-visit-direction='none']") .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), @@ -259,7 +259,7 @@ test("test Visit direction attribute on a replace visit", async ({ page }) => { ]) }) -test("test Turbo history state after a reload", async ({ page }) => { +test("Turbo history state after a reload", async ({ page }) => { await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") await page.reload() From 70cda684cc819e2a2ecd69528a3a1536985021e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= Date: Fri, 17 Nov 2023 13:23:09 +0000 Subject: [PATCH 8/9] Clean up assertions --- src/tests/functional/visit_tests.js | 40 ++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index f54fc5e44..fb44ab8ef 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -13,6 +13,7 @@ import { readEventLogs, scrollToSelector, visitAction, + waitUntilNoSelector, waitUntilSelector, willChangeBody } from "../helpers/page" @@ -222,21 +223,18 @@ test("test Visit with network error", async ({ page }) => { }) test("Visit direction data attribute when clicking a link", async ({ page }) => { - await Promise.all([ - waitUntilSelector(page, "[data-turbo-visit-direction='forward']") - .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), - page.click("#same-origin-link") - ]) + page.click("#same-origin-link") + + await assertVisitDirectionAttribute(page, "forward") }) test("Visit direction data attribute when navigating back", async ({ page }) => { await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") - await Promise.all([ - waitUntilSelector(page, "[data-turbo-visit-direction='back']") - .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), - page.goBack() - ]) + + page.goBack() + + await assertVisitDirectionAttribute(page, "back") }) test("Visit direction attribute when navigating forward", async ({ page }) => { @@ -244,19 +242,16 @@ test("Visit direction attribute when navigating forward", async ({ page }) => { await nextEventNamed(page, "turbo:load") await page.goBack() await nextEventNamed(page, "turbo:load") - await Promise.all([ - waitUntilSelector(page, "[data-turbo-visit-direction='forward']") - .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), - page.goForward() - ]) + + page.goForward() + + await assertVisitDirectionAttribute(page, "forward") }) test("Visit direction attribute on a replace visit", async ({ page }) => { - await Promise.all([ - waitUntilSelector(page, "[data-turbo-visit-direction='none']") - .then(() => waitUntilSelector(page, "html:not([data-turbo-visit-direction])")), - page.click("#same-origin-replace-link") - ]) + page.click("#same-origin-replace-link") + + await assertVisitDirectionAttribute(page, "none") }) test("Turbo history state after a reload", async ({ page }) => { @@ -273,3 +268,8 @@ test("Turbo history state after a reload", async ({ page }) => { async function visitLocation(page, location) { return page.evaluate((location) => window.Turbo.visit(location), location) } + +async function assertVisitDirectionAttribute(page, direction) { + await waitUntilSelector(page, `[data-turbo-visit-direction='${direction}']`) + await waitUntilNoSelector(page, "[data-turbo-visit-direction]") +} From c6f82a57117d7ed4d75ab37e0c847a7de2c9c514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= Date: Fri, 17 Nov 2023 13:50:08 +0000 Subject: [PATCH 9/9] Read the mutation logs to assert the direction attribute change It's less dependent on the timing of the mutation. Even if the attribute is no longer present, we can still assert that it was present at some point. --- src/tests/functional/visit_tests.js | 8 +++++--- src/tests/helpers/page.js | 6 ++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index fb44ab8ef..31a11141a 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -6,15 +6,16 @@ import { getSearchParam, isScrolledToSelector, isScrolledToTop, + nextAttributeMutationNamed, nextBeat, nextEventNamed, noNextAttributeMutationNamed, pathname, readEventLogs, + resetMutationLogs, scrollToSelector, visitAction, waitUntilNoSelector, - waitUntilSelector, willChangeBody } from "../helpers/page" @@ -224,7 +225,6 @@ test("test Visit with network error", async ({ page }) => { test("Visit direction data attribute when clicking a link", async ({ page }) => { page.click("#same-origin-link") - await assertVisitDirectionAttribute(page, "forward") }) @@ -232,6 +232,8 @@ test("Visit direction data attribute when navigating back", async ({ page }) => await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") + await resetMutationLogs(page) + page.goBack() await assertVisitDirectionAttribute(page, "back") @@ -270,6 +272,6 @@ async function visitLocation(page, location) { } async function assertVisitDirectionAttribute(page, direction) { - await waitUntilSelector(page, `[data-turbo-visit-direction='${direction}']`) + assert.equal(await nextAttributeMutationNamed(page, "html", "data-turbo-visit-direction"), direction) await waitUntilNoSelector(page, "[data-turbo-visit-direction]") } diff --git a/src/tests/helpers/page.js b/src/tests/helpers/page.js index 580327d25..5d203681b 100644 --- a/src/tests/helpers/page.js +++ b/src/tests/helpers/page.js @@ -162,6 +162,12 @@ export function propertyForSelector(page, selector, propertyName) { return page.locator(selector).evaluate((element, propertyName) => element[propertyName], propertyName) } +export function resetMutationLogs(page) { + return page.evaluate(() => { + window.mutationLogs = [] + }) +} + async function readArray(page, identifier, length) { return page.evaluate( ({ identifier, length }) => {