stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

Proposal to separate the javascript into two parts; core and rails specific. #333

Closed jonathan-s closed 1 year ago

jonathan-s commented 4 years ago

Feature Request

Right now the stimulus reflex javascript library is in a single file molded to work perfectly with actioncable. I would like to see a separation into two javascript packages; stimulusreflex-core and stimulus_reflex.

stimulus_reflex would depend on stimulusreflex-core and this package would be tailored to work with rails and thus have a dependency on actioncable.

stimulusreflex-core would contain all the logic for making stimulusreflex work the way it does today except for the part where it assumes that actioncable is the websocket connection.

This feature request can only be dealt with after we've done satisfactory work on #182. #345 Does some excellent groundwork as well, and if that gets merged that would also set us up to split the library into a core and 'rails' specific implementation.

Is your feature request related to a problem?

As a django user I'm able to use the javascript version of stimulus reflex. Right now it depends on the javascript actioncable library, as a django user I wouldn't want that to be included.

leastbad commented 3 years ago

Hey @jonathan-s, as you know @julianrubisch has been doing some high-quality thinking about how to make SR more modular with an adapter approach, especially around transport mechanisms.

It seems like in order to support other transports, we'd have to compartmentalize ActionCable anyhow. This would seem to (somewhat) implicitly address the spirit of this issue, because ActionCable would become just another adapter.

This would all very definitely require a major SemVer bump, so we do have some latitude to make big changes. However, the other side of the coin here is that this isn't a trivial adaptation and we are currently chasing other priorities so I want to be realistic about the degree to which this is on an unknown timeline. Hopefully Q2 2021? I'm spitballing.

One thing that could be really helpful is a triage of the API surface to identify which chunks of code would ultimately be moved into an adapter. I don't expect you to own this on the server, but on the client, there's some good conversation to be had around, for example, what I call "library-level" events for connectivity. Eg. does an adapter have to support these events? What does it mean to be connected or disconnected when you're using Ajax? Is there a heartbeat involved? Or does an adapter have some agency to say "I am BiffAdapter and I support features X and Z but not Y"?

jonathan-s commented 3 years ago

Thanks @leastbad, I definitely understand that this isn't a trivial update and needs some thinking around it. I added this issue so we can start some thinking around it for this very reason.

You also pose some great questions around what an adapter should support. I'll have a think about this and provide some input here!

julianrubisch commented 3 years ago

As I already noted elsewhere a couple of times, IMHO the hard part is to abstract away the CableReady dependency on the client side, since if we make transport adapters a reality, we are going to need a morphdom wrapper that is not bound to ActionCable.

So my €0.02 are: we should separate these two discussions. I think splitting CR into „core“ and „ws“ should be fairly straightforward

jonathan-s commented 3 years ago

Some thoughts on how I would structure things.

controller.StimulusReflex would have a consumer that you can access, instead of accessing the consumer via the subscription. The consumer should have a method isOpen. I would also rename isActionCableConnectionOpen to isWebsocketConnectionOpen

That would help here.

The register method would also take a channel argument and a create subscription function. The register method would be part of stimulusreflex-core and from stimulus_reflex you'd export a register method that doesn't require a channel argument or a create subscription function.

This would make it possible for stimulusreflex-core to not depend on getConsumer which is actioncable specific.

The object that is added in subscription here should be a constant that is available in stimulusreflex-core. You'd use that constant as a part to make your own create subscription method.

actionCableConsumer would also be renamed to websocketConsumer

The initialize function would follow a similar pattern as the register function.

I believe that these changes would largely be backwards compatible. I hope my description makes sense.

leastbad commented 3 years ago

This is exactly what I had hoped for.

My eyes are very close to the ground, so the best help comes from folks looking from above.

julianrubisch commented 1 year ago

Closing because going forward TurboBoost will take the role of the transport layer