-
Notifications
You must be signed in to change notification settings - Fork 331
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
Build-in integration with rails/ujs #257
Conversation
Since this PR is currently a draft, the implementation is coded directly into the Dummy application. If this approach is deemed viable, we can both build out more System Tests and figure out a way to integrate a new |
As an alternative to building support for Rails' Unobtrusive JavaScript into `@hotwired/turbo` itself, instead publish a `@hotwired/turbo-rails/ujs` file to re-use the existing `@rails/ujs` hooks, and bridge the gaps between new `turbo:`-prefixed and `ajax:`-prefixed events. This is a re-imagining of [hotwired/turbo#40][] and ([hotwired/turbo#384][]). If deemed viable, this work would yield some follow up tasks: * drop support for `[data-turbo-method]` ([hotwired/turbo#277][]) * drop support for `[data-confirm]` ([hotwired/turbo#379][]) [hotwired/turbo#40]: hotwired/turbo#40 [hotwired/turbo#277]: hotwired/turbo#277 [hotwired/turbo#379]: hotwired/turbo#379 [hotwired/turbo#384]:hotwired/turbo#384
Hey Sean! Cool to see work done to preserve UJS, I dont mean to hijack this issue, but I think these functions may be worth adding into a Rails-UJS rewrite I created that is intended to be as backwards compatible as possible. I think these functions could fit in nicely with the MrujsTurbo plugin plugin that exists in Mrujs today. Currently it only exists to auto-render TurboStream responses, but im realizing now there could be more functionality like this baked into it. I'm not sure where I'm going with this other than to say I'm going to grab these functions and add them into Mrujs' Turbo plugin. |
|
||
Rails.start() | ||
|
||
extendUJS(Rails) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ParamagicDev that's great to hear! I'm hoping that the "interface" we'll end up with will accept a Rails
-like interface.
Currently, the Rails
object from UJS exposes these selectors and methods as properties. If mrujs could expose those same properties and match the interface, swapping one for the other should "Just Work".
Does mrujs have an "API" outside of a .start()
and HTML data-
attributes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seanpdoyle it does! In fact I need to work on exposing it. Right now mrujs
exposes these under mrujs.querySelectors.<SelectorName>.selector
, which is less than ideal in terms of interfacing.
I tried my best to keep the data-*
attributes the same, however, it appears I missed the mark in how every function was surfaced top-level in Rails-UJS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other issue is that the idea of "delegate" doesnt exist in mrujs. Mrujs has a document-level MutationObserver that fires when attributes change and when new nodes are added to the DOM and then attaches behavior to anything that fits the querySelector parameters. I think this should be easy enough to port however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does mrujs have an "API" outside of a
.start()
and HTMLdata-
attributes?
@ParamagicDev is humble but he has really gone above and beyond with mrujs
. Being as close to "drop-in" backwards compatible - while allowing for progress wrt things like XHR -> fetch
- is a primary design criteria.
He's taken great care to be highly responsive to requests and put a huge amount of energy into making sure the docs are A+.
It's already got a growing userbase with over 2k/week downloads, and a 24/7 support channel.
It would be nice to see Rails core embrace this as an exciting path forward.
I don't see these functions as something fundamentally about UJS, but rather ways of controlling the flow of links and forms, which in my opinion fits well with the overall mission of Turbo. Meaning that these functions are generally valuable and inline with the goals for Turbo, and therefore should live in Turbo. The turbo-rails setup should be as slim as possible, just adding specific bridging to things that are uniquely Rails. I don't think neither data-turbo-method or data-turbo-confirm are uniquely Rails. |
Agree with your outlook and conclusion, for sure. My comment is really an expression of wishing that Rails itself, as an entity, would do more to promote exciting developments and ideas that emerge within the community - especially ones that aren't on any kind of inclusion track for Rails proper. When someone in the Laravel community really knocks it out of the park, Laravel really puts its weight behind it. Big idea modules go on the homepage and they presumably work it into their overall project marketing, socials, maybe even documentation. It's omakase... but with the waiter suggesting that you've just got to try the steak. Whether it's Haml, ViewComponent, StimulusReflex and CableReady, and now mrujs... it would be such a huge source of optimism and energy for the folks who work hard to make sure Rails kicks ass going into decade three to see you tweeting about things you might not plan on using, but think that some of your Twitter followers might love. For there to be excitement from the core team that someone would recognize a hole and spend months making mrujs. How amazing it would be to wake up and see DHH tweeting about something you poured your heart into instead of quietly wondering if you'll get put on blast for self-promotion if you post about it on the forum. We laugh, nervously, but trust me: I'm just vocalizing what others are shy about saying. |
I think it's wonderful to see that we have a wide array of options for substitutions and variations available to the default answers in Rails. And sometimes those options blaze a trail that then inspires adoption in Rails itself, even if it's in a different form. I also think we should have a proper forum where people who are working on such variations can feel good and safe about pointing to them. The Rails tent is so large that we could never all agree on a single omakase menu that everyone will love. Plenty of other examples out there, like minitest vs rspec. But the approach that Rails is taking is that there is one original omakase menu that includes a finished set of answers that someone can take to build their apps, without having to configure external packages. In this particular case, |
As to this specific PR, I could imagine having a shim that essentially makes the Turbo versions of -confirm and -disable accessible through the old bare options like |
TL;DRThe changes proposed in this pull request are intended to serve as an official UJS-provided There is an opportunity to cover the same ground as UJS' other
If I had had
This work is purely motivated by backwards-compatibility. The goal is to provide If an team is planning on migrating an application to Hotwire, code like this With that being said, I think it's OK for Turbo's idioms and events to be
I agree. I think there's an opportunity to integrate the features provided by For example, Turbo's It seems to me that we're all in agreement on that fact. Outside of that
|
@dhh With Rails 7 out the door, and The link_to and button_to helpers still support Since the Turbo versions of these data-attribute driven behaviors are prefixed by This means that applications upgrading and migrating from UJS need to modify Action View helpers throughout their view layer, removing options with built-in support for If we were to merge this PR (or another like it), the bridge code could enable applications to opt-into Turbo and more gradually remove or migrate Right now, the |
I'd rather try to solve this on the Rails side, tbh. Maybe we can do a shim gem that simply turns those |
As an alternative to building support for Rails' Unobtrusive JavaScript
into
@hotwired/turbo
itself, instead publish a@hotwired/turbo-rails/ujs
file to re-use the existing@rails/ujs
hooks, and bridge the gaps between new
turbo:
-prefixed andajax:
-prefixed events.This is a re-imagining of hotwired/turbo#40 and
(hotwired/turbo#384). If deemed viable, this work would yield some
follow up tasks:
[data-turbo-method]
(hotwired/turbo#277)[data-confirm]
(hotwired/turbo#379)