Skip to content

Commit

Permalink
Expose Frame load state via [loaded] attribute
Browse files Browse the repository at this point in the history 
Closes hotwired#429

---

Introduce the `<turbo-frame loaded>` boolean attribute. The attribute's
absence indicates that the frame has not yet been loaded, and is ready
to be navigated. Its presence means that the contents of the frame have
been fetch from its `[src]` attribute.

Encoding the load state into the element's HTML aims to integrate with
Snapshot caching. Once a frame is loaded, navigating away and then
restoring a page's state from an Historical Snapshot should preserve the
fact that the contents are already loaded.

For both `eager` and `lazy` loaded frames, changing the element's
`[src]` attribute (directly via JavaScript, or by clicking an `<a>`
element or submitting a `<form>` element) will remove the `[loaded]`
attribute. Eager-loaded frames will immediately initiate a request to
fetch the contents, and Lazy-loaded frames will initiate the request
once they enter the viewport, or are changed to be eager-loading.

When the `[src]` attribute is changed, the `FrameController` will only
remove the `[loaded]` attribute if the element [isConnected][] to the
document, so that the `[loaded]` attribute is not modified prior to
Snapshot Caching or when re-mounting a Cached Snapshot.

The act of "reloading" involves the removal of the `[loaded]` attribute,
which can be done either by `FrameElement.reload()` or
`document.getElementById("frame-element").removeAttribute("loaded")`.

A side-effect of introducing the `[loaded]` attribute is that the
`FrameController` no longer needs to internally track:

1. how the internal `currentURL` value compares to the external
  `sourceURL` value
2. whether or not the frame is "reloadable"

By no longer tracking the `sourceURL` and `currentURL` separately, the
implementation for the private `loadSourceURL` method can be simplified.
Since there is no longer a `currentURL` property to rollback, the `try {
... } catch (error) { ... }` can be omitted, and the `this.sourceURL`
presence check can be incorporated into the rest of the guard
conditional.

Finally, this commit introduce the `isIgnoringChangesTo()` and
`ignoringChangesToAttribute()` private methods to disable
FrameController observations for a given period of time. For example,
when setting the `<turbo-frame src="...">` attribute, previous
implementation would set, then check the value of a
`this.settingSourceURL` property to decide whether or not to fire
attribute change callback code. This commit refines that pattern to
support any property of the `FrameController`, including the
`"sourceURL"` or `"loaded"` value. When making internal modifications to
those values, it's important to temporarily disable observation
callbacks to avoid unnecessary requests and to limit the potential for
infinitely recursing loops.

[isConnected]: https://developer.mozilla.org/en-US/docs/Web/API/Node/isConnected
  • Loading branch information
@seanpdoyle
seanpdoyle committed Dec 1, 2021
1 parent aa9724d commit 735a6c6
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 49 deletions.
97 changes: 53 additions & 44 deletions src/core/frames/frame_controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,13 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
readonly appearanceObserver: AppearanceObserver
readonly linkInterceptor: LinkInterceptor
readonly formInterceptor: FormInterceptor
currentURL?: string | null
formSubmission?: FormSubmission
fetchResponseLoaded = (fetchResponse: FetchResponse) => {}
private currentFetchRequest: FetchRequest | null = null
private resolveVisitPromise = () => {}
private connected = false
private hasBeenLoaded = false
private settingSourceURL = false
private ignoredAttributes: Set<keyof FrameController> = new Set

constructor(element: FrameElement) {
this.element = element
Expand All @@ -40,13 +39,13 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
connect() {
if (!this.connected) {
this.connected = true
this.reloadable = false
this.linkInterceptor.start()
this.formInterceptor.start()
if (this.loadingStyle == FrameLoadingStyle.lazy) {
this.appearanceObserver.start()
} else {
this.loadSourceURL()
}
this.linkInterceptor.start()
this.formInterceptor.start()
this.sourceURLChanged()
}
}

Expand All @@ -66,11 +65,23 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}

sourceURLChanged() {
if (this.isIgnoringChangesTo("sourceURL")) return

if (this.element.isConnected) {
this.ignoringChangesToAttribute("loaded", () => this.loaded = false)
}

if (this.loadingStyle == FrameLoadingStyle.eager || this.hasBeenLoaded) {
this.loadSourceURL()
}
}

loadedChanged() {
if (this.isIgnoringChangesTo("loaded")) return

this.loadSourceURL()
}

loadingStyleChanged() {
if (this.loadingStyle == FrameLoadingStyle.lazy) {
this.appearanceObserver.start()
Expand All @@ -80,21 +91,12 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}
}

async loadSourceURL() {
if (!this.settingSourceURL && this.enabled && this.isActive && (this.reloadable || this.sourceURL != this.currentURL)) {
const previousURL = this.currentURL
this.currentURL = this.sourceURL
if (this.sourceURL) {
try {
this.element.loaded = this.visit(expandURL(this.sourceURL))
this.appearanceObserver.stop()
await this.element.loaded
this.hasBeenLoaded = true
} catch (error) {
this.currentURL = previousURL
throw error
}
}
private async loadSourceURL() {
if (this.enabled && this.isActive && !this.loaded && this.sourceURL) {
this.element.loaded = this.visit(expandURL(this.sourceURL))
this.appearanceObserver.stop()
await this.element.loaded
this.hasBeenLoaded = true
}
}

Expand All @@ -111,6 +113,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
const renderer = new FrameRenderer(this.view.snapshot, snapshot, false, false)
if (this.view.renderPromise) await this.view.renderPromise
await this.view.render(renderer)
this.ignoringChangesToAttribute("loaded", () => this.loaded = true)
session.frameRendered(fetchResponse, this.element)
session.frameLoaded(this.element)
this.fetchResponseLoaded(fetchResponse)
Expand Down Expand Up @@ -140,7 +143,6 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}

linkClickIntercepted(element: Element, url: string) {
this.reloadable = true
this.navigateFrame(element, url)
}

Expand All @@ -155,7 +157,6 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
this.formSubmission.stop()
}

this.reloadable = false
this.formSubmission = new FormSubmission(this, element, submitter)
const { fetchRequest } = this.formSubmission
this.prepareHeadersForRequest(fetchRequest.headers, fetchRequest)
Expand Down Expand Up @@ -256,7 +257,6 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest

this.proposeVisitIfNavigatedWithAction(frame, element, submitter)

frame.setAttribute("reloadable", "")
frame.src = url
}

Expand Down Expand Up @@ -287,11 +287,11 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
const id = CSS.escape(this.id)

try {
if (element = activateElement(container.querySelector(`turbo-frame#${id}`), this.currentURL)) {
if (element = activateElement(container.querySelector(`turbo-frame#${id}`), this.sourceURL)) {
return element
}

if (element = activateElement(container.querySelector(`turbo-frame[src][recurse~=${id}]`), this.currentURL)) {
if (element = activateElement(container.querySelector(`turbo-frame[src][recurse~=${id}]`), this.sourceURL)) {
await element.loaded
return await this.extractForeignFrameElement(element)
}
Expand Down Expand Up @@ -355,25 +355,10 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}
}

get reloadable() {
const frame = this.findFrameElement(this.element)
return frame.hasAttribute("reloadable")
}

set reloadable(value: boolean) {
const frame = this.findFrameElement(this.element)
if (value) {
frame.setAttribute("reloadable", "")
} else {
frame.removeAttribute("reloadable")
}
}

set sourceURL(sourceURL: string | undefined) {
this.settingSourceURL = true
this.element.src = sourceURL ?? null
this.currentURL = this.element.src
this.settingSourceURL = false
this.ignoringChangesToAttribute("sourceURL", () => {
this.element.src = sourceURL ?? null
})
}

get loadingStyle() {
Expand All @@ -384,6 +369,20 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
return this.formSubmission !== undefined || this.resolveVisitPromise() !== undefined
}

get loaded() {
return this.element.hasAttribute("loaded")
}

set loaded(value: boolean) {
this.ignoringChangesToAttribute("loaded", () => {
if (value) {
this.element.setAttribute("loaded", "")
} else {
this.element.removeAttribute("loaded")
}
})
}

get isActive() {
return this.element.isActive && this.connected
}
Expand All @@ -393,6 +392,16 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
const root = meta?.content ?? "/"
return expandURL(root)
}

private isIgnoringChangesTo(attributeName: keyof FrameController): boolean {
return this.ignoredAttributes.has(attributeName)
}

private ignoringChangesToAttribute(attributeName: keyof FrameController, callback: () => void) {
this.ignoredAttributes.add(attributeName)
callback()
this.ignoredAttributes.delete(attributeName)
}
}

class SnapshotSubstitution {
Expand Down
1 change: 0 additions & 1 deletion src/core/frames/frame_redirector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptor
formSubmissionIntercepted(element: HTMLFormElement, submitter?: HTMLElement) {
const frame = this.findFrameElement(element, submitter)
if (frame) {
frame.removeAttribute("reloadable")
frame.delegate.formSubmissionIntercepted(element, submitter)
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/elements/frame_element.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export enum FrameLoadingStyle { eager = "eager", lazy = "lazy" }
export interface FrameElementDelegate {
connect(): void
disconnect(): void
loadedChanged(): void
loadingStyleChanged(): void
sourceURLChanged(): void
disabledChanged(): void
Expand Down Expand Up @@ -38,7 +39,7 @@ export class FrameElement extends HTMLElement {
readonly delegate: FrameElementDelegate

static get observedAttributes() {
return ["disabled", "loading", "src"]
return ["disabled", "loaded", "loading", "src"]
}

constructor() {
Expand All @@ -56,13 +57,16 @@ export class FrameElement extends HTMLElement {

reload() {
const { src } = this;
this.removeAttribute("loaded")
this.src = null;
this.src = src;
}

attributeChangedCallback(name: string) {
if (name == "loading") {
this.delegate.loadingStyleChanged()
} else if (name == "loaded") {
this.delegate.loadedChanged()
} else if (name == "src") {
this.delegate.sourceURLChanged()
} else {
Expand Down
4 changes: 3 additions & 1 deletion src/tests/fixtures/loading.html
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!DOCTYPE html>
<html>
<html data-skip-event-details="turbo:before-render">
<head>
<meta charset="utf-8">
<title>Turbo</title>
Expand All @@ -13,6 +13,8 @@
<turbo-frame id="hello" src="/src/tests/fixtures/frames/hello.html" loading="lazy"></turbo-frame>
</details>

<a id="link-lazy-frame" href="/src/tests/fixtures/frames.html" data-turbo-frame="hello">Navigate #loading-lazy turbo-frame</a>

<details id="loading-eager">
<summary>Eager-loaded</summary>

Expand Down
75 changes: 73 additions & 2 deletions src/tests/functional/loading_tests.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FrameElement } from "../../elements/frame_element"
import { TurboDriveTestCase } from "../helpers/turbo_drive_test_case"

declare global {
Expand All @@ -14,19 +15,22 @@ export class LoadingTests extends TurboDriveTestCase {
async "test eager loading within a details element"() {
await this.nextBeat
this.assert.ok(await this.hasSelector("#loading-eager turbo-frame#frame h2"))
this.assert.ok(await this.hasSelector("#loading-eager turbo-frame[loaded]"), "has [loaded] attribute")
}

async "test lazy loading within a details element"() {
await this.nextBeat

const frameContents = "#loading-lazy turbo-frame h2"
this.assert.notOk(await this.hasSelector(frameContents))
this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame:not([loaded])"))

await this.clickSelector("#loading-lazy summary")
await this.nextBeat

const contents = await this.querySelector(frameContents)
this.assert.equal(await contents.getVisibleText(), "Hello from a frame")
this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "has [loaded] attribute")
}

async "test changing loading attribute from lazy to eager loads frame"() {
Expand Down Expand Up @@ -92,9 +96,12 @@ export class LoadingTests extends TurboDriveTestCase {

const frameContent = "#loading-eager turbo-frame#frame h2"
this.assert.ok(await this.hasSelector(frameContent))
// @ts-ignore
await this.remote.execute(() => document.querySelector("#loading-eager turbo-frame")?.reload())
this.assert.ok(await this.hasSelector("#loading-eager turbo-frame[loaded]"), "has [loaded] attribute")

await this.remote.execute(() => document.querySelector<FrameElement>("#loading-eager turbo-frame")?.reload())

this.assert.ok(await this.hasSelector(frameContent))
this.assert.ok(await this.hasSelector("#loading-eager turbo-frame:not([loaded])"), "clears [loaded] attribute")
}

async "test navigating away from a page does not reload its frames"() {
Expand All @@ -106,6 +113,70 @@ export class LoadingTests extends TurboDriveTestCase {
this.assert.equal(requestLogs.length, 1)
}

async "test removing the [loaded] attribute of an eager frame reloads the content"() {
await this.nextEventOnTarget("frame", "turbo:frame-load")
await this.remote.execute(() => document.querySelector("#loading-eager turbo-frame")?.removeAttribute("loaded"))
await this.nextEventOnTarget("frame", "turbo:frame-load")

this.assert.ok(await this.hasSelector("#loading-eager turbo-frame[loaded]"), "sets the [loaded] attribute after re-loading")
}

async "test changing [src] attribute on a [loaded] frame with loading=lazy defers navigation"() {
await this.nextEventOnTarget("frame", "turbo:frame-load")
await this.clickSelector("#loading-lazy summary")
await this.nextEventOnTarget("hello", "turbo:frame-load")

this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "lazy frame is loaded")
this.assert.equal(await (await this.querySelector("#hello h2")).getVisibleText(), "Hello from a frame")

await this.clickSelector("#loading-lazy summary")
await this.clickSelector("#one")
await this.nextEventNamed("turbo:load")
await this.goBack()
await this.nextBody
await this.noNextEventNamed("turbo:frame-load")

let src = new URL(await this.attributeForSelector("#hello", "src") || "")

this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "lazy frame is loaded")
this.assert.equal(src.pathname, "/src/tests/fixtures/frames/hello.html", "lazy frame retains [src]")

await this.clickSelector("#link-lazy-frame")
await this.noNextEventNamed("turbo:frame-load")

this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame:not([loaded])"), "lazy frame is not loaded")

await this.clickSelector("#loading-lazy summary")
await this.nextEventOnTarget("hello", "turbo:frame-load")

src = new URL(await this.attributeForSelector("#hello", "src") || "")

this.assert.equal(await (await this.querySelector("#loading-lazy turbo-frame h2")).getVisibleText(), "Frames: #hello")
this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "lazy frame is loaded")
this.assert.equal(src.pathname, "/src/tests/fixtures/frames.html", "lazy frame navigates")
}

async "test navigating away from a page and then back does not reload its frames"() {
await this.clickSelector("#one")
await this.nextBody
await this.eventLogChannel.read()
await this.goBack()
await this.nextBody

const eventLogs = await this.eventLogChannel.read()
const requestLogs = eventLogs.filter(([ name ]) => name == "turbo:before-fetch-request")
const requestsOnEagerFrame = requestLogs.filter(record => record[2] == "frame")
const requestsOnLazyFrame = requestLogs.filter(record => record[2] == "hello")

this.assert.equal(requestsOnEagerFrame.length, 0, "does not reload eager frame")
this.assert.equal(requestsOnLazyFrame.length, 0, "does not reload lazy frame")

await this.clickSelector("#loading-lazy summary")
await this.nextEventOnTarget("hello", "turbo:before-fetch-request")
await this.nextEventOnTarget("hello", "turbo:frame-render")
await this.nextEventOnTarget("hello", "turbo:frame-load")
}

async "test disconnecting and reconnecting a frame does not reload the frame"() {
await this.nextBeat

Expand Down

0 comments on commit 735a6c6

Please sign in to comment.