Add `render` to `turbo:before-stream-render` event #684

seanpdoyle · 2022-08-13T17:28:13Z

Follow-up to #479
Related to hotwired/turbo-site#107

As an alternative to exposing an otherwise private and internal
StreamActions "class", support custom <turbo-stream action="...">
values the same way as custom <turbo-frame> rendering or <body>
rendering: as part of its turbo:before-render-stream event.

For example, if Turbo were to rename StreamActions to something else,
it would need to continue to export StreamActions publicly for the
sake of backwards compatibility. By depending on an event listener
instead, client applications are insulated from any internal restructuring.

To change how Turbo renders the document during page rendering, client
applications declare a turbo:before-render event listener that
overrides its CustomEvent.detail.render function from its
default.

Similarly, to change how Turbo renders a frame, client applications
declare a turbo:before-frame-render and override its
CustomEvent.detail.render function from its default.

This commit introduces the StreamElement.renderElement function, and
extends the existing turbo:before-stream-render event to support the
same pattern with StreamElement.renderElement serving as the default
CustomEvent.detail.render value.

With those changes in place, callers can declare a document-wide event
listener and override based on the value of StreamElement.action:

const CustomActions = {
  customUpdate: (stream) => { /* ... */ },
  customReplace: (stream) => { /* ... */ },
  // ...
}

document.addEventListener("turbo:before-stream-render", (({ target, detail }) => {
  const defaultRender = detail.render

  detail.render = CustomActions[target.action] || defaultRender
}))

marcoroth · 2022-08-13T19:08:35Z

Nice, I like where this is heading! This also brings some consistency over which parts we have control over via event-listeners.

I like that you can intercept the rendering of a stream element, this allows the host application to intercept non-custom actions (like update or replace) but also custom actions introduced by third-party packages.

I can also see this feature useful when you conditionally want to "discard" a stream action or if you want to handle "the rendering" of certain actions by yourself.

In order to properly support a new custom action (like a custom morph action via morphdom) you also need to teach your server-side about it, which is not always desirable.

With this feature however, you would be able to intercept the rendering of the default actions and a) use morphdom without the need for a new custom action and b) you don't need to have any specific logic on the server-side for when to use morph over update/replace. You can simplify delegate this decision to the client.

Using the same example from above: I can also see a use-case where you have a server-side part which emits update / replace actions over which you don't have control over (like a part of the app which is served via a third-party Rails engine). With this feature you would be able to intercept and override the render function on the client-side without having to touch the emit-logic on the server-side.

With all of that said, I still think the export of the StreamActions module is beneficial. It feels more straightforward to let Turbo know about all the available Custom Actions. If you just rely on event-listeners to "register" and support custom actions you are never letting Turbo (or other packages) know that such custom actions even exist.

It feels more appropriate for packages to register new actions via StreamActions. Host applications can then intercept actions via the proposed event-listeners.

Versus the package needs to add an event-listener to override the render function in order to even "support and add" a new custom action. If the host application then wants to intercept the action it would need to add another event listener and override the function again, which feels kinda hacky (not to the mention the uncertainty that the two event-listener need to fire in the right order).

TL;DR: In my opinion your proposed changes have neat use-cases on their own and would be worth to add by themselves. In my opinion they don't interfere with the concept of defining Custom Actions via the exported StreamActions module, on the contrary, it even enhances them.

If it's about the public API / leaking internals, we could also add some helper functions on the exported Turbo object instead.

hopsoft · 2022-08-24T21:23:53Z

Agree that a turbo:before-stream-render event enhances the extensibility provided by StreamActions. The approaches aren't mutually exclusive. For example, I'd like to extend Turbo via StreamActions while allowing consumers of my extension(s) to change behavior via the event.

Hoping to see both of these features make it into the library. 🤞🏻

dhh · 2022-09-12T13:36:01Z

Agree with @marcoroth's assessment that this seems a nice AND but needn't be an OR. If all you want to do is add a custom action, the currently exposed approach seems more intuitive. But this then allows you to go even further. Happy to see the AND go in.

When serializing `CustomEvent.detail` objects from the browser to the Playwright test harness, the current `serializeToChannel` implementation is prone to recurse infinitely if an object nests another object that refers to the outer object. To prevent that, this commit tracks which objects have been visited during the current serialization process, and avoid an infinitely recursing loop. Related to that change, this commit also add more descriptive types to the `EventLog` and `MutationLog` arrays to clarify which positional elements correspond to their serialized values. Finally, extend the test server's `<turbo-stream>` creation actions to support serializing an element with an `[id]` value, so that it can be serialized from the browser to the test harness, and serve as a `target.id` value.

seanpdoyle · 2022-09-12T13:49:12Z

I've pushed up changes to restore the StreamActions symbol's export declaration.

Follow-up to hotwired#479 Related to hotwired/turbo-site#107 As an alternative to exposing an otherwise private and internal `StreamActions` "class", support custom `<turbo-stream action="...">` values the same way as custom `<turbo-frame>` rendering or `<body>` rendering: as part of its `turbo:before-render-stream` event. To change how Turbo renders the document during page rendering, client applications declare a `turbo:before-render` event listener that overrides its `CustomEvent.detail.render` function from its [default][PageRenderer.renderElement]. Similarly, to change how Turbo renders a frame, client applications declare a `turbo:before-frame-render` and override its `CustomEvent.detail.render` function from its [default][]. This commit introduces the `StreamElement.renderElement` function, and extends the existing `turbo:before-stream-render` event to support the same pattern with `StreamElement.renderElement` server as the default `CustomEvent.detail.render` value. With those changes in place, callers can declare a document-wide event listener and override based on the value of `StreamElement.action`: ```javascript const CustomActions = { customUpdate: (stream) => { /* ... */ } customReplace: (stream) => { /* ... */ } // ... } document.addEventListener("turbo:before-stream-render", (({ target, detail }) => { const defaultRender = detail.render detail.render = CustomActions[target.action] || defaultRender })) ``` [PageRenderer.renderElement]: https://github.com/hotwired/turbo/blob/256418fee0178ee483d82cd9bb579bd5df5a151f/src/core/drive/page_renderer.ts#L7-L13 [FrameRenderer.renderElement]: https://github.com/hotwired/turbo/blob/256418fee0178ee483d82cd9bb579bd5df5a151f/src/core/frames/frame_renderer.ts#L13-L24

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

seanpdoyle mentioned this pull request Aug 13, 2022

See hotwired/turbo#684 marcoroth/turbo-morph#3

Closed

seanpdoyle force-pushed the before-render-stream-element branch 2 times, most recently from 5ca24bf to b62b839 Compare August 13, 2022 17:47

seanpdoyle force-pushed the before-render-stream-element branch from b62b839 to 7b3def6 Compare September 12, 2022 13:47

seanpdoyle force-pushed the before-render-stream-element branch from 7b3def6 to 7465c4f Compare September 12, 2022 13:50

dhh merged commit 9e6e0b9 into hotwired:main Sep 13, 2022

seanpdoyle deleted the before-render-stream-element branch September 13, 2022 14:06

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

Add `render` to `turbo:before-stream-render` event #684

Add `render` to `turbo:before-stream-render` event #684

seanpdoyle commented Aug 13, 2022 •

edited

Loading

marcoroth commented Aug 13, 2022 •

edited

Loading

hopsoft commented Aug 24, 2022

dhh commented Sep 12, 2022

seanpdoyle commented Sep 12, 2022

Add render to turbo:before-stream-render event #684

Add render to turbo:before-stream-render event #684

Conversation

seanpdoyle commented Aug 13, 2022 • edited Loading

marcoroth commented Aug 13, 2022 • edited Loading

hopsoft commented Aug 24, 2022

dhh commented Sep 12, 2022

seanpdoyle commented Sep 12, 2022

Add `render` to `turbo:before-stream-render` event #684

Add `render` to `turbo:before-stream-render` event #684

seanpdoyle commented Aug 13, 2022 •

edited

Loading

marcoroth commented Aug 13, 2022 •

edited

Loading