Skip to content

Commit

Permalink
Page refreshes (#1019)
Browse files Browse the repository at this point in the history
* Page refreshes

This commit introduces the concept of page refresh. A page refresh
happens when Turbo renders the current page again. We will offer two new
options to control behavior when a page refresh happens:

- The method used to update the page: with a new option to use morphing
  (Turbo currently replaces the body).

- The scroll strategy: with a new option to keep it (Turbo currently
  resets scroll to the top-left).

The combination of morphing and scroll-keeping results in smoother
updates that keep the screen state. For example, this will keep both
horizontal and vertical scroll, the focus, the text selection, CSS
transition states, etc.

We will also introduce a new turbo stream action that, when broadcasted,
will request a page refresh. This will offer a simplified alternative
to fine-grained broadcasted turbo-stream actions.

Co-Authored-By: Jorge Manrubia <jorge@hey.com>

* Introduce refresh attribute for frame element

* Use dispatch util function in MorphRenderer

* Don't morph frames flagged with "refresh=morph" as part of the full page refresh

We will refresh those frames with morphing as part of the page refresh, so it does
not make sense to morph them also as part of the full page refresh. If we do, we'll
trigger a manual reload because the complete attribute will get removed. This aligns
the upstreamed version with the private gem we've been using internally.

This also fixes a couple of issues:

- We don't want to manually reload all the remote turbo-frames, only those that are
flagged with "refresh=morph". Regular remote frames will get reloaded automatically
when removing their complete attribute during regular page refreshes.

- Using idiomorph's "innerHTML" was resulting in a turbo-frame nested inside the
target turbo-frame. I think its semantics is not morphing inner contents from both
currentElement and newElement, but morphing newElement as the inner contents of currentElement.

See #1019 (comment)

* Delegate `StreamActions.refresh` to `Session` (#1026)

The bulk of the `StreamActions.refresh` implementation was reaching
through the global `window.Turbo` property, which itself was reaching
through either global variables or the `Session`.

This commit moves the implementation out of the `StreamActions` and into
a new `Session.refresh(url, requestId)` method. With that change, all
property access is encapsulated within the `Session`.

To support that change, this commit also introduces the
`StreamElement.requestId` property to read the `[request-id]` attribute.

* Only morph the turbo-frame contents, not the frame itself

I had removed this in #d1935bd15c85e0a8776afccb90393fd378aea2d2 but we do
need innerHTML so that the outer frame don't get touched. The problem we had
is that we were nesting turbo-frames, so using .children to only address the
contents instead.

* Don't patch fetch globally, import patched function where needed instead.

We'll export a `fetch` function apps can import too. This can be necessary if an app,
for example, is doing a manual `fetch` and it wants to prevent a reflected broadcast
triggered by that request.

We'll also make rails/request use the Turbo fetch version when available. That will be
the preferred form. In general, we don't want apps to care about having to import or
use Turbo's `fetch`, but it will be available for the cases an app needs it.

We'll also expose the patched version via `Turbo.fetch`.

See discussion #1019 (review)

* Always morph remote turbo-frames when a page refresh happens

Instead of flagging the frames you want to morph with an special attribute,
we are always going to reload and refresh with morphing all the turbo-frames
in the page.

This simplifies the API as it removes the concern of categorizing remote
turbo-frames on the programmer side when using page-refreshes.

This is an idea by @afcapel, who raised the concern of the confusion the attribute
replace=morph caused, and questioned its necessity.

We can bring the old approach back if we find real cases that justify it.

* Remove Circular Build Dependency (#1049)

Resolves #1026 (comment)

* Don't refresh automatically turbo-frames that descend from a [data-turbo-permanent] element

Since we are now reloading all the remote frames in the page, we need to make sure we ignore
those that are contained in elements to be preserved.

* Don't add new [data-turbo-permanent] elements when they already exist in the page.

It can happen that a same element exists but that idiomorph won't match it
because some JS moved the element to another position. This will handle such
scenarios automatically.

* Don't reload stimulus controllers after morphing

There was a flaw in the implementation: we wanted to reload the stimulus controllers
when their element was effectively morphed because some attribute had changed. Our
implementation was essentially reloading all the stimulus controllers instead.

But, even if we implemented our original idea, we have changed our mind about it being
the right call. The heuristic of "reload controllers when some attribute changed" came
from some tests with legacy controllers that used dom attributes to track certain
conditions. That doesn't seem like enough justification for the original idea.

In general, you don't want to reload controllers unless their elements get disconnected
or connected as part of the morphing operation. If it's important for a given controller
to track changes to the dom, than it should do that (e.g: listening to connection of targets or
outlets, or just with the mutation observer API), but we can't determine that from the outside.

If we introduce some API here, it will be the opposite: an API to force a "reconnect" during
morphing, but we need to see a real justification in practice.

* Respect morphing and scroll preservation settings when handling form errors

Turbo will render 422 responses to allow handling form errors. A common scenario
in Rails is to render those setting the satus like:

```
render "edit", status: :unprocessable_entity
```

This change will consider such operations a "page refresh" and will also consider
the scroll directive.

* Morph remote turbo frames where the src attribute has changed

There are some cases when we don't want to reload a remote turbo frame on
a page refresh.

This may be because Turbo has added a src attribute to the turbo frame
element, but we don't want to reload the frame from that URL.

Form example, when a form inside a turbo frame is submitted, Turbo adds
a src attribute to the form element. In those cases we don't want to
reload the Turbo frame from the src URL. The src attribute points to the
form submission URL, which may not be loadable with a GET request.

Same thing can happen when a link inside a turbo frame is clicked. Turbo
adds a src attribute to the frame element, but we don't want to reload the
frame from that URL.

If the src attribute of a turbo frame changes, this signals that the server
wants to render something different that what's currently on the DOM, and
Turbo should respect that.

This also matches the progressive enhancement behavior philosophy of Turbo.
The behaviour results in the Turbo frame that the server sends, which
is what would happen anyway if there was no morphing involved, or on a first
page load.

---------

Co-authored-by: Jorge Manrubia <jorge@hey.com>
Co-authored-by: Sean Doyle <seanpdoyle@users.noreply.github.com>
Co-authored-by: Jorge Manrubia <jorge.manrubia@gmail.com>
  • Loading branch information
4 people committed Nov 14, 2023
1 parent 0d2ec4b commit a178184
Show file tree
Hide file tree
Showing 34 changed files with 692 additions and 18 deletions.
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
"publishConfig": {
"access": "public"
},
"dependencies": {
"idiomorph": "https://github.com/basecamp/idiomorph#rollout-build"
},
"devDependencies": {
"@open-wc/testing": "^3.1.7",
"@playwright/test": "^1.28.0",
Expand Down
15 changes: 15 additions & 0 deletions src/core/drive/limited_set.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export class LimitedSet extends Set {
constructor(maxSize) {
super()
this.maxSize = maxSize
}

add(value) {
if (this.size >= this.maxSize) {
const iterator = this.values()
const oldestValue = iterator.next().value
this.delete(oldestValue)
}
super.add(value)
}
}
97 changes: 97 additions & 0 deletions src/core/drive/morph_renderer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import Idiomorph from "idiomorph"
import { dispatch } from "../../util"
import { urlsAreEqual } from "../url"
import { Renderer } from "../renderer"

export class MorphRenderer extends Renderer {
async render() {
if (this.willRender) await this.#morphBody()
}

get renderMethod() {
return "morph"
}

// Private

async #morphBody() {
this.#morphElements(this.currentElement, this.newElement)
this.#reloadRemoteFrames()

dispatch("turbo:morph", {
detail: {
currentElement: this.currentElement,
newElement: this.newElement
}
})
}

#morphElements(currentElement, newElement, morphStyle = "outerHTML") {
this.isMorphingTurboFrame = this.#remoteFrameReplacement(currentElement, newElement)

Idiomorph.morph(currentElement, newElement, {
morphStyle: morphStyle,
callbacks: {
beforeNodeAdded: this.#shouldAddElement,
beforeNodeMorphed: this.#shouldMorphElement,
beforeNodeRemoved: this.#shouldRemoveElement
}
})
}

#shouldAddElement = (node) => {
return !(node.id && node.hasAttribute("data-turbo-permanent") && document.getElementById(node.id))
}

#shouldMorphElement = (oldNode, newNode) => {
if (!(oldNode instanceof HTMLElement) || this.isMorphingTurboFrame) {
return true
}
else if (oldNode.hasAttribute("data-turbo-permanent")) {
return false
} else {
return !this.#remoteFrameReplacement(oldNode, newNode)
}
}

#remoteFrameReplacement = (oldNode, newNode) => {
return this.#isRemoteFrame(oldNode) && this.#isRemoteFrame(newNode) && urlsAreEqual(oldNode.getAttribute("src"), newNode.getAttribute("src"))
}

#shouldRemoveElement = (node) => {
return this.#shouldMorphElement(node)
}

#reloadRemoteFrames() {
this.#remoteFrames().forEach((frame) => {
if (this.#isRemoteFrame(frame)) {
this.#renderFrameWithMorph(frame)
frame.reload()
}
})
}

#renderFrameWithMorph(frame) {
frame.addEventListener("turbo:before-frame-render", (event) => {
event.detail.render = this.#morphFrameUpdate
}, { once: true })
}

#morphFrameUpdate = (currentElement, newElement) => {
dispatch("turbo:before-frame-morph", {
target: currentElement,
detail: { currentElement, newElement }
})
this.#morphElements(currentElement, newElement.children, "innerHTML")
}

#isRemoteFrame(node) {
return node instanceof HTMLElement && node.nodeName.toLowerCase() === "turbo-frame" && node.getAttribute("src")
}

#remoteFrames() {
return Array.from(document.querySelectorAll('turbo-frame[src]')).filter(frame => {
return !frame.closest('[data-turbo-permanent]')
})
}
}
4 changes: 3 additions & 1 deletion src/core/drive/navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ export class Navigator {
} else {
await this.view.renderPage(snapshot, false, true, this.currentVisit)
}
this.view.scrollToTop()
if(!snapshot.shouldPreserveScrollPosition) {
this.view.scrollToTop()
}
this.view.clearSnapshotCache()
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/core/drive/page_snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ export class PageSnapshot extends Snapshot {
return this.headSnapshot.getMetaValue("view-transition") === "same-origin"
}

get shouldMorphPage() {
return this.getSetting("refresh-method") === "morph"
}

get shouldPreserveScrollPosition() {
return this.getSetting("refresh-scroll") === "preserve"
}

// Private

getSetting(name) {
Expand Down
10 changes: 9 additions & 1 deletion src/core/drive/page_view.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { nextEventLoopTick } from "../../util"
import { View } from "../view"
import { ErrorRenderer } from "./error_renderer"
import { MorphRenderer } from "./morph_renderer"
import { PageRenderer } from "./page_renderer"
import { PageSnapshot } from "./page_snapshot"
import { SnapshotCache } from "./snapshot_cache"
Expand All @@ -15,7 +16,10 @@ export class PageView extends View {
}

renderPage(snapshot, isPreview = false, willRender = true, visit) {
const renderer = new PageRenderer(this.snapshot, snapshot, PageRenderer.renderElement, isPreview, willRender)
const shouldMorphPage = this.isPageRefresh(visit) && this.snapshot.shouldMorphPage
const rendererClass = shouldMorphPage ? MorphRenderer : PageRenderer

const renderer = new rendererClass(this.snapshot, snapshot, PageRenderer.renderElement, isPreview, willRender)

if (!renderer.shouldRender) {
this.forceReloaded = true
Expand Down Expand Up @@ -51,6 +55,10 @@ export class PageView extends View {
return this.snapshotCache.get(location)
}

isPageRefresh(visit) {
return !visit || this.lastRenderedLocation.href === visit.location.href
}

get snapshot() {
return PageSnapshot.fromElement(this.element)
}
Expand Down
1 change: 1 addition & 0 deletions src/core/drive/preloader.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { PageSnapshot } from "./page_snapshot"
import { fetch } from "../../http/fetch"

export class Preloader {
selector = "a[data-turbo-preload]"
Expand Down
2 changes: 1 addition & 1 deletion src/core/drive/visit.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ export class Visit {
// Scrolling

performScroll() {
if (!this.scrolled && !this.view.forceReloaded) {
if (!this.scrolled && !this.view.forceReloaded && !this.view.snapshot.shouldPreserveScrollPosition) {
if (this.action == "restore") {
this.scrollToRestoredPosition() || this.scrollToAnchor() || this.view.scrollToTop()
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/core/frames/frame_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ export class FrameController {
return !defaultPrevented
}

viewRenderedSnapshot(_snapshot, _isPreview) {}
viewRenderedSnapshot(_snapshot, _isPreview, _renderMethod) {}

preloadOnLoadLinksForView(element) {
session.preloadOnLoadLinksForView(element)
Expand Down
5 changes: 2 additions & 3 deletions src/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ import { PageRenderer } from "./drive/page_renderer"
import { PageSnapshot } from "./drive/page_snapshot"
import { FrameRenderer } from "./frames/frame_renderer"
import { FormSubmission } from "./drive/form_submission"
import { fetch } from "../http/fetch"

const session = new Session()
const { cache, navigator } = session
export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer }

export { StreamActions } from "./streams/stream_actions"
export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch }

/**
* Starts the main session.
Expand Down
4 changes: 4 additions & 0 deletions src/core/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,8 @@ export class Renderer {
get permanentElementMap() {
return this.currentSnapshot.getPermanentElementMapForSnapshot(this.newSnapshot)
}

get renderMethod() {
return "replace"
}
}
18 changes: 14 additions & 4 deletions src/core/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { clearBusyState, dispatch, findClosestRecursively, getVisitAction, markA
import { PageView } from "./drive/page_view"
import { FrameElement } from "../elements/frame_element"
import { Preloader } from "./drive/preloader"
import { LimitedSet } from "./drive/limited_set"
import { Cache } from "./cache"

export class Session {
Expand All @@ -35,6 +36,7 @@ export class Session {
frameRedirector = new FrameRedirector(this, document.documentElement)
streamMessageRenderer = new StreamMessageRenderer()
cache = new Cache(this)
recentRequests = new LimitedSet(20)

drive = true
enabled = true
Expand Down Expand Up @@ -93,6 +95,14 @@ export class Session {
}
}

refresh(url, requestId) {
const isRecentRequest = requestId && this.recentRequests.has(requestId)
if (!isRecentRequest) {
this.cache.exemptPageFromPreview()
this.visit(url, { action: "replace" })
}
}

connectStreamSource(source) {
this.streamObserver.connectStreamSource(source)
}
Expand Down Expand Up @@ -265,9 +275,9 @@ export class Session {
return !defaultPrevented
}

viewRenderedSnapshot(_snapshot, isPreview) {
viewRenderedSnapshot(_snapshot, isPreview, renderMethod) {
this.view.lastRenderedLocation = this.history.location
this.notifyApplicationAfterRender(isPreview)
this.notifyApplicationAfterRender(isPreview, renderMethod)
}

preloadOnLoadLinksForView(element) {
Expand Down Expand Up @@ -330,8 +340,8 @@ export class Session {
})
}

notifyApplicationAfterRender(isPreview) {
return dispatch("turbo:render", { detail: { isPreview } })
notifyApplicationAfterRender(isPreview, renderMethod) {
return dispatch("turbo:render", { detail: { isPreview, renderMethod } })
}

notifyApplicationAfterPageLoad(timing = {}) {
Expand Down
6 changes: 6 additions & 0 deletions src/core/streams/stream_actions.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { session } from "../"

export const StreamActions = {
after() {
this.targetElements.forEach((e) => e.parentElement?.insertBefore(this.templateContent, e.nextSibling))
Expand Down Expand Up @@ -30,5 +32,9 @@ export const StreamActions = {
targetElement.innerHTML = ""
targetElement.append(this.templateContent)
})
},

refresh() {
session.refresh(this.baseURI, this.requestId)
}
}
2 changes: 1 addition & 1 deletion src/core/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class View {
if (!immediateRender) await renderInterception

await this.renderSnapshot(renderer)
this.delegate.viewRenderedSnapshot(snapshot, isPreview)
this.delegate.viewRenderedSnapshot(snapshot, isPreview, this.renderer.renderMethod)
this.delegate.preloadOnLoadLinksForView(this.element)
this.finishRenderingSnapshot(renderer)
} finally {
Expand Down
18 changes: 18 additions & 0 deletions src/elements/frame_element.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,24 @@ export class FrameElement extends HTMLElement {
}
}

/**
* Gets the refresh mode for the frame.
*/
get refresh() {
return this.getAttribute("refresh")
}

/**
* Sets the refresh mode for the frame.
*/
set refresh(value) {
if (value) {
this.setAttribute("refresh", value)
} else {
this.removeAttribute("refresh")
}
}

/**
* Determines if the element is loading
*/
Expand Down
7 changes: 7 additions & 0 deletions src/elements/stream_element.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ export class StreamElement extends HTMLElement {
return this.getAttribute("targets")
}

/**
* Reads the request-id attribute
*/
get requestId() {
return this.getAttribute("request-id")
}

#raise(message) {
throw new Error(`${this.description}: ${message}`)
}
Expand Down
13 changes: 13 additions & 0 deletions src/http/fetch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { uuid } from "../util"

export function fetch(url, options = {}) {
const modifiedHeaders = new Headers(options.headers || {})
const requestUID = uuid()
window.Turbo.session.recentRequests.add(requestUID)
modifiedHeaders.append("X-Turbo-Request-Id", requestUID)

return window.fetch(url, {
...options,
headers: modifiedHeaders
})
}
1 change: 1 addition & 0 deletions src/http/fetch_request.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { FetchResponse } from "./fetch_response"
import { expandURL } from "../core/url"
import { dispatch } from "../util"
import { fetch } from "./fetch"

export function fetchMethodFromString(method) {
switch (method.toLowerCase()) {
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as Turbo from "./core"
window.Turbo = Turbo
Turbo.start()

export { StreamActions } from "./core/streams/stream_actions"
export * from "./core"
export * from "./elements"
export * from "./http"
3 changes: 3 additions & 0 deletions src/tests/fixtures/frame_refresh_morph.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<turbo-frame id="remote-frame">
<h2>Loaded morphed frame</h2>
</turbo-frame>
Loading

0 comments on commit a178184

Please sign in to comment.