diff --git a/Rakefile b/Rakefile index ef45ecd55..19e2df166 100644 --- a/Rakefile +++ b/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 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1d20235cc..3fa443e9e 100644 --- a/docs/CHANGELOG.md +++ b/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* diff --git a/lib/view_component/engine.rb b/lib/view_component/engine.rb index a0a350498..d2ea1bea3 100644 --- a/lib/view_component/engine.rb +++ b/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! } diff --git a/test/isolated/template_caching_test.rb b/test/isolated/template_caching_test.rb new file mode 100644 index 000000000..7db61f540 --- /dev/null +++ b/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", "
Hello
") + + 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 "
Hello
", request.get("/").body.strip + + write_file(root, "app/components/greeting_component.html.erb", "
Goodbye
") + + assert_equal "
Hello
", 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 diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index 014297a18..628dc92d7 100644 --- a/test/sandbox/test/integration_test.rb +++ b/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", "
Goodbye world!
" 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" diff --git a/test/test_helper.rb b/test/test_helper.rb index 3c7d69ba7..490d2fcc2 100644 --- a/test/test_helper.rb +++ b/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