diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 788f7661c..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,18 +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 - pageSnapshot?: PageSnapshot constructor(element: FrameElement) { this.element = element @@ -100,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() @@ -145,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 } @@ -174,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.` @@ -191,16 +175,12 @@ export class FrameController } catch (error) { console.error(error) this.view.invalidate() - } finally { - this.fetchResponseLoaded = () => {} } } // Appearance observer delegate - elementAppearedInViewport(element: FrameElement) { - this.pageSnapshot = PageSnapshot.fromElement(element).clone() - this.proposeVisitIfNavigatedWithAction(element, element) + elementAppearedInViewport(_element: FrameElement) { this.loadSourceURL() } @@ -232,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 @@ -351,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) - this.pageSnapshot = PageSnapshot.fromElement(frame).clone() - - 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 { 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: this.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) } } @@ -532,7 +444,7 @@ export class FrameController } get isLoading() { - return this.formSubmission !== undefined || this.resolveVisitPromise() !== undefined + return this.frameVisit !== undefined } get complete() { @@ -568,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..58d992738 --- /dev/null +++ b/src/core/frames/frame_visit.ts @@ -0,0 +1,181 @@ +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)) { + 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 ee94b5043..015ee34fa 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() }) } 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.html b/src/tests/fixtures/tabs.html index 702cea0a1..cf645d314 100644 --- a/src/tests/fixtures/tabs.html +++ b/src/tests/fixtures/tabs.html @@ -17,6 +17,12 @@

Tabs

One
+ +
+ Tab 1 + Tab 2 + Tab 3 +
diff --git a/src/tests/fixtures/tabs/three.html b/src/tests/fixtures/tabs/three.html index cc7804ef8..533d84dfd 100644 --- a/src/tests/fixtures/tabs/three.html +++ b/src/tests/fixtures/tabs/three.html @@ -6,4 +6,10 @@
Three
+ +
+ Tab 1 + Tab 2 + Tab 3 +
diff --git a/src/tests/fixtures/tabs/two.html b/src/tests/fixtures/tabs/two.html index 80d46f66c..eef4aa863 100644 --- a/src/tests/fixtures/tabs/two.html +++ b/src/tests/fixtures/tabs/two.html @@ -6,4 +6,10 @@
Two
+ +
+ Tab 1 + Tab 2 + Tab 3 +
diff --git a/src/tests/functional/frame_navigation_tests.ts b/src/tests/functional/frame_navigation_tests.ts index 3a2fadde1..f55bba461 100644 --- a/src/tests/functional/frame_navigation_tests.ts +++ b/src/tests/functional/frame_navigation_tests.ts @@ -30,6 +30,55 @@ 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 }) => { + const requestedPathnames: string[] = [] + await page.goto("/src/tests/fixtures/tabs.html") + await nextEventNamed(page, "turbo:load") + page.on("request", (request) => requestedPathnames.push(pathname(request.url()))) + 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 }) => { + const requestedPathnames: string[] = [] + 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") + page.on("requestfinished", (request) => requestedPathnames.push(pathname(request.url()))) + 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 }) => { + const requestedPathnames: string[] = [] + await page.goto("/src/tests/fixtures/tabs.html") + await nextEventNamed(page, "turbo:load") + await page.click("#tab-3") + await nextEventNamed(page, "turbo:load") + page.click("#slow-tab-2") + await page.click("#slow-tab-1") + await nextEventNamed(page, "turbo:load") + + assert.equal("/src/tests/fixtures/tabs.html", pathname(page.url())) + + page.on("requestfinished", (request) => requestedPathnames.push(pathname(request.url()))) + await page.goBack() + await nextEventNamed(page, "turbo:load") + + assert.deepEqual([], requestedPathnames) +}) + test("test lazy-loaded frame promotes navigation", async ({ page }) => { await page.goto("/src/tests/fixtures/frame_navigation.html") diff --git a/src/tests/server.ts b/src/tests/server.ts index a0b59cd0f..e53dae5f4 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -34,13 +34,13 @@ router.post("/redirect", (request, response) => { }) router.get("/redirect", (request, response) => { - const { path, ...query } = request.query as any + const { path, sleep, ...query } = request.query as any const pathname = path ?? "/src/tests/fixtures/one.html" const enctype = request.get("Content-Type") if (enctype) { query.enctype = enctype } - response.redirect(301, url.format({ pathname, query })) + setTimeout(() => response.redirect(301, url.format({ pathname, query })), parseInt(sleep || "0", 10)) }) router.post("/reject/tall", (request, response) => {