Revise how to disable eager loading app/channels

[fxn]

Background

If the parent application does not use Action Cable, turbo-rails prevents app/channels from being eager loaded, since some necessary classes do not exist.

The root issue

The interface of config.eager_load_paths and friends has always been ill-defined.

For example, config.autoload_paths is empty, while config.eager_load_paths is not. This is confusing, don't you think? In any case, only pushing to these collections was documented, and pushing ended up doing what it was expected, unless you also used the paths API, in which case you'd hit a long-standing bug that was fixed in Rails 7.1.2.

Bottom line, deletion from those collections was not something you could rely on. It worked on this collection by pure chance, and would have no effect in others. This interface did not exist, basically. And the fix in Rails 7.1.2 not only revises the logic, but also clarifies the contract for these collections.

The patch

In order to be able to not eager load app/channels, we leave that deletion for classic because it works and that won't change in old releases. But for applications loading with Zeitwerk, we use the interface that is documented to work.

Note that Rails.autoloaders.zeitwerk_enabled? still exists today, even if Zeitwerk has been the only autoloader since Rails 7. And it does so precisely for cases like this in which engines need to support multiple Rails versions.

Testing

Unfortunately, there are no tests for this. I only tried manually. Being able to test this would need work in the test harness of turbo-rails itself. There is more conditional code in the initializers to be tested. You'd need different dummy apps, and for this particular case, test/test_helper.rb should not assume the ActionCable constant is defined, or should be refactored somehow.

But that is out of the scope of this PR. This patch fixes the issue to be able to ship a new version of turbo-rails, and I'll leave work on an improved test harness to the Hotwire team.

Fixes #512.

[seanpdoyle]

There is more conditional code in the initializers to be tested. You'd need different dummy apps, and for this particular case, test/test_helper.rb should not assume the ActionCable constant is defined, or should be refactored somehow.

But that is out of the scope of this PR.

@fxn I've opened up #514. Does that achieve what you're describing?

[fxn]

@seanpdoyle in principle that might work.

However, what is really needed is a proof that an application that does not have Action Cable loaded does not eager load app/cache. And CI would test that for Rails 7 and Rails 6, say.

Same for the branch guarded by Rails.application.config.respond_to?(:assets).

Without knowing more about the project, I'd refactor that test_helper.rb depending on whether that eases writing those missing tests or not.

[ghiculescu]

Is this okay to merge? Otherwise I think it will break tests for any new Rails app that doesn't use Action Cable.