Repository navigation
Conversation
The executor hook that clears the compile cache before each request did nothing when Action View caches templates, as in production, but it still read `ActionView::Base.cache_template_loading` on every request. That reads `ActionView::Resolver`'s `@@caching` class variable, which non-main Ractors can't read on Ruby 4.0, so under Rails' experimental `Rails.application.ractorize!` every request failed once view_component was loaded. Decide once, when Action View loads, whether to register the hook. Rails' Action View railtie likewise registers its view reloader only when template caching is off. The hook still checks the flag on each run, so toggling it at runtime, with caching off at boot, works as before.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
Rails main has an experimental
Rails.application.ractorize!that serves requests from non-main Ractors on Ruby 4.0. Once view_component is loaded under it, every request fails, including requests that don't render a component:The executor hook in
engine.rbruns before every request and readsActionView::Base.cache_template_loading, which isActionView::Resolver's@@cachingclass variable. In production that flag is true, so the hook does nothing, but it still reads the flag.What approach did you choose and why?
The hook is now registered only if template caching is off when Action View loads. Inside
ActiveSupport.on_load(:action_view), Rails' ownaction_view.cachinghook has already set the flag, so this sees the app's real setting. Rails' Action View railtie makes the same decision for its view reloader: it registers it only when templates aren't cached.Development and test don't change. The hook is registered there, and it still checks the flag on each run, so
with_template_caching/with_new_cachetoggling it at runtime still work. I checked this: if the hook is never registered, the two..._when_cache_template_loading_is_falseintegration tests fail.I considered recomputing the flag from
config.action_view.cache_template_loadingandreloading_enabled?inafter_initialize, the way Rails' railtie does. I didn't, because it copies Rails' defaulting logic, misses apps that setActionView::Base.cache_template_loadingthemselves, and leaves anelseline the sandbox never runs.Anything you want to highlight for special attention from reviewers?
test/sandbox/test/initializers/engine_test.rbboots a minimal production app in a subprocess, like the Rakefile'seager_load_check, because the sandbox boots with template caching off. It countsActionView::Resolver.caching?calls duringexecutor.wrap {}. That count is 1 onmainand 0 with this change. Using a subprocess means it doesn't need Ractors and runs on every Ruby in the matrix.@__vc_compiler(base.rb:707, aCompilerholding aMutex), and after that comesCompileCache.cache(compile_cache.rb:7, amattr_accessor, so a class variable holding aSet). I'm happy to look at those separately.ActionView::Resolver.cachingis acattr_accessor. On Ruby master (4.1), non-main Ractors can read class variables that hold shareable values (see Make I18n locale switching work in non-main Ractors rails/rails#58974), and on 4.0 Rails may move it off a class variable. This change doesn't depend on either.Tests:
bundle exec rake test: Ruby 4.0.7 with Rails 8.1 and with Rails main gives 653 runs, 0 failures, 0 skips. Ruby 3.2.8 with Rails 7.1 gives 0 failures and 7 skips.rake eager_load_check engine_test spec(4.0.7/8.1) passes.audition . --static-only --plain: ok, 74 baselined, nothing new.I also checked it in a Rails main app under
ractorize!, with requests running in a non-main Ractor. Before:GET /postsfailed with the error above. After:GET /posts -> 200andGET /stack/plain-helpers -> 200.