diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index e65100d94..a6153f043 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 6 ## main +* Raise errors encountered while computing a component's cache digest instead of degrading to an untracked component that can serve stale fragments. + + *Erik Axel Nielsen* + * Give `<% cache %>` blocks inside a component's own template a digest, for components that `include ViewComponent::ExperimentallyCacheable`. Rails digests the virtual path of whichever template is rendering. Inside a component that path resolves to no template, because component templates aren't in the view paths, so the Digestor returned an empty digest and the fragment was never invalidated. The only signal was a `Couldn't find template for digesting` line in the log. 4.15.0 fixed the case where the `cache` block wraps the component in a view. This fixes the case where the block sits in the component's template. diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 0af337a16..5cbcfe3f0 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -204,6 +204,10 @@ The same works in a template, where the branch is often the more natural place f Declared components must include `ViewComponent::ExperimentallyCacheable` themselves, since a component that hasn't opted in has no digest to depend on. +## When a digest can't be computed + +Computing a digest touches the autoloader, the filesystem, and Action View's dependency trackers, any of which can fail. ViewComponent lets those errors raise, matching Rails' digest behavior. Otherwise a component could become untracked and serve stale fragments without warning. + ## Caveats **Self-caching components can't take content from their callers.** Besides a block, this covers `with_content` and slots set by the caller: diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index 98694c8dc..87fa35ed0 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -146,9 +146,6 @@ def partial_paths_in(source, name) RENDER_PARSER.new(name, source).render_calls.uniq.select do |path| source.include?(path) || source.include?(path.sub(%r{(\A|/)_}, '\1')) end - rescue - # Never let digest computation break rendering. - [] end # Action View has shipped its render parser as a class (Rails 7.1, and @@ -234,9 +231,6 @@ def constantize_component(constant_name) return unless component.respond_to?(:__vc_cacheable?) && component.__vc_cacheable? component - rescue - # Never let digest computation break rendering. - nil end end diff --git a/lib/view_component/cache_digest/dependency_tracking.rb b/lib/view_component/cache_digest/dependency_tracking.rb index 737616dde..f6100c0bb 100644 --- a/lib/view_component/cache_digest/dependency_tracking.rb +++ b/lib/view_component/cache_digest/dependency_tracking.rb @@ -27,11 +27,6 @@ def find_dependencies(name, template, view_paths = nil) end dependencies + CacheDigest.dependencies_in(template) - rescue - # A broken digest is preferable to a broken render. Falling back to the - # dependencies Rails found on its own means the component simply isn't - # tracked, which is the pre-existing behavior. - super end # @private diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb index eca6198bf..6ad9e9dc7 100644 --- a/lib/view_component/cache_digest/resolver.rb +++ b/lib/view_component/cache_digest/resolver.rb @@ -30,11 +30,6 @@ def find_templates(name, prefix, partial, details, locals = []) return [] unless component [build_template(component, virtual_path, details)] - rescue - # Never let digest resolution break rendering. Returning no template - # makes the Digestor treat this as a missing node, which degrades to - # the behavior components have without this feature. - [] end def to_s diff --git a/test/sandbox/app/components/cacheable_raising_ruby_dependency_component.rb b/test/sandbox/app/components/cacheable_raising_ruby_dependency_component.rb new file mode 100644 index 000000000..bf5445e59 --- /dev/null +++ b/test/sandbox/app/components/cacheable_raising_ruby_dependency_component.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class CacheableRaisingRubyDependencyComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + def call + render CacheDigestFixtures::RaisingRubyDependency.new + end +end diff --git a/test/sandbox/app/components/cacheable_raising_template_dependency_component.html.erb b/test/sandbox/app/components/cacheable_raising_template_dependency_component.html.erb new file mode 100644 index 000000000..8cd72058b --- /dev/null +++ b/test/sandbox/app/components/cacheable_raising_template_dependency_component.html.erb @@ -0,0 +1 @@ +<%= render CacheDigestFixtures::RaisingTemplateDependency.new %> diff --git a/test/sandbox/app/components/cacheable_raising_template_dependency_component.rb b/test/sandbox/app/components/cacheable_raising_template_dependency_component.rb new file mode 100644 index 000000000..29e3f2b08 --- /dev/null +++ b/test/sandbox/app/components/cacheable_raising_template_dependency_component.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class CacheableRaisingTemplateDependencyComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable +end diff --git a/test/sandbox/app/components/cacheable_unreadable_digest_source_component.rb b/test/sandbox/app/components/cacheable_unreadable_digest_source_component.rb new file mode 100644 index 000000000..19db7d5dd --- /dev/null +++ b/test/sandbox/app/components/cacheable_unreadable_digest_source_component.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class CacheableUnreadableDigestSourceComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + def call + "unreadable digest source" + end +end diff --git a/test/sandbox/app/components/cacheable_unreadable_digest_source_component.yml/placeholder b/test/sandbox/app/components/cacheable_unreadable_digest_source_component.yml/placeholder new file mode 100644 index 000000000..2e61bb449 --- /dev/null +++ b/test/sandbox/app/components/cacheable_unreadable_digest_source_component.yml/placeholder @@ -0,0 +1 @@ +This directory is intentionally unreadable as a digest source. diff --git a/test/sandbox/config/application.rb b/test/sandbox/config/application.rb index a8310d2c3..65c9082fd 100644 --- a/test/sandbox/config/application.rb +++ b/test/sandbox/config/application.rb @@ -44,6 +44,7 @@ class Application < Rails::Application # Prepare test_set_no_duplicate_autoload_paths config.autoload_paths.push("#{config.root}/my/components/previews") + config.autoload_paths.push("#{config.root}/test/fixtures") config.view_component.previews.paths << "#{config.root}/my/components/previews" config.view_component.previews.paths << "#{Rails.root}/lib/component_previews" diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index cf77f4d42..2db444fad 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -41,6 +41,24 @@ def test_cache_digest_is_computable_outside_a_request refute_empty digest end + def test_cache_digest_raises_when_a_ruby_dependency_fails_to_load + error = assert_raises(RuntimeError) { CacheableRaisingRubyDependencyComponent.cache_digest } + + assert_equal "raising Ruby dependency", error.message + end + + def test_cache_digest_raises_when_a_template_dependency_fails_to_load + error = assert_raises(RuntimeError) { CacheableRaisingTemplateDependencyComponent.cache_digest } + + assert_equal "raising template dependency", error.message + end + + def test_cache_digest_raises_when_a_digest_source_cannot_be_read + error = assert_raises(Errno::EISDIR) { CacheableUnreadableDigestSourceComponent.cache_digest } + + assert_includes error.message, "cacheable_unreadable_digest_source_component.yml" + end + def test_cache_digest_changes_when_the_template_changes assert_digest_changes( "app/components/cacheable_component.html.erb", @@ -203,12 +221,6 @@ def test_partial_paths_are_not_extracted_from_sources_without_render assert_empty ViewComponent::CacheDigest.partial_paths_in("def call; end", "a/b") end - def test_partial_path_extraction_swallows_parser_errors - ViewComponent::CacheDigest::RENDER_PARSER.stub(:new, ->(*) { raise "boom" }) do - assert_empty ViewComponent::CacheDigest.partial_paths_in("render \"a/b\"", "a/b") - end - end - # Action View has shipped the parser as a class (7.1, main) and as a module # with a `Default` implementation (7.2 through 8.1). Exercised with doubles so # both shapes are covered whichever version is running. @@ -479,37 +491,6 @@ def test_resolver_is_identified_by_class assert_equal resolver, ViewComponent::CacheDigest::Resolver.new end - def test_resolver_returns_no_template_when_synthesis_fails - resolver = ViewComponent::CacheDigest::Resolver.instance - - ViewComponent::CacheDigest.stub(:component_for, ->(_) { raise "boom" }) do - assert_empty resolver.find_templates("cacheable_component", "view_component/cache_digest", true, {}) - end - end - - def test_dependency_tracking_falls_back_when_scanning_fails - template = build_template("<%= render CacheableComponent.new(title: 'a') %>") - - ViewComponent::CacheDigest.stub(:dependencies_in, ->(_) { raise "boom" }) do - refute_includes( - ActionView::DependencyTracker.find_dependencies("some/template", template, []), - "view_component/cache_digest/cacheable_component" - ) - end - end - - def test_constantizing_swallows_unexpected_errors - Object.const_set(:BoomComponent, Class.new do - def self.__vc_cacheable? - raise ArgumentError - end - end) - - assert_nil ViewComponent::CacheDigest.send(:constantize_component, "BoomComponent") - ensure - Object.send(:remove_const, :BoomComponent) - end - def test_install_is_idempotent resolver_count = ActionController::Base.view_paths.count { |path| path.is_a?(ViewComponent::CacheDigest::Resolver) } diff --git a/test/sandbox/test/fixtures/cache_digest_fixtures/raising_ruby_dependency.rb b/test/sandbox/test/fixtures/cache_digest_fixtures/raising_ruby_dependency.rb new file mode 100644 index 000000000..cbdc86b12 --- /dev/null +++ b/test/sandbox/test/fixtures/cache_digest_fixtures/raising_ruby_dependency.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +raise "raising Ruby dependency" diff --git a/test/sandbox/test/fixtures/cache_digest_fixtures/raising_template_dependency.rb b/test/sandbox/test/fixtures/cache_digest_fixtures/raising_template_dependency.rb new file mode 100644 index 000000000..3a53bd73f --- /dev/null +++ b/test/sandbox/test/fixtures/cache_digest_fixtures/raising_template_dependency.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +raise "raising template dependency"