Skip to content

Commit

Permalink
Move cache snapshot loading to Visit#start()
Browse files Browse the repository at this point in the history
The cache interface changed in #949

Since the cache can be loaded from disk using the browser Cache API, it is
possible that the cache snapshot will be loaded asynchronously. This means
that the calls to load cache snapshots need to be awaited.

We also changed visit.hasCachedSnapshot() to be async and return a Promise.
However, [hasCachedSnapshot is used in the iOS adapter](https://github.com/hotwired/turbo-ios/blob/c476bac66f260adbfe930ed9a151e7637973ff99/Source/WebView/turbo.js#L119)
and serialized into and data object we send to the native side via
postMessage. When postMessagereceives a Promise instead of a boolean
value it fails with a DataCloneError since it can't serialize the Promise.

This commit moves the cache snapshot loading to Visit#start() and stores
the result in a cachedSnapshot property. This allows us to keep the
hasCachedSnapshot() method synchronous and return a boolean value.

It means that Visit.start is now async, but I haven't found any callers
that rely on it being synchronous.
  • Loading branch information
afcapel committed Oct 31, 2023
1 parent 725fd51 commit 23b2c6b
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/core/drive/navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class Navigator {
referrer: this.location,
...options
})
this.currentVisit.start()
return this.currentVisit.start()
}

submitForm(form, submitter) {
Expand Down
14 changes: 7 additions & 7 deletions src/core/drive/visit.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,11 @@ export class Visit {
return this.isSamePage
}

start() {
async start() {
if (this.state == VisitState.initialized) {
this.recordTimingMetric(TimingMetric.visitStart)
this.state = VisitState.started
this.cachedSnapshot = await this.getCachedSnapshot()
this.adapter.visitStarted(this)
this.delegate.visitStarted(this)
}
Expand Down Expand Up @@ -231,13 +232,12 @@ export class Visit {
}
}

async hasCachedSnapshot() {
return (await this.getCachedSnapshot()) != null
hasCachedSnapshot() {
return this.cachedSnapshot != null
}

async loadCachedSnapshot() {
const snapshot = await this.getCachedSnapshot()
if (snapshot) {
if (this.cachedSnapshot) {
const isPreview = await this.shouldIssueRequest()
this.render(async () => {
this.cacheSnapshot()
Expand All @@ -246,7 +246,7 @@ export class Visit {
} else {
if (this.view.renderPromise) await this.view.renderPromise

await this.renderPageSnapshot(snapshot, isPreview)
await this.renderPageSnapshot(this.cachedSnapshot, isPreview)

this.adapter.visitRendered(this)
if (!isPreview) {
Expand Down Expand Up @@ -395,7 +395,7 @@ export class Visit {
if (this.isSamePage) {
return false
} else if (this.action === "restore") {
return !(await this.hasCachedSnapshot())
return !this.hasCachedSnapshot()
} else {
return this.willRender
}
Expand Down
2 changes: 1 addition & 1 deletion src/tests/unit/deprecated_adapter_support_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ test("test visit proposal location includes deprecated absoluteURL property", as
})

test("test visit start location includes deprecated absoluteURL property", async () => {
Turbo.navigator.startVisit(window.location.toString(), "123")
await Turbo.navigator.startVisit(window.location.toString(), "123")
assert.equal(adapter.locations.length, 1)

const [location] = adapter.locations
Expand Down
14 changes: 7 additions & 7 deletions src/tests/unit/native_adapter_support_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ test("test visit proposal external location is proposed to adapter", async () =>
test("test visit started notifies adapter", async () => {
const locatable = window.location.toString()

Turbo.navigator.startVisit(locatable)
await Turbo.navigator.startVisit(locatable)
assert.equal(adapter.startedVisits.length, 1)

const [visit] = adapter.startedVisits
Expand All @@ -99,7 +99,7 @@ test("test visit started notifies adapter", async () => {
test("test visit completed notifies adapter", async () => {
const locatable = window.location.toString()

Turbo.navigator.startVisit(locatable)
await Turbo.navigator.startVisit(locatable)

const [startedVisit] = adapter.startedVisits
startedVisit.complete()
Expand All @@ -111,7 +111,7 @@ test("test visit completed notifies adapter", async () => {
test("test visit request started notifies adapter", async () => {
const locatable = window.location.toString()

Turbo.navigator.startVisit(locatable)
await Turbo.navigator.startVisit(locatable)

const [startedVisit] = adapter.startedVisits
startedVisit.startRequest()
Expand All @@ -124,7 +124,7 @@ test("test visit request started notifies adapter", async () => {
test("test visit request completed notifies adapter", async () => {
const locatable = window.location.toString()

Turbo.navigator.startVisit(locatable)
await Turbo.navigator.startVisit(locatable)

const [startedVisit] = adapter.startedVisits
startedVisit.recordResponse({ statusCode: 200, responseHTML: "responseHtml", redirected: false })
Expand All @@ -137,7 +137,7 @@ test("test visit request completed notifies adapter", async () => {
test("test visit request failed notifies adapter", async () => {
const locatable = window.location.toString()

Turbo.navigator.startVisit(locatable)
await Turbo.navigator.startVisit(locatable)

const [startedVisit] = adapter.startedVisits
startedVisit.recordResponse({ statusCode: 404, responseHTML: "responseHtml", redirected: false })
Expand All @@ -150,7 +150,7 @@ test("test visit request failed notifies adapter", async () => {
test("test visit request finished notifies adapter", async () => {
const locatable = window.location.toString()

Turbo.navigator.startVisit(locatable)
await Turbo.navigator.startVisit(locatable)

const [startedVisit] = adapter.startedVisits
startedVisit.finishRequest()
Expand Down Expand Up @@ -181,7 +181,7 @@ test("test visit follows redirect and proposes replace visit to adapter", async
const locatable = window.location.toString()
const redirectedLocation = "https://example.com"

Turbo.navigator.startVisit(locatable)
await Turbo.navigator.startVisit(locatable)

const [startedVisit] = adapter.startedVisits
startedVisit.redirectedToLocation = redirectedLocation
Expand Down

0 comments on commit 23b2c6b

Please sign in to comment.