From 17e54f4dcfc0ca0ebfba926ab96af0ca2fb5f1f1 Mon Sep 17 00:00:00 2001 From: Mike Robbins Date: Thu, 9 Mar 2023 16:57:50 -0500 Subject: [PATCH 1/3] Simple demo of cache bug at /__turbo/timestamp --- src/tests/fixtures/timestamp.html | 12 ++++++++++++ src/tests/server.ts | 6 ++++++ 2 files changed, 18 insertions(+) create mode 100644 src/tests/fixtures/timestamp.html diff --git a/src/tests/fixtures/timestamp.html b/src/tests/fixtures/timestamp.html new file mode 100644 index 000000000..271865c95 --- /dev/null +++ b/src/tests/fixtures/timestamp.html @@ -0,0 +1,12 @@ + + + + + Turbo + + + +

Hello $TIMESTAMP

+

Refresh

+ + diff --git a/src/tests/server.ts b/src/tests/server.ts index 399e930be..ea95cb0b8 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -67,6 +67,12 @@ router.get("/headers", (request, response) => { .send(template.replace("$HEADERS", JSON.stringify(request.headers, null, 4))) }) +router.get("/timestamp", (request, response) => { + const template = fs.readFileSync("src/tests/fixtures/timestamp.html").toString() + const timestamp = (new Date().getTime() / 1000).toString() + setTimeout(() => response.type("html").status(200).send(template.replaceAll("$TIMESTAMP", timestamp)), 500) +}) + router.get("/delayed_response", (request, response) => { const fixture = path.join(__dirname, "../../src/tests/fixtures/one.html") setTimeout(() => response.status(200).sendFile(fixture), 1000) From 8a53a3d70f42552125433232818e91ceb99d509e Mon Sep 17 00:00:00 2001 From: Mike Robbins Date: Thu, 9 Mar 2023 17:29:06 -0500 Subject: [PATCH 2/3] Use isSameCacheKey to prevent previewing stale cache snapshot on same-page link click --- src/core/drive/visit.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts index 6028f579b..9060307b6 100644 --- a/src/core/drive/visit.ts +++ b/src/core/drive/visit.ts @@ -2,7 +2,7 @@ import { Adapter } from "../native/adapter" import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetch_request" import { FetchResponse } from "../../http/fetch_response" import { History } from "./history" -import { getAnchor } from "../url" +import { getAnchor, toCacheKey } from "../url" import { Snapshot } from "../snapshot" import { PageSnapshot } from "./page_snapshot" import { Action } from "../types" @@ -293,11 +293,12 @@ export class Visit implements FetchRequestDelegate { loadCachedSnapshot() { const snapshot = this.getCachedSnapshot() + const isSameCacheKey = toCacheKey(this.location) === toCacheKey(this.view.lastRenderedLocation) if (snapshot) { const isPreview = this.shouldIssueRequest() this.render(async () => { this.cacheSnapshot() - if (this.isSamePage) { + if (isSameCacheKey) { this.adapter.visitRendered(this) } else { if (this.view.renderPromise) await this.view.renderPromise From 7a6b785e522d3b18b85b18401719cb205db9042e Mon Sep 17 00:00:00 2001 From: Mike Robbins Date: Thu, 9 Mar 2023 18:12:58 -0500 Subject: [PATCH 3/3] Add test for cache bug --- src/tests/fixtures/timestamp.html | 4 ++-- src/tests/functional/visit_tests.ts | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/tests/fixtures/timestamp.html b/src/tests/fixtures/timestamp.html index 271865c95..a4078f079 100644 --- a/src/tests/fixtures/timestamp.html +++ b/src/tests/fixtures/timestamp.html @@ -6,7 +6,7 @@ -

Hello $TIMESTAMP

-

Refresh

+

Hello $TIMESTAMP

+

Refresh

diff --git a/src/tests/functional/visit_tests.ts b/src/tests/functional/visit_tests.ts index 146bcda58..0f344dc65 100644 --- a/src/tests/functional/visit_tests.ts +++ b/src/tests/functional/visit_tests.ts @@ -220,6 +220,31 @@ test("test Visit with network error", async ({ page }) => { await nextEventNamed(page, "turbo:fetch-request-error") }) +test("test clicking link to current page does not preview a stale cache snapshot", async ({ page }) => { + const timestamps: number[] = [] + const fetchTimestamp = async () => timestamps.push(parseFloat(await page.locator("#timestamp").innerText())) + const assertTimestampNotDecreasing = () => { + const n = timestamps.length + if (n < 2) return + assert(timestamps[n - 1] >= timestamps[n - 2], "timestamps are non-decreasing") + } + + await visitLocation(page, "/__turbo/timestamp") + await nextEventNamed(page, "turbo:load") + await fetchTimestamp() + + for (let i = 0; i < 5; i++) { + await page.click("a#refresh") + await nextEventNamed(page, "turbo:render") + await fetchTimestamp() + assertTimestampNotDecreasing() + + await nextEventNamed(page, "turbo:load") + await fetchTimestamp() + assertTimestampNotDecreasing() + } +}) + async function visitLocation(page: Page, location: string) { return page.evaluate((location) => window.Turbo.visit(location), location) }