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

Only include Turbo::Broadcastable::TestHelper with Action Cable #574

Merged
merged 1 commit into from Feb 9, 2024

Conversation

seanpdoyle
Copy link
Contributor

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.

if defined? ActionCable
require "turbo/broadcastable/test_helper"
include Turbo::Broadcastable::TestHelper
end
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@t27duck
Copy link
Contributor

t27duck commented Feb 9, 2024

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)
@afcapel afcapel merged commit 07d3488 into hotwired:main Feb 9, 2024
15 checks passed
@afcapel
Copy link
Contributor

afcapel commented Feb 9, 2024

Thanks @seanpdoyle 🙏

@seanpdoyle seanpdoyle deleted the issue-573 branch February 9, 2024 20:05
@joshuay03
Copy link

@seanpdoyle Is this planned to be released / back-ported anytime soon?

mshibuya added a commit to railsadminteam/rails_admin that referenced this pull request Feb 12, 2024
@nflorentin
Copy link

nflorentin commented Feb 13, 2024

@seanpdoyle

Upgrading from 2.0.0 to 2.0.2 breaks with the following error :

uninitialized constant Turbo::Broadcastable::TestHelper::ActionCable (NameError)

          include ActionCable::TestHelper

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 :-)

@seanpdoyle
Copy link
Contributor Author

@nflorentin this change is not included in 2.0.2. Does depending on main resolve the issue?

@nflorentin
Copy link

@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 !

@aaronjensen
Copy link
Contributor

aaronjensen commented Feb 14, 2024

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!

@seanpdoyle
Copy link
Contributor Author

@afcapel @jorgemanrubia could either of you consider a fast-tracked 2.0.3 release?

@afcapel
Copy link
Contributor

afcapel commented Feb 16, 2024

I've just published 2.0.3 👍

@aaronjensen
Copy link
Contributor

Thank you!

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

Successfully merging this pull request may close these issues.

uninitialized constant Turbo::Broadcastable::TestHelper::ActionCable
6 participants