Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Turbo] Improving Turbo Stream Sources and Listeners (proposal) #2460

Open
smnandre opened this issue Dec 21, 2024 · 7 comments
Open

[Turbo] Improving Turbo Stream Sources and Listeners (proposal) #2460

smnandre opened this issue Dec 21, 2024 · 7 comments

Comments

@smnandre
Copy link
Member

TL;DR; - Turbo Stream

Proposal to introduce new helper / classes

  • StreamSourceUrlGenerator
  • turbo-stream-source-controller.js
  • turbo_stream_source_url
  • <twig:turbo:stream:source />

With all implementation details being hidden behind the StreamSourceUrlGenerator.

This would ease turbo stream usage, and open features to custom implementations while keeping things seamless for users.

Open to feedback, ideas, comments !

1. Main Goal

We aim to improve the developer experience (DX) offered by UX Turbo when using Turbo, especially Turbo Streams.

Several recent pull requests have helped us move forward:

  • New TurboStreamResponse and TurboStream helper classes in #2196, completed in #2298
  • New <twig:Turbo:Stream /> components in #2227, completed in #2302
  • New <twig:Turbo:Frame /> in #2303

(Apologies if I missed one—please correct me! And thank you to everyone involved.)

2. Current Situation

@norkunas recently had to find clever internal tricks (#2407) to allow registration on multiple topics at once.

In #2447, @Fan2Shrek tried to adapt the current Twig function to pass Mercure options to handle authorization.

This, however, raised questions about implementation, coupling, current limitations, and DX.

Challenges

As demonstrated by @norkunas and @Fan2Shrek, we must ensure compliance with Symfony's BC (backward compatibility) promise.

For a full explanation of what this entails, refer to Symfony’s BC guidelines.

In this context, our actions are mostly constrained by:

  • The TurboStreamListenRendererInterface
  • The TurboStreamListenRenderer

Limitations

But there is also several issues with the current DX.

The turbo_render_listen function does not render a full HTML element, only attributes.

This requires users to create the HTML tag themselves, which is not intuitive and slightly inconsistent with other UX features.

Additionally:

  • Stimulus attributes cannot be chained, making it impossible to add another Stimulus controller to the same tag.
  • Directly rendering attributes as escaped strings makes the function incompatible with Twig components, custom Stimulus attributes, or JavaScript. This also makes it unsafe for use with includes, embeds, etc., without requiring a |raw filter.

Coupling

The PHP code utilizes interfaces, so the coupling with the Mercure bridge is currently low.

However, on the JavaScript/controller side, the situation is different. The turbo_stream_controller.js is exclusively used as a Mercure stream controller, making it currently impossible to use it with other types of event sources. The controller requires multiple inputs/values (hub, topics, etc.), but ultimately only needs a URL for the EventSource required by Turbo. We should construct this URL in PHP and pass it directly.

Less important, the Twig function renderer interface depends on the Environment class. Yet, this dependency is not currently used in TurboStreamListenRenderer, as it is injected into the StimulusHelper.

3. Future Changes

Mercure Internals

We should avoid implementing anything in the Turbo Bundle that duplicates functionality from the Mercure Bundle. Instead, it should be clear that the Mercure Bundle is a requirement for using Mercure streams with Turbo. All aspects related to security, authorization, etc., should be delegated to the Mercure Bundle.

Simultaneously, any direct usage of Mercure Bundle code (or assumptions about its existence) should include safeguards to avoid issues arising from future updates to the bundle.

Turbo Needs

Taken from the Turbo documentation:

Turbo's <turbo-stream-source> custom element connects to a stream source through its [src] attribute. When declared with a ws:// or wss:// URL, the underlying stream source will be a WebSocket instance. Otherwise, the connection uses an EventSource.

Ultimately, we only need a URL and possibly some options. Constructing this (including creating cookies or JWTs if necessary) should be handled internally.

4. Proposal

  • Keep existing methods/interfaces (?)
  • Introduce a generic StreamSourceUrlGenerator
  • Introduce a new generic function/component to render Turbo Stream sources
  • Create a new turbo_stream_source_controller to handle EventSource

Stream Source URL

Interface: TurboStreamUrlGenerator

Introduce an interface:

interface UX\Turbo\StreamSourceUrlGenerator {
    public function generateUrl(): string;
}

This interface would function similarly to the Router component, allowing for implementation flexibility.

We may need to pass additional parameters or introduce a Value Object (VO). I'm open to ideas as long as it remains generic and future-proof.

This allows users to use the assembled URL for any purpose.

Stream Source Controller

A new Stimulus Controller could be introduced, doing exactly what does the existing one, but manipulating only one value (the URL).

<div 
    class="foo" 
    data-controller="[...]-stream-source" 
    data-...-stream-source-url-value="{{ event_source }}" 
>
   ...
</div>

Stream Source Tag

Rather than rendering and escaping HTML attributes (which requires a Stimulus controller), we could generate a simple HTML element:

<turbo-stream-source src="..." />

This approach provides a clean DX for most users.

Twig Function: turbo_stream_source()

A new turbo_stream_source() function could be introduced. This function would:

  • Use the same signature as turbo_stream_url() (which it would call internally) ?
  • ...Or maybe accept only an url parameter ?
  • Handle escaping and other concerns
{{ turbo_stream_source() }}
{{ turbo_stream_source(url: ...) }}

Twig Component: <twig:turbo:stream:source />

The same functionality could be offered as a Twig component. The exact name is up for discussion (<twig:turbo:source /> might be simpler but less descriptive).

Example:

<twig:turbo:stream:source foo="..." topic="{{ [topicA, topicB] }}" ... />

or

<twig:turbo:stream:source url="{{ urlParams }}" />

Next Steps

I’d love to hear your feedback, ideas, and desires on this topic (as long as they stay within scope, haha).

I’m happy to start working on this or assist anyone interested in contributing!

References

Mercure

Turbo

UX Turbo

Web Events

@smnandre
Copy link
Member Author

Also cc seb-jean Guervyl gremo DRaichev

@seb-jean
Copy link
Contributor

seb-jean commented Feb 1, 2025

My main wish is to subscribe to private topic using turbo_stream_listen(). This is notably related to the issue #2447 and the fact of creating a cookie.

@smnandre
Copy link
Member Author

smnandre commented Feb 1, 2025

I know, and I had the impression i explained the challenges it raised but seen the feedback on this issue I guess there is not much interest (other than you; thank you 😘)

@hafkenscheid
Copy link

There is actually more interest in subscribing to private topics. To be honest, I don't really had the time yet to understand the problem with private topics in combination with this topic yet.

@seb-jean
Copy link
Contributor

seb-jean commented Feb 2, 2025

Hello @gremo , @Guervyl, @DRaichev , @Fan2Shrek , @norkunas and sorry to bother you :).
But feel free to share your ideas / feedback on this topic that Simon took the time to do.

@norkunas
Copy link
Contributor

norkunas commented Feb 3, 2025

The turbo_render_listen function does not render a full HTML element, only attributes.
This requires users to create the HTML tag themselves, which is not intuitive and slightly inconsistent with other UX features.

For me that's not a limitation, and does what I expected - I already have an element/component on which I apply the turbo_render_listen function

@DRaichev
Copy link
Contributor

DRaichev commented Feb 3, 2025

I like the proposal, but I need to look further into it so I can maybe give more specific feedback.

In general I feel like Symfony UX is not on par with Symfony Core, when it comes to the various components being independent/modular/decoupled, so any push in this direction is welcome. And it seems most of the issues outlined here are related to this.

When it comes to turbo_render_listen, I don't have a strong opinion on rendering full html element vs attributes. I do however, really like the twig component syntax and for me this is a clear way to differentiate between the two, i.e. for functions that that add attributes you use them as functions, and the ones that create full elements always use them as twig components, never as functions. This avoids any confusion. A static analysis rule that forbids usage of the non-twig component variant might be a good idea, for those who want it, but this is besides the topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants