From c8dbbab79144f5885d704ad59890e789be3f66d0 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Mon, 25 Oct 2021 19:22:25 -0400 Subject: [PATCH] Extract `FrameVisit` to drive `FrameController` The problem --- Programmatically driving a `` element when its `[src]` attribute changes is a suitable end-user experience in consumer applications. It's a fitting black-box interface for the outside world: change the value of the attribute and let Turbo handle the rest. However, internally, it's a lossy abstraction. For example, when the `FrameRedirector` class listens for page-wide `click` and `submit` events, it determines if their targets are meant to drive a `` element by: 1. finding an element that matches a clicked `` element's `[data-turbo-frame]` attribute 2. finding an element that matches a submitted `
` element's `[data-turbo-frame]` attribute 3. finding an element that matches a submitted `` element's _submitter's_ `[data-turbo-frame]` attribute 4. finding the closest `` ancestor to the `` or `` Once it finds the matching frame element, it disposes of all that additional context and navigates the `` by updating its `[src]` attribute. This makes it impossible to control various aspects of the frame navigation (like its "rendering" explored in [hotwired/turbo#146][]) outside of its destination URL. Similarly, since a `` and submitter pairing have an impact on which `` is navigated, the `FrameController` implementation passes around a `HTMLFormElement` and `HTMLSubmitter?` data clump and constantly re-fetches a matching `` instance. Outside of frames, page-wide navigation is driven by a `Visit` instance that manages the HTTP life cycle and delegates along the way to a `VisitDelegate`. It also pairs calls to visit with a `VisitOption` object to capture additional context. The proposal --- This commit introduces the `FrameVisit` class. It serves as an encapsulation of the `FetchRequest` and `FormSubmission` lifecycle events involved in navigating a frame. It's implementation draws inspiration from the `Visit`, `VisitDelegate`, and `VisitOptions` pairing. Since the `FrameVisit` knows how to unify both `FetchRequest` and `FormSubmission` hooks, the resulting callbacks fired from within the `FrameController` are flat and consistent. Extra benefits --- The biggest benefit is the introduction of a DRY abstraction to manage the behind the scenes HTTP calls necessary to drive a ``. With the introduction of the `FrameVisit` concept, we can also declare a `visit()` and `submit()` method for `FrameElementDelegate` implementations in the place of other implementation-specific methods like `loadResponse()` and `formSubmissionIntercepted()`. In addition, these changes have the potential to close [hotwired/turbo#326][], since we can consistently invoke `loadResponse()` across ``-click-initiated and ``-submission-initiated visits. To ensure that's the case, this commit adds test coverage for navigating a `` by making a `GET` request to an endpoint that responds with a `500` status. [hotwired/turbo#146]: https://github.com/hotwired/turbo/pull/146 [hotwired/turbo#326]: https://github.com/hotwired/turbo/issues/326 --- src/core/frames/frame_controller.ts | 210 +++++------------- src/core/frames/frame_visit.ts | 183 +++++++++++++++ src/core/session.ts | 3 +- src/elements/frame_element.ts | 8 +- src/tests/fixtures/tabs/three.html | 29 ++- src/tests/fixtures/tabs/two.html | 29 ++- .../functional/frame_navigation_tests.ts | 117 +++++++++- src/tests/functional/frame_tests.ts | 20 +- 8 files changed, 418 insertions(+), 181 deletions(-) create mode 100644 src/core/frames/frame_visit.ts diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 48c37c9a6..22b60992c 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -4,20 +4,10 @@ import { FrameLoadingStyle, FrameElementObservedAttribute, } from "../../elements/frame_element" -import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetch_request" +import { FetchRequest, TurboFetchRequestErrorEvent } from "../../http/fetch_request" import { FetchResponse } from "../../http/fetch_response" import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer" -import { - clearBusyState, - dispatch, - getAttribute, - parseHTMLDocument, - markAsBusy, - uuid, - getHistoryMethodForAction, - getVisitAction, -} from "../../util" -import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission" +import { dispatch, getAttribute, parseHTMLDocument, uuid, getHistoryMethodForAction } from "../../util" import { Snapshot } from "../snapshot" import { ViewDelegate, ViewRenderOptions } from "../view" import { Locatable, getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url" @@ -28,10 +18,9 @@ import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../obse import { FrameRenderer } from "./frame_renderer" import { session } from "../index" import { Action } from "../types" +import { FrameVisit, FrameVisitDelegate, FrameVisitOptions } from "./frame_visit" import { VisitOptions } from "../drive/visit" import { TurboBeforeFrameRenderEvent } from "../session" -import { StreamMessage } from "../streams/stream_message" -import { PageSnapshot } from "../drive/page_snapshot" type VisitFallback = (location: Response | Locatable, options: Partial) => Promise export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: VisitFallback }> @@ -39,11 +28,10 @@ export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: Vi export class FrameController implements AppearanceObserverDelegate, - FetchRequestDelegate, FormSubmitObserverDelegate, - FormSubmissionDelegate, FrameElementDelegate, FormLinkClickObserverDelegate, + FrameVisitDelegate, LinkInterceptorDelegate, ViewDelegate> { @@ -53,17 +41,12 @@ export class FrameController readonly formLinkClickObserver: FormLinkClickObserver readonly linkInterceptor: LinkInterceptor readonly formSubmitObserver: FormSubmitObserver - formSubmission?: FormSubmission - fetchResponseLoaded = (_fetchResponse: FetchResponse) => {} - private currentFetchRequest: FetchRequest | null = null - private resolveVisitPromise = () => {} + frameVisit?: FrameVisit private connected = false private hasBeenLoaded = false private ignoredAttributes: Set = new Set() - private action: Action | null = null readonly restorationIdentifier: string private previousFrameElement?: FrameElement - private currentNavigationElement?: Element constructor(element: FrameElement) { this.element = element @@ -99,6 +82,11 @@ export class FrameController } } + visit(options: FrameVisitOptions): Promise { + const frameVisit = new FrameVisit(this, this.element, options) + return frameVisit.start() + } + disabledChanged() { if (this.loadingStyle == FrameLoadingStyle.eager) { this.loadSourceURL() @@ -144,14 +132,11 @@ export class FrameController private async loadSourceURL() { if (this.enabled && this.isActive && !this.complete && this.sourceURL) { - this.element.loaded = this.visit(expandURL(this.sourceURL)) - this.appearanceObserver.stop() - await this.element.loaded - this.hasBeenLoaded = true + await this.visit({ url: this.sourceURL }) } } - async loadResponse(fetchResponse: FetchResponse) { + async loadResponse(fetchResponse: FetchResponse, frameVisit: FrameVisit) { if (fetchResponse.redirected || (fetchResponse.succeeded && fetchResponse.isHTML)) { this.sourceURL = fetchResponse.response.url } @@ -173,13 +158,13 @@ export class FrameController false ) if (this.view.renderPromise) await this.view.renderPromise - this.changeHistory() + this.changeHistory(frameVisit.action) await this.view.render(renderer) this.complete = true session.frameRendered(fetchResponse, this.element) session.frameLoaded(this.element) - this.fetchResponseLoaded(fetchResponse) + this.proposeVisitIfNavigatedWithAction(frameVisit, fetchResponse) } else if (this.willHandleFrameMissingFromResponse(fetchResponse)) { console.warn( `A matching frame for #${this.element.id} was missing from the response, transforming into full-page Visit.` @@ -190,15 +175,12 @@ export class FrameController } catch (error) { console.error(error) this.view.invalidate() - } finally { - this.fetchResponseLoaded = () => {} } } // Appearance observer delegate - elementAppearedInViewport(element: FrameElement) { - this.proposeVisitIfNavigatedWithAction(element, element) + elementAppearedInViewport(_element: FrameElement) { this.loadSourceURL() } @@ -230,78 +212,42 @@ export class FrameController } formSubmitted(element: HTMLFormElement, submitter?: HTMLElement) { - if (this.formSubmission) { - this.formSubmission.stop() - } - - this.formSubmission = new FormSubmission(this, element, submitter) - const { fetchRequest } = this.formSubmission - this.prepareRequest(fetchRequest) - this.formSubmission.start() - } - - // Fetch request delegate - - prepareRequest(request: FetchRequest) { - request.headers["Turbo-Frame"] = this.id - - if (this.currentNavigationElement?.hasAttribute("data-turbo-stream")) { - request.acceptResponseType(StreamMessage.contentType) - } - } - - requestStarted(_request: FetchRequest) { - markAsBusy(this.element) - } - - requestPreventedHandlingResponse(_request: FetchRequest, _response: FetchResponse) { - this.resolveVisitPromise() - } - - async requestSucceededWithResponse(request: FetchRequest, response: FetchResponse) { - await this.loadResponse(response) - this.resolveVisitPromise() - } - - async requestFailedWithResponse(request: FetchRequest, response: FetchResponse) { - console.error(response) - await this.loadResponse(response) - this.resolveVisitPromise() + const frame = this.findFrameElement(element, submitter) + frame.delegate.visit(FrameVisit.optionsForSubmit(element, submitter)) } - requestErrored(request: FetchRequest, error: Error) { - console.error(error) - this.resolveVisitPromise() - } + // Frame visit delegate - requestFinished(_request: FetchRequest) { - clearBusyState(this.element) + shouldVisit(_frameVisit: FrameVisit) { + return this.enabled && this.isActive } - // Form submission delegate - - formSubmissionStarted({ formElement }: FormSubmission) { - markAsBusy(formElement, this.findFrameElement(formElement)) + visitStarted(frameVisit: FrameVisit) { + this.ignoringChangesToAttribute("complete", () => { + this.frameVisit?.stop() + this.frameVisit = frameVisit + this.element.removeAttribute("complete") + }) } - formSubmissionSucceededWithResponse(formSubmission: FormSubmission, response: FetchResponse) { - const frame = this.findFrameElement(formSubmission.formElement, formSubmission.submitter) - - frame.delegate.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter) - - frame.delegate.loadResponse(response) + async visitSucceededWithResponse(frameVisit: FrameVisit, response: FetchResponse) { + await this.loadResponse(response, frameVisit) } - formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) { - this.element.delegate.loadResponse(fetchResponse) + async visitFailedWithResponse(frameVisit: FrameVisit, response: FetchResponse) { + await this.loadResponse(response, frameVisit) } - formSubmissionErrored(formSubmission: FormSubmission, error: Error) { + visitErrored(frameVisit: FrameVisit, request: FetchRequest, error: Error) { console.error(error) + dispatch("turbo:fetch-request-error", { + target: this.element, + detail: { request, error }, + }) } - formSubmissionFinished({ formElement }: FormSubmission) { - clearBusyState(formElement, this.findFrameElement(formElement)) + visitCompleted(_frameVisit: FrameVisit) { + this.hasBeenLoaded = true } // View delegate @@ -349,64 +295,32 @@ export class FrameController // Private - private async visit(url: URL) { - const request = new FetchRequest(this, FetchMethod.get, url, new URLSearchParams(), this.element) - - this.currentFetchRequest?.cancel() - this.currentFetchRequest = request - - return new Promise((resolve) => { - this.resolveVisitPromise = () => { - this.resolveVisitPromise = () => {} - this.currentFetchRequest = null - resolve() + private navigateFrame(element: Element, url: string) { + const frame = this.findFrameElement(element) + frame.delegate.visit(FrameVisit.optionsForClick(element, expandURL(url))) + } + + private proposeVisitIfNavigatedWithAction({ action, element, snapshot }: FrameVisit, fetchResponse: FetchResponse) { + if (element.src && action) { + const { statusCode, redirected } = fetchResponse + const responseHTML = element.ownerDocument.documentElement.outerHTML + const options: Partial = { + action, + snapshot, + response: { statusCode, redirected, responseHTML }, + restorationIdentifier: this.restorationIdentifier, + updateHistory: false, + visitCachedSnapshot: this.visitCachedSnapshot, + willRender: false, } - request.perform() - }) - } - - private navigateFrame(element: Element, url: string, submitter?: HTMLElement) { - const frame = this.findFrameElement(element, submitter) - - frame.delegate.proposeVisitIfNavigatedWithAction(frame, element, submitter) - this.withCurrentNavigationElement(element, () => { - frame.src = url - }) - } - - proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) { - this.action = getVisitAction(submitter, element, frame) - - if (this.action) { - const pageSnapshot = PageSnapshot.fromElement(frame).clone() - const { visitCachedSnapshot } = frame.delegate - - frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => { - if (frame.src) { - const { statusCode, redirected } = fetchResponse - const responseHTML = frame.ownerDocument.documentElement.outerHTML - const response = { statusCode, redirected, responseHTML } - const options: Partial = { - response, - visitCachedSnapshot, - willRender: false, - updateHistory: false, - restorationIdentifier: this.restorationIdentifier, - snapshot: pageSnapshot, - } - - if (this.action) options.action = this.action - - session.visit(frame.src, options) - } - } + session.visit(element.src, options) } } - changeHistory() { - if (this.action) { - const method = getHistoryMethodForAction(this.action) + changeHistory(action: Action | null) { + if (action) { + const method = getHistoryMethodForAction(action) session.history.update(method, expandURL(this.element.src || ""), this.restorationIdentifier) } } @@ -530,7 +444,7 @@ export class FrameController } get isLoading() { - return this.formSubmission !== undefined || this.resolveVisitPromise() !== undefined + return this.frameVisit !== undefined } get complete() { @@ -566,12 +480,6 @@ export class FrameController callback() this.ignoredAttributes.delete(attributeName) } - - private withCurrentNavigationElement(element: Element, callback: () => void) { - this.currentNavigationElement = element - callback() - delete this.currentNavigationElement - } } function getFrameElementById(id: string | null) { diff --git a/src/core/frames/frame_visit.ts b/src/core/frames/frame_visit.ts new file mode 100644 index 000000000..f0019109c --- /dev/null +++ b/src/core/frames/frame_visit.ts @@ -0,0 +1,183 @@ +import { Locatable, expandURL } from "../url" +import { Action } from "../types" +import { clearBusyState, getVisitAction, markAsBusy } from "../../util" +import { FrameElement } from "../../elements/frame_element" +import { FetchRequest, FetchRequestDelegate, FetchMethod } from "../../http/fetch_request" +import { FetchResponse } from "../../http/fetch_response" +import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission" +import { PageSnapshot } from "../drive/page_snapshot" +import { StreamMessage } from "../streams/stream_message" + +type Options = { + action: Action | null + acceptsStreamResponse: boolean + submit: { form: HTMLFormElement; submitter?: HTMLElement } + url: Locatable +} +type ClickFrameVisitOptions = Partial & { url: Options["url"] } +type SubmitFrameVisitOptions = Partial & { submit: Options["submit"] } + +export type FrameVisitOptions = ClickFrameVisitOptions | SubmitFrameVisitOptions + +export interface FrameVisitDelegate { + shouldVisit(frameVisit: FrameVisit): boolean + visitStarted(frameVisit: FrameVisit): void + visitSucceededWithResponse(frameVisit: FrameVisit, response: FetchResponse): void + visitFailedWithResponse(frameVisit: FrameVisit, response: FetchResponse): void + visitErrored(frameVisit: FrameVisit, request: FetchRequest, error: Error): void + visitCompleted(frameVisit: FrameVisit): void +} + +export class FrameVisit implements FetchRequestDelegate, FormSubmissionDelegate { + readonly delegate: FrameVisitDelegate + readonly element: FrameElement + readonly action: Action | null + readonly previousURL: string | null + readonly options: FrameVisitOptions + readonly isFormSubmission: boolean = false + readonly acceptsStreamResponse: boolean + snapshot?: PageSnapshot + + private readonly fetchRequest?: FetchRequest + private readonly formSubmission?: FormSubmission + private resolveVisitPromise = () => {} + + static optionsForClick(element: Element, url: URL): ClickFrameVisitOptions { + const action = getVisitAction(element) + const acceptsStreamResponse = element.hasAttribute("data-turbo-stream") + + return { acceptsStreamResponse, action, url } + } + + static optionsForSubmit(form: HTMLFormElement, submitter?: HTMLElement): SubmitFrameVisitOptions { + const action = getVisitAction(form, submitter) + + return { action, submit: { form, submitter } } + } + + constructor(delegate: FrameVisitDelegate, element: FrameElement, options: FrameVisitOptions) { + this.delegate = delegate + this.element = element + this.previousURL = this.element.src + + const { acceptsStreamResponse, action, url, submit } = (this.options = options) + + this.acceptsStreamResponse = acceptsStreamResponse || false + this.action = action || getVisitAction(this.element) + + if (submit) { + const { fetchRequest } = (this.formSubmission = new FormSubmission(this, submit.form, submit.submitter)) + this.prepareRequest(fetchRequest) + this.isFormSubmission = true + } else if (url) { + this.fetchRequest = new FetchRequest(this, FetchMethod.get, expandURL(url), new URLSearchParams(), this.element) + } else { + throw new Error("FrameVisit must be constructed with either a url: or submit: option") + } + } + + async start(): Promise { + if (this.delegate.shouldVisit(this)) { + if (this.action) { + this.snapshot = PageSnapshot.fromElement(this.element).clone() + } + + if (this.formSubmission) { + await this.formSubmission.start() + } else { + await this.performRequest() + } + + return this.element.loaded + } else { + return Promise.resolve() + } + } + + stop() { + this.fetchRequest?.cancel() + this.formSubmission?.stop() + } + + // Fetch request delegate + + prepareRequest(request: FetchRequest) { + request.headers["Turbo-Frame"] = this.element.id + + if (this.acceptsStreamResponse || this.isFormSubmission) { + request.acceptResponseType(StreamMessage.contentType) + } + } + + requestStarted(request: FetchRequest) { + this.delegate.visitStarted(this) + + if (request.target instanceof HTMLFormElement) { + markAsBusy(request.target) + } + + markAsBusy(this.element) + } + + requestPreventedHandlingResponse(_request: FetchRequest, _response: FetchResponse) { + this.resolveVisitPromise() + } + + requestFinished(request: FetchRequest) { + clearBusyState(this.element) + + if (request.target instanceof HTMLFormElement) { + clearBusyState(request.target) + } + + this.delegate.visitCompleted(this) + } + + async requestSucceededWithResponse(fetchRequest: FetchRequest, fetchResponse: FetchResponse) { + await this.delegate.visitSucceededWithResponse(this, fetchResponse) + this.resolveVisitPromise() + } + + async requestFailedWithResponse(request: FetchRequest, fetchResponse: FetchResponse) { + console.error(fetchResponse) + await this.delegate.visitFailedWithResponse(this, fetchResponse) + this.resolveVisitPromise() + } + + requestErrored(request: FetchRequest, error: Error) { + this.delegate.visitErrored(this, request, error) + this.resolveVisitPromise() + } + + // Form submission delegate + + formSubmissionStarted({ fetchRequest }: FormSubmission) { + this.requestStarted(fetchRequest) + } + + async formSubmissionSucceededWithResponse({ fetchRequest }: FormSubmission, response: FetchResponse) { + await this.requestSucceededWithResponse(fetchRequest, response) + } + + async formSubmissionFailedWithResponse({ fetchRequest }: FormSubmission, fetchResponse: FetchResponse) { + await this.requestFailedWithResponse(fetchRequest, fetchResponse) + } + + formSubmissionErrored({ fetchRequest }: FormSubmission, error: Error) { + this.requestErrored(fetchRequest, error) + } + + formSubmissionFinished({ fetchRequest }: FormSubmission) { + this.requestFinished(fetchRequest) + } + + private performRequest() { + this.element.loaded = new Promise((resolve) => { + this.resolveVisitPromise = () => { + this.resolveVisitPromise = () => {} + resolve() + } + this.fetchRequest?.perform() + }) + } +} diff --git a/src/core/session.ts b/src/core/session.ts index d988ef3bb..5e35cfeab 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -113,8 +113,7 @@ export class Session const frameElement = options.frame ? document.getElementById(options.frame) : null if (frameElement instanceof FrameElement) { - frameElement.src = location.toString() - frameElement.loaded + frameElement.delegate.visit({ url: location.toString(), action: options.action }) } else { this.navigator.proposeVisit(expandURL(location), options) } diff --git a/src/elements/frame_element.ts b/src/elements/frame_element.ts index 8a177a6d7..9c973f70c 100644 --- a/src/elements/frame_element.ts +++ b/src/elements/frame_element.ts @@ -1,5 +1,4 @@ -import { FetchResponse } from "../http/fetch_response" -import { Snapshot } from "../core/snapshot" +import { FrameVisitOptions } from "../core/frames/frame_visit" import { LinkInterceptorDelegate } from "../core/frames/link_interceptor" import { FormSubmitObserverDelegate } from "../observers/form_submit_observer" @@ -13,15 +12,12 @@ export type FrameElementObservedAttribute = keyof FrameElement & ("disabled" | " export interface FrameElementDelegate extends LinkInterceptorDelegate, FormSubmitObserverDelegate { connect(): void disconnect(): void + visit(options: Partial): Promise completeChanged(): void loadingStyleChanged(): void sourceURLChanged(): void sourceURLReloaded(): Promise disabledChanged(): void - loadResponse(response: FetchResponse): void - proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement): void - fetchResponseLoaded: (fetchResponse: FetchResponse) => void - visitCachedSnapshot: (snapshot: Snapshot) => void isLoading: boolean } diff --git a/src/tests/fixtures/tabs/three.html b/src/tests/fixtures/tabs/three.html index cc7804ef8..9fe63f430 100644 --- a/src/tests/fixtures/tabs/three.html +++ b/src/tests/fixtures/tabs/three.html @@ -1,9 +1,22 @@ - -
- Tab 1 - Tab 2 - Tab 3 -
+ + + + + Tabs: Three + + + + +

Tabs: Three

-
Three
- + +
+ Tab 1 + Tab 2 + Tab 3 +
+ +
Three
+ + + diff --git a/src/tests/fixtures/tabs/two.html b/src/tests/fixtures/tabs/two.html index 80d46f66c..bd7641863 100644 --- a/src/tests/fixtures/tabs/two.html +++ b/src/tests/fixtures/tabs/two.html @@ -1,9 +1,22 @@ - -
- Tab 1 - Tab 2 - Tab 3 -
+ + + + + Tabs: Two + + + + +

Tabs: Two

-
Two
- + +
+ Tab 1 + Tab 2 + Tab 3 +
+ +
Two
+ + + diff --git a/src/tests/functional/frame_navigation_tests.ts b/src/tests/functional/frame_navigation_tests.ts index 99fe6c388..ba3a57065 100644 --- a/src/tests/functional/frame_navigation_tests.ts +++ b/src/tests/functional/frame_navigation_tests.ts @@ -1,5 +1,13 @@ -import { test } from "@playwright/test" -import { getFromLocalStorage, nextEventNamed, nextEventOnTarget, pathname, scrollToSelector } from "../helpers/page" +import { Page, test } from "@playwright/test" +import { + getFromLocalStorage, + nextBeat, + nextEventNamed, + nextEventOnTarget, + pathname, + scrollToSelector, + sleep, +} from "../helpers/page" import { assert } from "chai" test("test frame navigation with descendant link", async ({ page }) => { @@ -37,6 +45,90 @@ test("test frame navigation emits fetch-request-error event when offline", async await nextEventOnTarget(page, "tab-frame", "turbo:fetch-request-error") }) +test("test promoted frame submits a single request per navigation", async ({ page }) => { + await page.goto("/src/tests/fixtures/tabs.html") + await nextEventNamed(page, "turbo:load") + + const requestedPathnames = await capturingRequestPathnames(page, async () => { + await page.click("#tab-2") + await nextEventNamed(page, "turbo:load") + await page.click("#tab-3") + await nextEventNamed(page, "turbo:load") + }) + + assert.deepEqual(requestedPathnames, ["/src/tests/fixtures/tabs/two.html", "/src/tests/fixtures/tabs/three.html"]) +}) + +test("test promoted frames do not submit requests when navigating back and forward with history", async ({ page }) => { + await page.goto("/src/tests/fixtures/tabs.html") + await nextEventNamed(page, "turbo:load") + await page.click("#tab-2") + await nextEventNamed(page, "turbo:load") + await page.click("#tab-3") + await nextEventNamed(page, "turbo:load") + + const requestedPathnames = await capturingRequestPathnames(page, async () => { + await page.goBack() + await nextEventNamed(page, "turbo:load") + await page.goForward() + await nextEventNamed(page, "turbo:load") + }) + + assert.deepEqual(requestedPathnames, []) +}) + +test("test navigating back when frame navigation has been canceled does not submit a request", async ({ page }) => { + await page.goto("/src/tests/fixtures/tabs/three.html") + await nextEventNamed(page, "turbo:load") + await delayResponseForLink(page, "#tab-2", 2000) + page.click("#tab-2") + await page.click("#tab-1") + await nextEventNamed(page, "turbo:load") + + assert.equal("/src/tests/fixtures/tabs.html", pathname(page.url())) + + const requestedPathnames = await capturingRequestPathnames(page, async () => { + await page.goBack() + await nextEventNamed(page, "turbo:load") + }) + + assert.deepEqual([], requestedPathnames) +}) + +test("test canceling frame requests don't mutate the history", async ({ page }) => { + await page.goto("/src/tests/fixtures/tabs.html") + await page.click("#tab-2") + await nextEventOnTarget(page, "tab-frame", "turbo:frame-load") + await nextEventNamed(page, "turbo:load") + + assert.equal(await page.textContent("#tab-content"), "Two") + assert.equal(pathname((await page.getAttribute("#tab-frame", "src")) || ""), "/src/tests/fixtures/tabs/two.html") + assert.equal(await page.getAttribute("#tab-frame", "complete"), "", "sets [complete]") + + // This request will be canceled + await delayResponseForLink(page, "#tab-1", 2000) + page.click("#tab-1") + await page.click("#tab-3") + + await nextEventOnTarget(page, "tab-frame", "turbo:frame-load") + await nextEventNamed(page, "turbo:load") + + assert.equal(await page.textContent("#tab-content"), "Three") + assert.equal(pathname((await page.getAttribute("#tab-frame", "src")) || ""), "/src/tests/fixtures/tabs/three.html") + + await page.goBack() + await nextEventNamed(page, "turbo:load") + + assert.equal(await page.textContent("#tab-content"), "Two") + assert.equal(pathname((await page.getAttribute("#tab-frame", "src")) || ""), "/src/tests/fixtures/tabs/two.html") + + // Make sure the frame is not mutated after some time. + await nextBeat() + + assert.equal(await page.textContent("#tab-content"), "Two") + assert.equal(pathname((await page.getAttribute("#tab-frame", "src")) || ""), "/src/tests/fixtures/tabs/two.html") +}) + test("test lazy-loaded frame promotes navigation", async ({ page }) => { await page.goto("/src/tests/fixtures/frame_navigation.html") @@ -104,3 +196,24 @@ test("test promoted frame navigations are cached", async ({ page }) => { assert.equal(await page.getAttribute("#tab-frame", "src"), null, "caches one.html without #tab-frame[src]") assert.equal(await page.getAttribute("#tab-frame", "complete"), null, "caches one.html without [complete]") }) + +async function capturingRequestPathnames(page: Page, callback: () => void) { + const requestedPathnames: string[] = [] + + page.on("request", (request) => requestedPathnames.push(pathname(request.url()))) + + await callback() + + return requestedPathnames +} + +async function delayResponseForLink(page: Page, selector: string, delayInMilliseconds: number) { + const href = await page.locator(selector).evaluate((link) => (link as HTMLAnchorElement).href) + + await page.route(href, async (route) => { + await sleep(delayInMilliseconds) + route.continue() + }) + + return page +} diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts index 4242d1f23..14c5df874 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -35,19 +35,31 @@ test.beforeEach(async ({ page }) => { }) test("test navigating a frame with Turbo.visit", async ({ page }) => { - const pathname = "/src/tests/fixtures/frames/frame.html" + const path = "/src/tests/fixtures/frames/frame.html" await page.locator("#frame").evaluate((frame) => frame.setAttribute("disabled", "")) - await page.evaluate((pathname) => window.Turbo.visit(pathname, { frame: "frame" }), pathname) + await page.evaluate((path) => window.Turbo.visit(path, { frame: "frame" }), path) await nextBeat() assert.equal(await page.textContent("#frame h2"), "Frames: #frame", "does not navigate a disabled frame") await page.locator("#frame").evaluate((frame) => frame.removeAttribute("disabled")) - await page.evaluate((pathname) => window.Turbo.visit(pathname, { frame: "frame" }), pathname) - await nextBeat() + await page.evaluate((path) => window.Turbo.visit(path, { frame: "frame" }), path) + await nextEventOnTarget(page, "frame", "turbo:frame-load") + + assert.equal(await page.textContent("#frame h2"), "Frame: loaded", "navigates the target frame") + assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html") + assert.equal(pathname((await attributeForSelector(page, "#frame", "src")) || ""), path) +}) + +test("test navigating a frame with Turbo.visit and an action: option", async ({ page }) => { + const path = "/src/tests/fixtures/frames/frame.html" + await page.evaluate((path) => window.Turbo.visit(path, { frame: "frame", action: "advance" }), path) + await nextEventOnTarget(page, "frame", "turbo:frame-load") assert.equal(await page.textContent("#frame h2"), "Frame: loaded", "navigates the target frame") + assert.equal(pathname(page.url()), path) + assert.equal(pathname((await attributeForSelector(page, "#frame", "src")) || ""), path) }) test("test navigating a frame a second time does not leak event listeners", async ({ page }) => {