Support custom rendering in `turbo:before{-frame,}-render` events #431

seanpdoyle · 2021-10-31T17:56:34Z

Renderer: Extract `renderElement()`

For each renderer (PageRenderer, ErrorRenderer, FrameRenderer)
extract their rendering logic into a class-static- property named
renderElement(). That function accepts two arguments: the first is the
current version of the element being rendered, the second is the new
version of the element being rendered.

The PageRenderer and ErrorRenderer types are genericized to accept
HTMLBodyElement instances. The FrameRenderer generics accept a
FrameElement instance. This means that the renderElement() function
shares the same class-level generics.

Extract `ViewRenderOptions`

Introduce the ViewRenderOptions<E extends Element> to store the
resume() function dispatched with the turbo:before-render event.

Future commits will add additional properties to this option interface.

Support custom rendering from `turbo:before-render`

The problem

The rendering process during a page-wide navigation is opaque, and
cannot be extended. Proposals have been made to use third-party
rendering tools like morphdom, or other animation libraries.

Outside of the HTML manipulation, Turbo is also responsible for loading
script, transposing permanent elements, etc.

How might these tools integrate with Turbo in a way that's compliant
with permanent elements.

The solution

When publishing a turbo:before-render event, dispatch it with a
render() function property in addition to the resume().

This way, consumer applications can override rendering:

import morphdom from "morphdom"

addEventListener("turbo:before-render", ({ detail }) => {
  detail.render = (currentElement, newElement) => {
    morphdom(currentElement, newElement)
  }

  // or, more tersely
  detail.render = morphdom
})

Potentially Closes hotwired/turbo#197
Potentially Closes hotwired/turbo#378
Potentially Closes hotwired/turbo#218

Introduce `turbo:before-frame-render` event

The problem

Similar to turbo:before-render events and the <body> element,
rendering <turbo-frame> events is opaque and isn't extensible.

The solution

Publish a turbo:before-frame-render event, dispatch it with a
render() function property in addition to the resume().

This way, consumer applications can override rendering in the same style
as turbo:before-render events.

seanpdoyle · 2021-10-31T17:57:26Z

This is a companion PR to #146. #146 is meant to expose the out-of-the-box rendering in a plug-and-play way, this PR intends to allow for total overrides.

src/core/frames/frame_controller.ts

aaronjensen · 2022-02-20T17:44:04Z

@seanpdoyle this and #146 are very interesting. Are you still pursuing getting these merged or do you have alternative ways of accomplishing the same things?

seb-jean · 2022-05-15T08:50:02Z

Nice PR

seanpdoyle · 2022-07-16T14:21:42Z

If this were to ship, it might be worthwhile to reverse the changes made in #568 and #305 so that the PageRenderer and FrameRenderer classes remain private implementation details.

rainerborene · 2022-07-17T13:14:25Z

I think this is pretty useful. Right now we're monkey patching the PageRenderer class.

import { PageRenderer } from "@hotwired/turbo"

Object.assign(PageRenderer.prototype, {
  assignNewBody() {
    const container = document.querySelector("#app")
    const newContainer = this.newElement.querySelector("#app")

    if (container && newContainer) {
      container.replaceWith(newContainer)
    } else {
      document.body.replaceWith(this.newElement)
    }
  }
})

dhh · 2022-07-17T23:25:15Z

Be happy to see this refreshed for 7.2.0.

For each renderer (`PageRenderer, `ErrorRenderer`, `FrameRenderer`) extract their rendering logic into a class-`static-` property named `renderElement()`. That function accepts two arguments: the first is the current version of the element being rendered, the second is the new version of the element being rendered. The `PageRenderer` and `ErrorRenderer` types are genericized to accept `HTMLBodyElement` instances. The `FrameRenderer` generics accept a `FrameElement` instance. This means that the `renderElement()` function shares the same class-level generics.

Introduce the `ViewRenderOptions` to store the `resume()` function dispatched with the `turbo:before-render` event. Future commits will add additional properties to this option interface.

The problem --- The rendering process during a page-wide navigation is opaque, and cannot be extended. Proposals have been made to use third-party rendering tools like [morphdom][], or other animation libraries. Outside of the HTML manipulation, Turbo is also responsible for loading script, transposing permanent elements, etc. How might these tools integrate with Turbo in a way that's compliant with permanent elements. The solution --- When publishing a `turbo:before-render` event, dispatch it with a `render()` function property in addition to the `resume()`. This way, consumer applications can override rendering: ```html import morphdom from "morphdom" addEventListener("turbo:before-render", ({ detail }) => { detail.render = (currentElement, newElement) => { morphdom(currentElement, newElement) } // or, more tersely detail.render = morphdom }) ``` Potentially Closes [hotwired#197][] Potentially Closes [hotwired#378][] Potentially Closes [hotwired#218][] [morphdom]: https://github.com/patrick-steele-idem/morphdom [hotwired#218]: hotwired#218 [hotwired#378]: hotwired#378 [hotwired#197]: hotwired#197

The problem --- Similar to `turbo:before-render` events and the `<body>` element, rendering `<turbo-frame>` events is opaque and isn't extensible. The solution --- Publish a `turbo:before-frame-render` event, dispatch it with a `render()` function property in addition to the `resume()`. This way, consumer applications can override rendering in the same style as `turbo:before-render` events.

The test passes consistently locally, but fails consistently in CI: ``` 1) [chrome] › form_submission_tests.ts:909:1 › test following a link with [data-turbo-method] and [data-turbo=true] set when html[data-turbo=false] AssertionError: does not navigate the full page: expected 'Hello' to equal 'Form' 918 | await link.click() 919 | > 920 | assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page") | ^ 921 | assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame") 922 | }) 923 | at /home/runner/work/turbo/turbo/src/tests/functional/form_submission_tests.ts:920:10 2) [chrome] › form_submission_tests.ts:924:1 › test following a link with [data-turbo-method] and [data-turbo=true] set when Turbo.session.drive = false AssertionError: does not navigate the full page: expected 'Hello' to equal 'Form' 932 | await link.click() 933 | > 934 | assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page") | ^ 935 | assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame") 936 | }) 937 | at /home/runner/work/turbo/turbo/src/tests/functional/form_submission_tests.ts:934:10 ``` As a precaution, escape the URL path to embedded into the `[href]` attribute. Additionally, execute CI with the GitHub Actions reporter.

dhh · 2022-07-18T03:04:21Z

This could use a solid doc PR with examples of how and why to overwrite this.

Follow-up to hotwired#431 Support for the `turbo:before-frame-render` event relies on the same underlying infrastructure as the `turbo:before-render` event (i.e. the `Renderer` abstract class). As a result of re-using that abstraction, listeners for the `turbo:before-frame-render` event can also leverage the `detail.resume` function in the same way to handle asynchronous rendering. The changes made in [hotwired#431][] excluded test coverage for that behavior. This commit adds that coverage to guard against regressions in that behvaior. [hotwired#431]: hotwired#431

* Add tests for pausable Frame Rendering Follow-up to #431 Support for the `turbo:before-frame-render` event relies on the same underlying infrastructure as the `turbo:before-render` event (i.e. the `Renderer` abstract class). As a result of re-using that abstraction, listeners for the `turbo:before-frame-render` event can also leverage the `detail.resume` function in the same way to handle asynchronous rendering. The changes made in [#431][] excluded test coverage for that behavior. This commit adds that coverage to guard against regressions in that behvaior. [#431]: #431 * Form Links: Copy `[data-turbo-frame]` from `<a>` to `<form>` Follow-up to #633 The flaky test outlined in [#633][] was "flaky", but not in the sense that was originally suspected. Somehow, it was presenting as a false negative, failing when we thought it should consistently pass. On further inspection, it was _passing when it should consistently fail_. This commit addresses the underlying issue by copying any `[data-turbo-frame]` attributes onto the `<form>` element from the `<a>` element that is clicked outside of the targeted `<turbo-frame>`. [#633]: #633

* Add a updateHistory check to visit * make frame change history before rendering * add frame fixture * extract history logic to History class * maintain the same restorationIdentifier * remove guard that was always true * lint * make history receive a method * lint * reset frame and action at each nav * udpate ids to reflect each link * extract getVisitAction and fix compiler issues * lint * add before-frame-render event and make the render pausable * Add test showcasing the URL being updated first * add pause/abort tests for frames * update ids to match initial page * remove before-frame-render event code since it was introduced in #431 * yarn.lock * yarn.lock

scottnicolson · 2022-08-22T19:49:39Z

I think this is pretty useful. Right now we're monkey patching the PageRenderer class.

import { PageRenderer } from "@hotwired/turbo"

Object.assign(PageRenderer.prototype, {
  assignNewBody() {
    const container = document.querySelector("#app")
    const newContainer = this.newElement.querySelector("#app")

    if (container && newContainer) {
      container.replaceWith(newContainer)
    } else {
      document.body.replaceWith(this.newElement)
    }
  }
})

@rainerborene I tried this approach while we wait for version 7.2.0. It only worked for forward links for me. i.e. if I used the browser back button (both chrome and safari) then the page would not update. Did you not experience this issue with your patch?

manuelpuyol · 2022-10-27T22:31:41Z

@seanpdoyle I'm experiencing some issues with back/forward buttons with custom renders. I created a very simple example here https://github.com/hotwired/turbo/compare/main...manuelpuyol:turbo:showcase-custom-render-bug?expand=1

There, instead of replacing the <body>, I'm replacing an element marked by data-turbo-body, but that change makes back/forward navigation inconsistent, sometimes loading the correct page:

Screen.Recording.2022-10-27.at.5.27.37.PM.mov

brunoprietog · 2022-10-28T06:05:00Z

Hi @seanpdoyle

I also have a similar problem.

In my case, I am replacing the div#app element instead of the body, because I have additional elements I want to keep.

I can reproduce it by entering one page, navigating to another, navigating back to the previous page, and trying to navigate to the current page again.

What I found is that the turbo:before-render event is executed twice in some cases. When that happens, the first time the event runs, I get the new element with all elements except div#app.

If I don't replace the detail.render method, I can see that when it happens that the event is emited twice, the first time the new element is an empty body, and the second time it is the new element.

I don't know what's going on, but I temporarily fixed it like this:

const findAppElement = (element) => element.querySelector("#app")

document.addEventListener("turbo:before-render", ({ detail }) => {
  detail.render = (currentElement, newElement) => {
    const currentAppElement = findAppElement(currentElement)
    const newAppElement = findAppElement(newElement)
    if (newAppElement) currentAppElement.replaceWith(newAppElement)
    else currentElement.prepend(currentAppElement)
  }
})

By doing this, the second time the event is emited I really get the new element and can replace it correctly.

It would be great if you could find a way to fix this so that Turbo handles this case correctly and other people don't have this problem, because it can't consistently rely on the new element. Actually, Turbo should not emit this event twice in any case.

Thanks!

scottnicolson · 2022-10-28T12:57:00Z

@manuelpuyol I resolved my issue adding a in a delay as detailed here

Please note that it is Janky, as I point out here. You should be aware that using data-turbo-permanent with this approach doesn't work. I got around that by using a stimulus controller to clear the Turbo cache in certain circumstances where I was using data-turbo-permanent. I would say just use with caution and keep our fingers crossed that #711 gets some attention soon.

manuelpuyol · 2022-10-28T16:06:56Z

thanks for the tip @scottnicolson !

I got around that by using a stimulus controller to clear the Turbo cache in certain circumstances where I was using data-turbo-permanent

Could you elaborate a bit more here? We do have some data-turbo-permanent elements

dylanfisher · 2022-10-29T19:29:24Z

This is a great addition to Turbo, but I don't think it's documented in the handbook yet. Would be awesome if there was some documentation for it.

scottnicolson · 2022-10-30T16:27:39Z

Could you elaborate a bit more here? We do have some data-turbo-permanent elements

@manuelpuyol I had an index page of items and each item had a bookmark icon. The bookmark icon also showed up on the show page for each item. The bookmark icon was marked as data-turbo-permanent because if you toggled the bookmark on the show page and used the browser back button to go back to the index, I wanted the bookmark state to be correct. I was using a turbo-stream to update the bookmark state as I didn't want it to affect the browser history. After updating the assignNewBody method the bookmarks would not show on the show page when navigating from the index page. I decided to forego the hassle and instead just used a stimulus controller to clear the Turbo cache when the bookmark was toggled. This ensured all stale pages would be reloaded (even on browser back).

Also worth noting that #623 makes it so data-turbo-permanent delivered in a turbo stream response are now ignored, so my bookmarking solution got worse with the introduction of that PR.

Documents [hotwired/turbo#431][] Documents [hotwired/turbo#684][] Share examples for `turbo:before-render`, `turbo:before-frame-render`, and `turbo:before-stream-render` events that override the `event.detail.render` function. Additionally, document pausable rendering for `<turbo-frame>` elements. [hotwired/turbo#431]: hotwired/turbo#431 [hotwired/turbo#684]: hotwired/turbo#684

trsteel88 · 2023-05-23T23:44:11Z

Out of curiosity, is there a reason Turbo doesn't use morphdom by default. It seems like this would be better for merging the dom?

It looks like morphdom is better when you're focused on a input field which is changing the state of the current frame. When the frame reloads with just Turbo, you lose focus. If you use morphdom, it retains its current focused state.

brunoprietog · 2023-05-24T00:56:20Z

I wonder the same thing. This would be quite good for accessibility too

marcoroth · 2023-05-24T19:09:42Z

Morphdom is good in a lot of use-cases, but it also has its drawbacks and weird side effects. Some of the behavior feels super random and unpredictable if you give it improper markup for example. Which is why I think it shouldn't be enabled by default, because it would probably break more.

But it should be straightforward enough to configure Turbo to do all the updates using morphdom, if you think its worth it for all the updates in your app.

There's also https://github.com/marcoroth/turbo-morph if you are looking for a pre-configured Turbo Stream action.

seb-jean · 2023-05-24T20:21:56Z

With this feature, how to avoid creating a Stimulus controller? Indeed, I followed a tutorial to create a pagination that scrolls infinitely (https://github.com/thoughtbot/hotwire-example-template/tree/hotwire-example-pagination#replacing-frames-with-their-content).

I created this Stimulus controller below but I think we can do better using this feature.

// app/javascript/controllers/element_controller.js

import { Controller } from "@hotwired/stimulus"

export default class extends Controller {
  replaceWithChildren({ target }) {
    this.element.replaceWith(...target.children)
  }
}

Originally it was using [rendering="replace"] from PR #146 but it was closed.

seanpdoyle force-pushed the before-render-events branch from b8ecfc7 to 3ec4072 Compare October 31, 2021 18:06

seanpdoyle commented Oct 31, 2021

View reviewed changes

src/core/frames/frame_controller.ts Outdated Show resolved Hide resolved

seanpdoyle force-pushed the before-render-events branch from 3ec4072 to 87e4f23 Compare November 11, 2021 18:14

seanpdoyle marked this pull request as draft November 13, 2021 15:19

This was referenced Nov 26, 2021

Adds an update-attribute stream action so you can update an attribute on targets with a value #477

Closed

Custom Actions: Export StreamActions module #479

Merged

seanpdoyle mentioned this pull request Jan 4, 2022

Feature request for turbo frame? #509

Closed

botandrose mentioned this pull request Apr 13, 2022

Expose FrameRenderer so we can extend its behavior, e.g. vDOM. #568

Merged

seanpdoyle force-pushed the before-render-events branch from 87e4f23 to 0ba0d8e Compare July 16, 2022 14:15

seanpdoyle marked this pull request as ready for review July 16, 2022 14:15

seanpdoyle mentioned this pull request Jul 16, 2022

Expand rendering capabilities of <turbo-frame> #146

Closed

seanpdoyle force-pushed the before-render-events branch 3 times, most recently from 4630def to 3b7a448 Compare July 16, 2022 14:40

seanpdoyle mentioned this pull request Jul 17, 2022

Allow customizing the element that Drive replaces #627

Merged

seanpdoyle force-pushed the before-render-events branch 2 times, most recently from b70b0c8 to 449420e Compare July 18, 2022 00:49

seanpdoyle added 3 commits July 17, 2022 20:54

Extract ViewRenderOptions

ac503a4

Introduce the `ViewRenderOptions` to store the `resume()` function dispatched with the `turbo:before-render` event. Future commits will add additional properties to this option interface.

seanpdoyle force-pushed the before-render-events branch 2 times, most recently from 5385c8c to 5fdfce1 Compare July 18, 2022 01:28

seanpdoyle mentioned this pull request Jul 18, 2022

[Flaky Tests]: Link Form Submission #633

Closed

seanpdoyle force-pushed the before-render-events branch from 1d1a31e to 3a3a29d Compare July 18, 2022 02:27

seanpdoyle added 2 commits July 17, 2022 22:39

seanpdoyle force-pushed the before-render-events branch from 3a3a29d to 4ca2dbc Compare July 18, 2022 02:39

dhh merged commit 837e977 into hotwired:main Jul 18, 2022

seanpdoyle deleted the before-render-events branch July 18, 2022 03:06

seanpdoyle mentioned this pull request Jul 18, 2022

Add tests for pausable Frame Rendering #635

Merged

manuelpuyol mentioned this pull request Jul 20, 2022

[main branch] Cache is incorrect when navigating using promoted frames #643

Closed

manuelpuyol mentioned this pull request Oct 28, 2022

Allow users to define an async custom render function #782

Merged

marcoroth mentioned this pull request Nov 4, 2022

Prevent external IFrame from being reloaded between visits #783

Open

TastyPi mentioned this pull request Dec 8, 2022

Does the Render type have the parameter order wrong? #812

Closed

seanpdoyle mentioned this pull request Dec 29, 2022

Document turbo:before-{,frame,stream}-render events hotwired/turbo-site#126

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support custom rendering in `turbo:before{-frame,}-render` events #431

Support custom rendering in `turbo:before{-frame,}-render` events #431

seanpdoyle commented Oct 31, 2021 •

edited

Loading

seanpdoyle commented Oct 31, 2021

aaronjensen commented Feb 20, 2022

seb-jean commented May 15, 2022

seanpdoyle commented Jul 16, 2022

rainerborene commented Jul 17, 2022

dhh commented Jul 17, 2022

dhh commented Jul 18, 2022

scottnicolson commented Aug 22, 2022

manuelpuyol commented Oct 27, 2022

brunoprietog commented Oct 28, 2022

scottnicolson commented Oct 28, 2022

manuelpuyol commented Oct 28, 2022

dylanfisher commented Oct 29, 2022 •

edited

Loading

scottnicolson commented Oct 30, 2022

trsteel88 commented May 23, 2023

brunoprietog commented May 24, 2023

marcoroth commented May 24, 2023

seb-jean commented May 24, 2023 •

edited

Loading

Support custom rendering in turbo:before{-frame,}-render events #431

Support custom rendering in turbo:before{-frame,}-render events #431

Conversation

seanpdoyle commented Oct 31, 2021 • edited Loading

Renderer: Extract renderElement()

Extract ViewRenderOptions

Support custom rendering from turbo:before-render

The problem

The solution

Introduce turbo:before-frame-render event

The problem

The solution

seanpdoyle commented Oct 31, 2021

aaronjensen commented Feb 20, 2022

seb-jean commented May 15, 2022

seanpdoyle commented Jul 16, 2022

rainerborene commented Jul 17, 2022

dhh commented Jul 17, 2022

dhh commented Jul 18, 2022

scottnicolson commented Aug 22, 2022

manuelpuyol commented Oct 27, 2022

brunoprietog commented Oct 28, 2022

scottnicolson commented Oct 28, 2022

manuelpuyol commented Oct 28, 2022

dylanfisher commented Oct 29, 2022 • edited Loading

scottnicolson commented Oct 30, 2022

trsteel88 commented May 23, 2023

brunoprietog commented May 24, 2023

marcoroth commented May 24, 2023

seb-jean commented May 24, 2023 • edited Loading

Support custom rendering in `turbo:before{-frame,}-render` events #431

Support custom rendering in `turbo:before{-frame,}-render` events #431

seanpdoyle commented Oct 31, 2021 •

edited

Loading

Renderer: Extract `renderElement()`

Extract `ViewRenderOptions`

Support custom rendering from `turbo:before-render`

Introduce `turbo:before-frame-render` event

dylanfisher commented Oct 29, 2022 •

edited

Loading

seb-jean commented May 24, 2023 •

edited

Loading