From aeeaae8edb5f6ec6ef6b19eaf93dc060a1e67b10 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 28 Jul 2022 17:40:59 -0400 Subject: [PATCH] Return `Promise` from `Turbo.visit` (#650) When consumer applications navigate through the `Turbo.visit`, return a `Promise` that will resolve when the visit is complete. If a visit fails or is cancelled, the `Promise` will be rejected. To achieve this behavior, extract the `ResolvingFunctions` type from the `Renderer` class into the shared `src/core/types.ts` module. Following the pattern established by `Renderer` instances, add a `promise: Promise` property to the `Visit` class, and use that `Promise` to assign a `resolvingFunctions: ResolvingFunctions` property to make the underlying `resolve()` and `reject()` calls available to the `Visit`. When a `Visit` completes successfully, call `this.resolvingFunctions.resolve()`. When it fails or is canceled from the outside, call `this.resolvingFunctions.reject()`. --- src/core/drive/navigator.ts | 10 +++- src/core/drive/visit.ts | 12 ++++- src/core/index.ts | 4 +- src/core/native/adapter.ts | 2 +- src/core/native/browser_adapter.ts | 2 +- src/core/renderer.ts | 6 +-- src/core/session.ts | 6 +-- src/core/types.ts | 5 ++ src/tests/functional/visit_tests.ts | 51 +++++++++++++++++++ .../unit/deprecated_adapter_support_test.ts | 4 +- 10 files changed, 86 insertions(+), 16 deletions(-) diff --git a/src/core/drive/navigator.ts b/src/core/drive/navigator.ts index 1b8f00d42..e52e63b92 100644 --- a/src/core/drive/navigator.ts +++ b/src/core/drive/navigator.ts @@ -9,7 +9,7 @@ import { PageSnapshot } from "./page_snapshot" export type NavigatorDelegate = VisitDelegate & { allowsVisitingLocationWithAction(location: URL, action?: Action): boolean - visitProposedToLocation(location: URL, options: Partial): void + visitProposedToLocation(location: URL, options: Partial): Promise notifyApplicationAfterVisitingSamePageLocation(oldURL: URL, newURL: URL): void } @@ -26,10 +26,14 @@ export class Navigator { proposeVisit(location: URL, options: Partial = {}) { if (this.delegate.allowsVisitingLocationWithAction(location, options.action)) { if (locationIsVisitable(location, this.view.snapshot.rootLocation)) { - this.delegate.visitProposedToLocation(location, options) + return this.delegate.visitProposedToLocation(location, options) } else { window.location.href = location.toString() + + return Promise.resolve() } + } else { + return Promise.reject() } } @@ -41,6 +45,8 @@ export class Navigator { ...options, }) this.currentVisit.start() + + return this.currentVisit.promise } submitForm(form: HTMLFormElement, submitter?: HTMLElement) { diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts index 260739e87..b09d0980a 100644 --- a/src/core/drive/visit.ts +++ b/src/core/drive/visit.ts @@ -5,7 +5,7 @@ import { History } from "./history" import { getAnchor } from "../url" import { Snapshot } from "../snapshot" import { PageSnapshot } from "./page_snapshot" -import { Action } from "../types" +import { Action, ResolvingFunctions } from "../types" import { getHistoryMethodForAction, uuid } from "../../util" import { PageView } from "./page_view" @@ -81,6 +81,9 @@ export class Visit implements FetchRequestDelegate { readonly visitCachedSnapshot: (snapshot: Snapshot) => void readonly willRender: boolean readonly updateHistory: boolean + readonly promise: Promise + + private resolvingFunctions!: ResolvingFunctions followedRedirect = false frame?: number @@ -105,6 +108,7 @@ export class Visit implements FetchRequestDelegate { this.delegate = delegate this.location = location this.restorationIdentifier = restorationIdentifier || uuid() + this.promise = new Promise((resolve, reject) => (this.resolvingFunctions = { resolve, reject })) const { action, @@ -169,6 +173,8 @@ export class Visit implements FetchRequestDelegate { } this.cancelRender() this.state = VisitState.canceled + + this.resolvingFunctions.reject() } } @@ -182,6 +188,8 @@ export class Visit implements FetchRequestDelegate { this.adapter.visitCompleted(this) this.delegate.visitCompleted(this) } + + this.resolvingFunctions.resolve() } } @@ -189,6 +197,8 @@ export class Visit implements FetchRequestDelegate { if (this.state == VisitState.started) { this.state = VisitState.failed this.adapter.visitFailed(this) + + this.resolvingFunctions.reject() } } diff --git a/src/core/index.ts b/src/core/index.ts index 7d8f5a0c6..779931936 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -64,8 +64,8 @@ export function registerAdapter(adapter: Adapter) { * @param options.snapshotHTML Cached snapshot to render * @param options.response Response of the specified location */ -export function visit(location: Locatable, options?: Partial) { - session.visit(location, options) +export function visit(location: Locatable, options?: Partial): Promise { + return session.visit(location, options) } /** diff --git a/src/core/native/adapter.ts b/src/core/native/adapter.ts index 3ecfd9b22..ddf577dc2 100644 --- a/src/core/native/adapter.ts +++ b/src/core/native/adapter.ts @@ -3,7 +3,7 @@ import { FormSubmission } from "../drive/form_submission" import { ReloadReason } from "./browser_adapter" export interface Adapter { - visitProposedToLocation(location: URL, options?: Partial): void + visitProposedToLocation(location: URL, options?: Partial): Promise visitStarted(visit: Visit): void visitCompleted(visit: Visit): void visitFailed(visit: Visit): void diff --git a/src/core/native/browser_adapter.ts b/src/core/native/browser_adapter.ts index 9b5988e3c..c7e53a4e1 100644 --- a/src/core/native/browser_adapter.ts +++ b/src/core/native/browser_adapter.ts @@ -24,7 +24,7 @@ export class BrowserAdapter implements Adapter { } visitProposedToLocation(location: URL, options?: Partial) { - this.navigator.startVisit(location, options?.restorationIdentifier || uuid(), options) + return this.navigator.startVisit(location, options?.restorationIdentifier || uuid(), options) } visitStarted(visit: Visit) { diff --git a/src/core/renderer.ts b/src/core/renderer.ts index a097473d5..53096bc87 100644 --- a/src/core/renderer.ts +++ b/src/core/renderer.ts @@ -1,13 +1,9 @@ +import { ResolvingFunctions } from "./types" import { Bardo, BardoDelegate } from "./bardo" import { Snapshot } from "./snapshot" import { ReloadReason } from "./native/browser_adapter" import { getMetaContent } from "../util" -type ResolvingFunctions = { - resolve(value: T | PromiseLike): void - reject(reason?: any): void -} - export type Render = (newElement: E, currentElement: E) => void export abstract class Renderer = Snapshot> implements BardoDelegate { diff --git a/src/core/session.ts b/src/core/session.ts index a6d53540c..918ce8c63 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -106,8 +106,8 @@ export class Session this.adapter = adapter } - visit(location: Locatable, options: Partial = {}) { - this.navigator.proposeVisit(expandURL(location), options) + visit(location: Locatable, options: Partial = {}): Promise { + return this.navigator.proposeVisit(expandURL(location), options) } connectStreamSource(source: StreamSource) { @@ -194,7 +194,7 @@ export class Session visitProposedToLocation(location: URL, options: Partial) { extendURLWithDeprecatedProperties(location) - this.adapter.visitProposedToLocation(location, options) + return this.adapter.visitProposedToLocation(location, options) } visitStarted(visit: Visit) { diff --git a/src/core/types.ts b/src/core/types.ts index 673ebdfff..de13bbc3f 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -18,3 +18,8 @@ export type StreamSource = { options?: boolean | EventListenerOptions ): void } + +export type ResolvingFunctions = { + resolve(value: T | PromiseLike): void + reject(reason?: any): void +} diff --git a/src/tests/functional/visit_tests.ts b/src/tests/functional/visit_tests.ts index b5603121d..24ad9cda7 100644 --- a/src/tests/functional/visit_tests.ts +++ b/src/tests/functional/visit_tests.ts @@ -28,6 +28,24 @@ test("test programmatically visiting a same-origin location", async ({ page }) = assert.ok(timing) }) +test("test programmatically Turbo.visit-ing a same-origin location", async ({ page }) => { + const urlBeforeVisit = page.url() + await page.evaluate(async () => await window.Turbo.visit("/src/tests/fixtures/one.html")) + + const urlAfterVisit = page.url() + assert.notEqual(urlBeforeVisit, urlAfterVisit) + assert.equal(await visitAction(page), "advance") + + const { url: urlFromBeforeVisitEvent } = await nextEventNamed(page, "turbo:before-visit") + assert.equal(urlFromBeforeVisitEvent, urlAfterVisit) + + const { url: urlFromVisitEvent } = await nextEventNamed(page, "turbo:visit") + assert.equal(urlFromVisitEvent, urlAfterVisit) + + const { timing } = await nextEventNamed(page, "turbo:load") + assert.ok(timing) +}) + test("skip programmatically visiting a cross-origin location falls back to window.location", async ({ page }) => { const urlBeforeVisit = page.url() await visitLocation(page, "about:blank") @@ -37,6 +55,17 @@ test("skip programmatically visiting a cross-origin location falls back to windo assert.equal(await visitAction(page), "load") }) +test("skip programmatically Turbo.visit-ing a cross-origin location falls back to window.location", async ({ + page, +}) => { + const urlBeforeVisit = page.url() + await page.evaluate(async () => await window.Turbo.visit("about:blank")) + + const urlAfterVisit = page.url() + assert.notEqual(urlBeforeVisit, urlAfterVisit) + assert.equal(await visitAction(page), "load") +}) + test("test visiting a location served with a non-HTML content type", async ({ page }) => { const urlBeforeVisit = page.url() await visitLocation(page, "/src/tests/fixtures/svg.svg") @@ -66,6 +95,28 @@ test("test canceling a before-visit event prevents navigation", async ({ page }) assert.equal(urlAfterVisit, urlBeforeVisit) }) +test("test canceling a before-visit event prevents a Turbo.visit-initiated navigation", async ({ page }) => { + await cancelNextVisit(page) + const urlBeforeVisit = page.url() + + assert.notOk( + await willChangeBody(page, async () => { + await page.evaluate(async () => { + try { + return await window.Turbo.visit("/src/tests/fixtures/one.html") + } catch { + const title = document.querySelector("h1") + if (title) title.innerHTML = "Visit canceled" + } + }) + }) + ) + + const urlAfterVisit = page.url() + assert.equal(urlAfterVisit, urlBeforeVisit) + assert.equal(await page.textContent("h1"), "Visit canceled") +}) + test("test navigation by history is not cancelable", async ({ page }) => { await page.click("#same-origin-link") await nextEventNamed(page, "turbo:load") diff --git a/src/tests/unit/deprecated_adapter_support_test.ts b/src/tests/unit/deprecated_adapter_support_test.ts index 071ffe4f4..fa931b404 100644 --- a/src/tests/unit/deprecated_adapter_support_test.ts +++ b/src/tests/unit/deprecated_adapter_support_test.ts @@ -34,8 +34,10 @@ export class DeprecatedAdapterSupportTest extends DOMTestCase implements Adapter // Adapter interface - visitProposedToLocation(location: URL, _options?: Partial): void { + visitProposedToLocation(location: URL, _options?: Partial): Promise { this.locations.push(location) + + return Promise.resolve() } visitStarted(visit: Visit): void {