Merge pull request #2728 from mhenrixon/register-compile-cache-hook-only-without-template-caching

Only register the compile cache executor hook without template caching

Author: joelhawksley

Rakefile

@@ -14,7 +14,13 @@ RSpec::Core::RakeTask.new(:spec)
 Rake::TestTask.new(:test) do |t|
   t.libs << "test"
   t.libs << "lib"
-  t.test_files = FileList["test/**/*_test.rb"]
+  t.test_files = FileList["test/**/*_test.rb"].exclude("test/isolated/**/*")
+end
+
+# Each test boots its own application, so these can't share a process with the sandbox.
+Rake::TestTask.new(:isolated_test) do |t|
+  t.libs << "lib"
+  t.test_files = FileList["test/isolated/**/*_test.rb"]
 end
 
 Rake::TestTask.new(:engine_test) do |t|
@@ -176,6 +182,10 @@ task :all_tests do
   Rake::Task["test"].invoke
   puts
   puts
+  puts "Running isolated Minitests"
+  Rake::Task["isolated_test"].invoke
+  puts
+  puts
   puts "Running Minitests for Rails Engine compatibility"
   Rake::Task["engine_test"].invoke
   puts

docs/CHANGELOG.md

@@ -10,6 +10,10 @@ nav_order: 6
 
 ## main
 
+* Only register the executor hook that watches component templates when Action View doesn't cache templates at boot. With template caching on, as in production, the hook did nothing but still read `ActionView::Base.cache_template_loading` on every request. That value lives in a class variable, which non-main Ractors can't read on Ruby 4.0, so under Rails' experimental `Rails.application.ractorize!` every request failed.
+
+    *Mikael Henriksson*
+
 * Fix `Component.translate` raising `UncaughtThrowError` for keys missing from the sidecar translations, and `NoMethodError` for components without sidecar translations. It now falls back to the global translations, like the instance method does. Fix `translate` returning the `default:` value instead of an existing global translation.
 
     *Andrii Furmanets*

lib/view_component/engine.rb

@@ -132,13 +132,19 @@ class Engine < Rails::Engine # :nodoc:
         end
       end
 
-      template_watcher = nil
-
-      app.executor.to_run :before do
+      # Like Rails' own template cache expiry, only watch templates when they
+      # aren't cached at boot. With template caching on, as in production, no
+      # hook runs per request, so requests never read ActionView's caching
+      # class variable, which non-main Ractors can't access.
+      ActiveSupport.on_load(:action_view) do
         next if ActionView::Base.cache_template_loading
 
-        template_watcher ||= CompileCache.template_watcher(app.config.file_watcher)
-        template_watcher.execute_if_updated
+        template_watcher = nil
+
+        app.executor.to_run :before do
+          template_watcher ||= CompileCache.template_watcher(app.config.file_watcher)
+          template_watcher.execute_if_updated
+        end
       end
 
       app.reloader.before_class_unload { CompileCache.invalidate! }

test/isolated/template_caching_test.rb

@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require "bundler/setup"
+require "minitest/autorun"
+require "minitest/mock"
+require "tmpdir"
+require "rails"
+require "action_controller/railtie"
+require "active_support/testing/isolation"
+require "view_component"
+
+# Boots a fresh application per test in a forked process, so tests can choose
+# their own environment. Runs separately from the sandbox, which boots in test.
+class TemplateCachingTest < ActiveSupport::TestCase
+  include ActiveSupport::Testing::Isolation
+
+  def test_executor_checks_for_template_changes_when_templates_are_not_cached
+    app = boot_app("development")
+    refute ActionView::Base.cache_template_loading
+
+    watcher = Minitest::Mock.new
+    watcher.expect(:execute_if_updated, nil)
+    ViewComponent::CompileCache.stub(:template_watcher, watcher) { app.executor.wrap {} }
+
+    watcher.verify
+  end
+
+  # Non-main Ractors can't read ActionView::Resolver's class variable on Ruby 4.0,
+  # so the executor must not read it on every request when templates are cached.
+  def test_executor_does_nothing_when_templates_are_cached
+    app = boot_app("production")
+    assert ActionView::Base.cache_template_loading
+
+    reads = 0
+    ActionView::Resolver.singleton_class.prepend(Module.new do
+      define_method(:caching?) do
+        reads += 1
+        super()
+      end
+    end)
+    ViewComponent::CompileCache.stub(:template_watcher, ->(*) { flunk "templates were watched" }) { app.executor.wrap {} }
+
+    assert_equal 0, reads
+  end
+
+  def test_template_changes_are_not_reflected_on_new_request_when_templates_are_cached
+    Dir.mktmpdir do |root|
+      write_file(root, "app/components/greeting_component.rb", "class GreetingComponent < ViewComponent::Base; end")
+      write_file(root, "app/components/greeting_component.html.erb", "<div>Hello</div>")
+
+      app = boot_app("production", root: root)
+      app.routes.draw do
+        get "/", to: ->(_env) { [200, {"content-type" => "text/html"}, [ActionController::Base.render(GreetingComponent.new)]] }
+      end
+      request = Rack::MockRequest.new(app)
+
+      assert_equal "<div>Hello</div>", request.get("/").body.strip
+
+      write_file(root, "app/components/greeting_component.html.erb", "<div>Goodbye</div>")
+
+      assert_equal "<div>Hello</div>", request.get("/").body.strip
+    end
+  end
+
+  private
+
+  def boot_app(env, root: __dir__)
+    Rails.env = env
+
+    Class.new(Rails::Application) do
+      config.root = root
+      config.eager_load = env == "production"
+      config.enable_reloading = env == "development"
+      config.secret_key_base = "test"
+      config.logger = Logger.new(nil)
+    end.tap(&:initialize!)
+  end
+
+  def write_file(root, path, content)
+    FileUtils.mkdir_p(File.dirname(File.join(root, path)))
+    File.write(File.join(root, path), content)
+  end
+end

test/sandbox/test/integration_test.rb

@@ -50,24 +50,6 @@ def test_rendering_component_in_a_controller_with_block
     assert_response :success
   end
 
-  def test_template_changes_are_not_reflected_on_new_request_when_cache_template_loading_is_true
-    with_template_caching do
-      get "/controller_inline"
-      assert_select("div", "bar")
-      assert_response :success
-
-      modify_file "app/components/controller_inline_component.html.erb", "<div>Goodbye world!</div>" do
-        get "/controller_inline"
-        assert_select("div", "bar")
-        assert_response :success
-      end
-
-      get "/controller_inline"
-      assert_select("div", "bar")
-      assert_response :success
-    end
-  end
-
   def test_template_changes_are_reflected_on_new_request_when_cache_template_loading_is_false
     with_new_cache do
       get "/controller_inline"

test/test_helper.rb

@@ -128,15 +128,6 @@ def with_generate_sidecar(enabled, &block)
   with_generate_option(:sidecar, enabled, &block)
 end
 
-def with_template_caching
-  old_cache_template_loading = ActionView::Base.cache_template_loading
-  ActionView::Base.cache_template_loading = true
-
-  yield
-ensure
-  ActionView::Base.cache_template_loading = old_cache_template_loading
-end
-
 def with_new_cache
   old_cache = ViewComponent::CompileCache.cache
   ViewComponent::CompileCache.cache = Set.new