Expose Frame load state via `[complete]` attribute #487

seanpdoyle · 2021-12-01T17:08:52Z

Closes #429

Introduce the <turbo-frame complete> 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 [complete] attribute if the element isConnected to the
document, so that the [complete] attribute is not modified prior to
Snapshot Caching or when re-mounting a Cached Snapshot.

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

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

how the internal currentURL value compares to the external
sourceURL value
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 "complete" 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.

Follow-up to [hotwired#441][] Depends on [hotwired#487][] Closes hotwired#472 --- When caching Snapshots during a `Visit`, elements are not cached until after the `turbo:before-cache` event fires. This affords client applications with an opportunity to disconnect and deconstruct and JavaScript state, and provides an opportunity to encode that state into the HTML so that it can survive the caching process. The timing of the construction of the `SnapshotSubstitution` instance occurs too early in the frame rendering process: the `<turbo-frame>` descendants have not been disconnected. The handling of the `<turbo-frame>` caching is already an exception from the norm. Unfortunately, the current implementation is caching _too early_ in the process. If the Snapshot were cached too late in the process with the rest of the page (as described in [hotwired#441][]), the `[src]` attribute and descendant content would have already changed, so any previous state would be lost. This commit strikes a balance between the two extremes by introducing the `FrameRendererDelegate` interface and the `frameContentsExtracted()` hook. During `<turbo-frame>` rendering, the `FrameRenderer` instance selects a [Range][] of nodes and removes them by calling [Range.deleteContents][]. The `deleteContents()` method removes the Nodes and discards them. This commit replaces the `deleteContents()` call with one to [Range.extractContents][], so that the Nodes are retained as a [DocumentFragment][] instance. While handling the callback, the `FrameController` retains that instance by setting an internal `previousContents` property. Later on in the Frame rendering-to-Visit-promotion process, the `FrameController` implements the `visitCachedSnapshot()` hook to read from the `previousContents` property and substitute the frame's contents with the `previousContents`, replacing the need for the `SnapshotSubstitution` class. [hotwired#441]: hotwired#441 [hotwired#487]: hotwired#487 [Range]: https://developer.mozilla.org/en-US/docs/Web/API/Range [Range.deleteContents]: https://developer.mozilla.org/en-US/docs/Web/API/range/deleteContents [Range.extractContents]: https://developer.mozilla.org/en-US/docs/Web/API/Range/extractContents [DocumentFragment]: https://developer.mozilla.org/en-US/docs/Web/API/DocumentFragment

seanpdoyle · 2021-12-01T23:48:03Z

It's worth highlighting that a [loaded] element attribute collides with the FrameElement.loaded property that exposes the underlying Promise instance used during loading.

This is probably a bad collision for a Boolean attribute, since myFrame.loaded will never evaluate to false, even when the <turbo-frame> element is missing the attribute and when the frame isn't loaded (since the default value for loaded is Promise.resolve().

We have two options:

Rename the FrameElement.loaded property and replace it with a .loaded boolean reader that wraps the [loaded] attribute
Rename the [loaded] attribute and introduce (or omit) a FrameElement.thatProperty reader and writer.

Options 1. involves breaking changes.

Option 2. is viable, but [loaded] is a really good attribute name! The front-runner competitor is probably [complete], since it'd correspond to the FrameElement.complete property, which itself draws inspiration from the HTMLImageElement.complete property.

tobyzerner · 2021-12-03T22:49:45Z

Hi @seanpdoyle, thanks for this PR. I can confirm that this does fix the issue I was having in #429.

I agree that [loaded] would be the best name for the attribute. I always found the FrameElement.loaded property referring to a Promise a bit counter-intuitive anyway – I would've expected it to return a boolean value before reading the docs. However if it's desirable to maintain BC in this case, that would be very understandable, and [complete] is a good runner-up.

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 `FrameElement` that's returned from the `FrameElement.observedAttributes` static property, including the `"src"` 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

dhh · 2022-06-19T15:21:00Z

I think [complete], and its parity to a similar attribute on image, is excellent. In some ways even preferable. Let's roll with that!

Closes hotwired#429 --- Introduce the `<turbo-frame complete>` 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 `[complete]` 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 `[complete]` attribute if the element [isConnected][] to the document, so that the `[complete]` attribute is not modified prior to Snapshot Caching or when re-mounting a Cached Snapshot. The act of "reloading" involves the removal of the `[complete]` attribute, which can be done either by `FrameElement.reload()` or `document.getElementById("frame-element").removeAttribute("complete")`. A side-effect of introducing the `[complete]` 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 `"complete"` 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

Explain the `[complete]` attribute's behavior introduced in [hotwired/turbo#487][]. [hotwired/turbo#487]: hotwired/turbo#487

* main: Drive Browser tests with `playwright` (hotwired#609) Allow Turbo Streams w/ GET via `data-turbo-stream` (hotwired#612) Only update history when Turbo visit is renderable (hotwired#601) Support development ChromeDriver version overrides (hotwired#606) Turbo stream source (hotwired#415) Expose Frame load state via `[complete]` attribute (hotwired#487)

* main: Allow frames to scroll smoothly into view (hotwired#607) Export Type declarations for `turbo:` events (hotwired#452) Add .php as a valid isHTML extension (hotwired#629) Add original click event to 'turbo:click' details (hotwired#611) Drive Browser tests with `playwright` (hotwired#609) Allow Turbo Streams w/ GET via `data-turbo-stream` (hotwired#612) Only update history when Turbo visit is renderable (hotwired#601) Support development ChromeDriver version overrides (hotwired#606) Turbo stream source (hotwired#415) Expose Frame load state via `[complete]` attribute (hotwired#487) fix(ie/edge): form.method='delete', raises Invalid argument. (hotwired#586) Do not declare global types/constants (hotwired#524) Defensively create custom turbo elements (hotwired#483) Use `replaceChildren` in StreamActions.update (hotwired#534)

Follow-up to [hotwired#441][] Depends on [hotwired#487][] Closes hotwired#472 --- When caching Snapshots during a `Visit`, elements are not cached until after the `turbo:before-cache` event fires. This affords client applications with an opportunity to disconnect and deconstruct and JavaScript state, and provides an opportunity to encode that state into the HTML so that it can survive the caching process. The timing of the construction of the `SnapshotSubstitution` instance occurs too early in the frame rendering process: the `<turbo-frame>` descendants have not been disconnected. The handling of the `<turbo-frame>` caching is already an exception from the norm. Unfortunately, the current implementation is caching _too early_ in the process. If the Snapshot were cached too late in the process with the rest of the page (as described in [hotwired#441][]), the `[src]` attribute and descendant content would have already changed, so any previous state would be lost. This commit strikes a balance between the two extremes by introducing the `FrameRendererDelegate` interface and the `frameContentsExtracted()` hook. During `<turbo-frame>` rendering, the `FrameRenderer` instance selects a [Range][] of nodes and removes them by calling [Range.deleteContents][]. The `deleteContents()` method removes the Nodes and discards them. This commit replaces the `deleteContents()` call with one to [Range.extractContents][], so that the Nodes are retained as a [DocumentFragment][] instance. While handling the callback, the `FrameController` retains that instance by setting an internal `previousContents` property. Later on in the Frame rendering-to-Visit-promotion process, the `FrameController` implements the `visitCachedSnapshot()` hook to read from the `previousContents` property and substitute the frame's contents with the `previousContents`, replacing the need for the `SnapshotSubstitution` class. [hotwired#441]: hotwired#441 [hotwired#487]: hotwired#487 [Range]: https://developer.mozilla.org/en-US/docs/Web/API/Range [Range.deleteContents]: https://developer.mozilla.org/en-US/docs/Web/API/range/deleteContents [Range.extractContents]: https://developer.mozilla.org/en-US/docs/Web/API/Range/extractContents [DocumentFragment]: https://developer.mozilla.org/en-US/docs/Web/API/DocumentFragment

Follow-up to [#441][] Depends on [#487][] Closes #472 --- When caching Snapshots during a `Visit`, elements are not cached until after the `turbo:before-cache` event fires. This affords client applications with an opportunity to disconnect and deconstruct and JavaScript state, and provides an opportunity to encode that state into the HTML so that it can survive the caching process. The timing of the construction of the `SnapshotSubstitution` instance occurs too early in the frame rendering process: the `<turbo-frame>` descendants have not been disconnected. The handling of the `<turbo-frame>` caching is already an exception from the norm. Unfortunately, the current implementation is caching _too early_ in the process. If the Snapshot were cached too late in the process with the rest of the page (as described in [#441][]), the `[src]` attribute and descendant content would have already changed, so any previous state would be lost. This commit strikes a balance between the two extremes by introducing the `FrameRendererDelegate` interface and the `frameContentsExtracted()` hook. During `<turbo-frame>` rendering, the `FrameRenderer` instance selects a [Range][] of nodes and removes them by calling [Range.deleteContents][]. The `deleteContents()` method removes the Nodes and discards them. This commit replaces the `deleteContents()` call with one to [Range.extractContents][], so that the Nodes are retained as a [DocumentFragment][] instance. While handling the callback, the `FrameController` retains that instance by setting an internal `previousContents` property. Later on in the Frame rendering-to-Visit-promotion process, the `FrameController` implements the `visitCachedSnapshot()` hook to read from the `previousContents` property and substitute the frame's contents with the `previousContents`, replacing the need for the `SnapshotSubstitution` class. [#441]: #441 [#487]: #487 [Range]: https://developer.mozilla.org/en-US/docs/Web/API/Range [Range.deleteContents]: https://developer.mozilla.org/en-US/docs/Web/API/range/deleteContents [Range.extractContents]: https://developer.mozilla.org/en-US/docs/Web/API/Range/extractContents [DocumentFragment]: https://developer.mozilla.org/en-US/docs/Web/API/DocumentFragment

When redirecting to a page that contains elements marked with `[data-turbo-cache="false"]`, those elements are removed _before_ the initial render, instead of _after_ the render and _before_ the page is cached. This behavior seems to have stemmed from [hotwired#516][], which was shipped in response to [hotwired#515][]. As an alternative to the `willRender: false` option passed to `this.adapter.visitProposedToLocation` in `Visit.followRedirect`, the implementation can instead [rely on the presence of the `turbo-frame[complete]`][comment] to guard against double fetching. To guard against regressions, this commit adds coverage for the unwanted behavior by redirecting from `navigation.html` to `cache_observer.html`, and asserting the presence of a `[data-turbo-cache="false"]` element that resembles and application's Flash messaging. [hotwired#515]: hotwired#515 [hotwired#516]: hotwired#516 [comment]: hotwired#515 (comment) [hotwired#487]: hotwired#487

When redirecting to a page that contains elements marked with `[data-turbo-cache="false"]`, those elements are removed _before_ the initial render, instead of _after_ the render and _before_ the page is cached. This behavior seems to have stemmed from [#516][], which was shipped in response to [#515][]. As an alternative to the `willRender: false` option passed to `this.adapter.visitProposedToLocation` in `Visit.followRedirect`, the implementation can instead [rely on the presence of the `turbo-frame[complete]`][comment] to guard against double fetching. To guard against regressions, this commit adds coverage for the unwanted behavior by redirecting from `navigation.html` to `cache_observer.html`, and asserting the presence of a `[data-turbo-cache="false"]` element that resembles and application's Flash messaging. [#515]: #515 [#516]: #516 [comment]: #515 (comment) [#487]: #487

seanpdoyle mentioned this pull request Dec 1, 2021

Support navigating a Frame to its current src #383

Closed

seanpdoyle force-pushed the frame-loaded-attribute branch from d7f79e4 to 735a6c6 Compare December 1, 2021 17:19

seanpdoyle mentioned this pull request Dec 1, 2021

Frame Visits: Cache Snapshot later in process #488

Merged

seanpdoyle force-pushed the frame-loaded-attribute branch 2 times, most recently from 4a4b481 to 9c26441 Compare December 1, 2021 21:19

seanpdoyle force-pushed the frame-loaded-attribute branch from 9c26441 to 31b6f29 Compare December 1, 2021 21:24

seanpdoyle force-pushed the frame-loaded-attribute branch from 31b6f29 to bccbe40 Compare June 19, 2022 14:16

seanpdoyle mentioned this pull request Jun 19, 2022

Double fetch requests for eager-loaded frames #515

Closed

seanpdoyle force-pushed the frame-loaded-attribute branch from 3313f1d to ffc66b7 Compare June 19, 2022 15:50

seanpdoyle force-pushed the frame-loaded-attribute branch from ffc66b7 to f80597c Compare June 19, 2022 15:53

seanpdoyle changed the title ~~Expose Frame load state via [loaded] attribute~~ Expose Frame load state via [complete] attribute Jun 19, 2022

dhh merged commit 4a61c68 into hotwired:main Jun 19, 2022

seanpdoyle deleted the frame-loaded-attribute branch June 19, 2022 16:09

seanpdoyle added a commit to seanpdoyle/turbo-site that referenced this pull request Jun 19, 2022

Document the turbo-frame[complete] attribute

590df63

Explain the `[complete]` attribute's behavior introduced in [hotwired/turbo#487][]. [hotwired/turbo#487]: hotwired/turbo#487

seanpdoyle mentioned this pull request Jun 19, 2022

Document the turbo-frame[complete] attribute hotwired/turbo-site#97

Merged

seanpdoyle added a commit to seanpdoyle/turbo-site that referenced this pull request Jun 19, 2022

Document the turbo-frame[complete] attribute

7f901bb

Explain the `[complete]` attribute's behavior introduced in [hotwired/turbo#487][]. [hotwired/turbo#487]: hotwired/turbo#487

manuelmeurer mentioned this pull request Sep 27, 2022

frame.reload() triggers turbo:before-fetch-request callback twice #739

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose Frame load state via `[complete]` attribute #487

Expose Frame load state via `[complete]` attribute #487

seanpdoyle commented Dec 1, 2021 •

edited by dhh

Loading

seanpdoyle commented Dec 1, 2021

tobyzerner commented Dec 3, 2021

dhh commented Jun 19, 2022 •

edited

Loading

Expose Frame load state via [complete] attribute #487

Expose Frame load state via [complete] attribute #487

Conversation

seanpdoyle commented Dec 1, 2021 • edited by dhh Loading

seanpdoyle commented Dec 1, 2021

tobyzerner commented Dec 3, 2021

dhh commented Jun 19, 2022 • edited Loading

Expose Frame load state via `[complete]` attribute #487

Expose Frame load state via `[complete]` attribute #487

seanpdoyle commented Dec 1, 2021 •

edited by dhh

Loading

dhh commented Jun 19, 2022 •

edited

Loading