-
Notifications
You must be signed in to change notification settings - Fork 329
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
Only include Turbo::Broadcastable::TestHelper
with Action Cable
#574
Conversation
if defined? ActionCable | ||
require "turbo/broadcastable/test_helper" | ||
include Turbo::Broadcastable::TestHelper | ||
end |
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 what do you think about adding a ActiveSupport.on_load(:action_cable)
block here? The Rails guides suggest this technique to execute code that depends on several dependencies.
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.
I like that idea. I was unsure about nesting, but I'll try it out.
Cautiously optimistic, but this branch seems to fix the test suite for my personal app. |
Closes [hotwired#573][] Related to [hotwired#565][] Re-structure the automatic inclusion of the `Turbo::Broadcastable::TestHelper` module so that it's only automatically loaded and installed when Action Cable is available to the runtime. [hotwired#573]: hotwired#573 [hotwired#565]: hotwired#565 (comment)
Thanks @seanpdoyle 🙏 |
@seanpdoyle Is this planned to be released / back-ported anytime soon? |
Upgrading from 2.0.0 to 2.0.2 breaks with the following error :
I use turbo-rails without ActionCable, I don't need broadcasting. I don't understand why Turbo::Broadcastable::TestHelper is included by default, it seems to make turbo-rails not usable without action cable. Is there an option to override that behaviour ? Thanks in advance :-) |
@nflorentin this change is not included in |
@seanpdoyle ops ! yes, I'm sorry, I misunderstood the change and though it was in 2.0.2. It does resolve the issue. Will wait for 2.0.3. Thanks for your hard work ! |
Asking a question here with the best intentions given that this change fixes a breaking change that prevents people from updating to 2.X. Is there a significant impediment in the release process that prevents an immediate release after a fix like this is merged in order to unblock people that are downstream? If so, is there any way that I could help alleviate it? In a few of the Rails-adjacent projects (propshaft, turbo, turbo-rails) I've noticed that things get merged (which is great) but then releases don't happen for sometimes weeks or months. I know everyone is busy doing their full-time job so I'm not asking for heroics, I'm genuinely curious if there's anything I can do to help alleviate the cost of a release? Thank you! |
@afcapel @jorgemanrubia could either of you consider a fast-tracked 2.0.3 release? |
I've just published 2.0.3 👍 |
Thank you! |
Closes #573
Related to #565
Re-structure the automatic inclusion of the
Turbo::Broadcastable::TestHelper
module so that it's only automatically loaded and installed when Action Cable is available to the runtime.