Fix breaking in non-Zeitwerk environment #304
Conversation
|
We are using Rails 6.1 and Ruby 2.7.5 with |
lib/turbo/engine.rb
Outdated
|
|
||
| initializer "turbo.no_action_cable" do | ||
| Rails.autoloaders.once.do_not_eager_load(Dir["#{root}/app/channels/turbo/*_channel.rb"]) unless defined?(ActionCable) | ||
| Rails.autoloaders.once.do_not_eager_load(Dir["#{root}/app/channels/turbo/*_channel.rb"]) if !defined?(ActionCable) && Rails.autoloaders.zeitwerk_enabled? |
There was a problem hiding this comment.
Would this fix the problem? If zetiwerk is disable and action cable is not present, the action cable files would still be loaded. We need to teach the classic autoload to not load this code as well when zeitwerk is not enabled.
There was a problem hiding this comment.
Because the purpose of this fix is not to stop autoloading the code, but to fix breaking with NoMethodError.
Loading things what might be unnecessary is trivial, compared to completely breaking the classic-autoloader application.
And I couldn't find a way to
teach the classic autoload to not load this code
, but do you have any idea on it?
There was a problem hiding this comment.
I think maybe paths["app/channel"].clear? I think that would fix in both modes and not need to check if zeitwerk is enabled.
There was a problem hiding this comment.
Thanks for the suggestion. The given one ended up with:
/path/to/turbo-rails/lib/turbo/engine.rb:22:in `block in <class:Engine>': undefined method `clear' for #<Rails::Paths::Path:0x00000001348f1198 @paths=["app/channels"], ...>
but I managed to write an implementation which works with both Zeitwerk and the classic autoloader.
Please kindly check.
Change the implementation to support both Zeitwerk and the classic autoloader
mshibuya
force-pushed
the
fix/breaking-in-non-zeitwerk-env
branch
from
c72adf8 to
713864f
Compare
* main: (175 commits) Support `[formmethod]` overrides to `_method` (hotwired#239) call ActionView::RecordIdentifier.dom_id explicitly (hotwired#333) removed a handful of extra 'the' in method descriptions (hotwired#334) Fix missing `activejob` dependency (hotwired#331) Document the turbo_frame_tag helper with multiple arguments (hotwired#329) Add possibility to nest turbo frames (hotwired#296) Fix breaking in non-Zeitwerk environment (hotwired#304) Problem: inability to pass additional parameters to stream channels (hotwired#308) Fix CI with Rails main (hotwired#314) add targets to assert_{,no_}turbo_stream (hotwired#321) fix tests for rails 7.0 (hotwired#320) Test on Rails 7.0 Use where on Windows instead of which (hotwired#299) (hotwired#300) Bump version for 1.0.1 Upgrade @rails/actioncable to 7.0.1 Bump @rails/actioncable dependency to ^7.0 Add Turbo::Broadcastable support for #broadcast_render and #broadcast_render_to (hotwired#298) fix a little typo for wrong constant name (hotwired#290) Bump version for 1.0.0 Make clear this is included by default in Rails 7 ...
4 participants
I'm trying to use turbo-rails in our project, but with JRuby and Rails 6.1 the Rails app fails to start with emitting the following error:
https://github.com/railsadminteam/rails_admin/runs/4995149973?check_suite_focus=true
What I found was that Rails 6.1 with JRuby doesn't enable the Zeitwerk by default, which results in the value of
Rails.autoloaders.onceto be nil.https://github.com/rails/rails/blob/6-1-stable/railties/lib/rails/autoloaders.rb#L19-L26
So my fix here is to call
do_not_eager_loadonly if Zeitwerk is enabled. This will prevent stopping eager-load of files underapp/channels/turbo/in non-Zeitwerk environment, but for most of users it won't be a problem because they should have ActionCable loaded in a normal Rails setup. Or if there's a better implementation I'd be glad to adopt it.Thanks in advance!