diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 36f2da188..0d321d2eb 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,12 @@ nav_order: 6 ## main +* Track discovered components without requiring each dependency to include `ViewComponent::ExperimentallyCacheable`, once experimental caching is enabled. + + Literal renders and `# Template Dependency:` declarations register the component's actual class name and virtual path, including acronym names and custom paths. Changes to unopted-in dependencies now invalidate enclosing fragments. Component output caching and digest-aware `cache` blocks inside component templates remain opt-in, and unexpected digest errors continue to raise. + + *Erik Axel Nielsen, Joel Hawksley* + * Invalidate Action View's memoized template digests when a component registers with `ViewComponent::CacheDigest`, so a digest computed before the component loaded isn't served for the rest of the process. *Erik Axel Nielsen* diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 5cbcfe3f0..e8575e1e4 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -28,7 +28,7 @@ Editing `PostComponent`'s template, Ruby class, or sidecar files doesn't invalid ## Opting in -Include `ViewComponent::ExperimentallyCacheable` in each component that should participate in caching: +Include `ViewComponent::ExperimentallyCacheable` in a component to enable experimental caching: ```ruby class PostComponent < ViewComponent::Base @@ -42,6 +42,10 @@ end That's all that's needed for the `<% cache %>` block above to work. The component is registered with Rails' digest tree, and the fragment is invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change, including components and partials rendered from an inline template or a `#call` method. +Once any component in the application includes the module, dependency tracking discovers components throughout digested views and their render trees, even when those components don't include the module. This includes transitively rendered components, components with acronym names, and components that override `virtual_path`. Applications that never include the module are unaffected. + +Discovery only tracks source dependencies. It doesn't cache a component's output or make `cache` blocks inside its own template digest-aware. Those still require the opt-ins described below. Dynamic renders still need explicit dependency declarations. + ## Caching inside a component template A `<% cache %>` block written inside a component's own template has the same problem, for the same reason: Rails digests the template that's rendering, and a component's template isn't in the view paths, so there's nothing to digest. @@ -202,7 +206,7 @@ The same works in a template, where the branch is often the more natural place f <%= render component.new(post: @post) %> ``` -Declared components must include `ViewComponent::ExperimentallyCacheable` themselves, since a component that hasn't opted in has no digest to depend on. +Once experimental caching is enabled, declared components don't need to include `ViewComponent::ExperimentallyCacheable` themselves. The declaration registers the component's actual class name and virtual path, so its template, Ruby class, sidecar files, superclasses, and discoverable dependencies are digested too. ## When a digest can't be computed diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index 4db6bc48c..4557ffaeb 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -21,9 +21,10 @@ module ViewComponent # not just its template. # # This module fixes both, reusing Rails' own `ActionView::Digestor` rather than - # reimplementing static analysis. Components opt in individually by including - # `ViewComponent::ExperimentallyCacheable`; until at least one component does, - # every hook here short-circuits. + # reimplementing static analysis. Until at least one component includes + # `ViewComponent::ExperimentallyCacheable`, dependency discovery short-circuits. + # Once enabled, every discovered component is tracked, whether or not it + # included the module. # # @private module CacheDigest @@ -56,8 +57,8 @@ module CacheDigest # Rails' escape hatch for dependencies static analysis can't see. EXPLICIT_DEPENDENCY = /#\s*Template Dependency:\s*(\S+)/ class << self - # Virtual paths of components that have opted into caching, mapped to - # their class names. + # Virtual paths of opted-in and discovered components, mapped to their + # class names. # # Class *names* rather than class objects so the registry survives # autoloader reloads without pinning stale constants in memory. @@ -108,7 +109,7 @@ def component_for(virtual_path) constantize_component(name) end - # Scan a template's source for renders of cacheable components. + # Scan a template's source for renders of components. # # Called for every template Rails digests, so it exits early when the # feature is unused. @@ -121,15 +122,15 @@ def dependencies_in(template) end # Scan arbitrary source (a template or a component's Ruby file) for - # renders of cacheable components. + # renders of components. # # @return [Array] synthetic virtual paths def component_paths_in(source) + return [] unless enabled? return [] unless source.is_a?(String) && source.include?("render") source.scan(RENDER_CALL).flatten.uniq.filter_map do |constant_name| - component = constantize_component(constant_name) - virtual_path_for(component) if component + registered_path_for(constantize_component(constant_name)) end end @@ -176,13 +177,15 @@ def resolve_render_parser(parser) # @return [Array] pairs of declared name and # synthetic virtual path def explicit_component_dependencies(source) + return [] unless enabled? return [] unless source.is_a?(String) && source.include?("Template Dependency:") source.scan(EXPLICIT_DEPENDENCY).flatten.uniq.filter_map do |declared| next unless /\A(?:::)?[A-Z]/.match?(declared) component = constantize_component(declared) - [declared, virtual_path_for(component)] if component + virtual_path = registered_path_for(component) + [declared, virtual_path] if virtual_path end end @@ -228,6 +231,15 @@ def install! private + # Record the actual class name rather than trying to reverse a virtual + # path, which may contain acronyms or be overridden by the component. + def registered_path_for(component) + return unless component + + register(component) + virtual_path_for(component) + end + # Drop Action View's memoized template digests, leaving its resolver # caches alone: no template changed, only the set of dependencies the # Digestor can see. @@ -235,15 +247,14 @@ def expire_digests ActionView::LookupContext::DetailsKey.digest_caches.each(&:clear) end - # Resolve a constant name to a component that opted into caching. + # Resolve a constant name to any ViewComponent. # # Returns nil for anything else, including constants that don't exist. # Autoloading here is safe: the template is about to render this constant # anyway. def constantize_component(constant_name) component = constant_name.safe_constantize - return unless component.is_a?(Class) - return unless component.respond_to?(:__vc_cacheable?) && component.__vc_cacheable? + return unless component.is_a?(Class) && component < ViewComponent::Base component end diff --git a/lib/view_component/experimentally_cacheable.rb b/lib/view_component/experimentally_cacheable.rb index 3a87be6a7..062d0a30d 100644 --- a/lib/view_component/experimentally_cacheable.rb +++ b/lib/view_component/experimentally_cacheable.rb @@ -15,7 +15,8 @@ module ViewComponent # `<% cache %>` block is invalidated when the component's template, Ruby # class, sidecar files, or child components change. This covers blocks # wrapping the component in a view and blocks inside the component's own - # template. + # template. Once enabled, dependency tracking also discovers components + # that haven't included this module. # 2. Enables the `cache_on` macro, which caches the component's own rendered # output. # @@ -186,7 +187,7 @@ def render_in(view_context, **, &block) if (cached = store.read(key)) # Safe to mark as HTML-safe: the cached string was produced by this same # rendering pipeline, which escapes output before it's written. - return cached.html_safe # rubocop:disable Rails/OutputSafety + return cached.html_safe end super.tap do |output| diff --git a/test/sandbox/app/components/cacheable_plain_dependency_component.rb b/test/sandbox/app/components/cacheable_plain_dependency_component.rb new file mode 100644 index 000000000..f5f482276 --- /dev/null +++ b/test/sandbox/app/components/cacheable_plain_dependency_component.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CacheablePlainDependencyComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + # Template Dependency: ErbComponent + + def initialize(component: ErbComponent) + @component = component + end + + def call + render @component.new(message: "plain") + end +end diff --git a/test/sandbox/app/components/cacheable_raising_explicit_dependency_component.rb b/test/sandbox/app/components/cacheable_raising_explicit_dependency_component.rb new file mode 100644 index 000000000..9dd0ae832 --- /dev/null +++ b/test/sandbox/app/components/cacheable_raising_explicit_dependency_component.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class CacheableRaisingExplicitDependencyComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + # Template Dependency: CacheDigestFixtures::RaisingRubyDependency + + def call + "unreachable" + end +end diff --git a/test/sandbox/app/components/cacheable_untracked_parent_component.html.erb b/test/sandbox/app/components/cacheable_untracked_parent_component.html.erb new file mode 100644 index 000000000..6898f8e50 --- /dev/null +++ b/test/sandbox/app/components/cacheable_untracked_parent_component.html.erb @@ -0,0 +1 @@ +
<%= render UntrackedChildComponent.new %>
diff --git a/test/sandbox/app/components/cacheable_untracked_parent_component.rb b/test/sandbox/app/components/cacheable_untracked_parent_component.rb new file mode 100644 index 000000000..a3c800012 --- /dev/null +++ b/test/sandbox/app/components/cacheable_untracked_parent_component.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class CacheableUntrackedParentComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable +end diff --git a/test/sandbox/app/components/format_sensitive_cacheable_component.rb b/test/sandbox/app/components/format_sensitive_cacheable_component.rb index f37bb016a..f3337fbb3 100644 --- a/test/sandbox/app/components/format_sensitive_cacheable_component.rb +++ b/test/sandbox/app/components/format_sensitive_cacheable_component.rb @@ -8,7 +8,7 @@ class FormatSensitiveCacheableComponent < ViewComponent::Base cache_on :identity def call - view_context.lookup_context.formats.first.to_s.html_safe # rubocop:disable Rails/OutputSafety + view_context.lookup_context.formats.first.to_s.html_safe end private diff --git a/test/sandbox/app/components/http_untracked_component.html.erb b/test/sandbox/app/components/http_untracked_component.html.erb new file mode 100644 index 000000000..a0f6a85b3 --- /dev/null +++ b/test/sandbox/app/components/http_untracked_component.html.erb @@ -0,0 +1 @@ +HTTP diff --git a/test/sandbox/app/components/http_untracked_component.rb b/test/sandbox/app/components/http_untracked_component.rb new file mode 100644 index 000000000..a259067b5 --- /dev/null +++ b/test/sandbox/app/components/http_untracked_component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class HTTPUntrackedComponent < ViewComponent::Base +end diff --git a/test/sandbox/app/components/positional_cache_key_component.rb b/test/sandbox/app/components/positional_cache_key_component.rb index 7ef549109..629d25744 100644 --- a/test/sandbox/app/components/positional_cache_key_component.rb +++ b/test/sandbox/app/components/positional_cache_key_component.rb @@ -13,7 +13,7 @@ def initialize(first:, second:) end def call - "#{first}-#{second}".html_safe # rubocop:disable Rails/OutputSafety + "#{first}-#{second}".html_safe end private diff --git a/test/sandbox/app/components/untracked_child_component.html.erb b/test/sandbox/app/components/untracked_child_component.html.erb new file mode 100644 index 000000000..7c6719c9e --- /dev/null +++ b/test/sandbox/app/components/untracked_child_component.html.erb @@ -0,0 +1 @@ +untracked<%= render HTTPUntrackedComponent.new %> diff --git a/test/sandbox/app/components/untracked_child_component.rb b/test/sandbox/app/components/untracked_child_component.rb new file mode 100644 index 000000000..d07a10665 --- /dev/null +++ b/test/sandbox/app/components/untracked_child_component.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class UntrackedChildComponent < ViewComponent::Base + self.virtual_path = "custom/untracked_child" +end diff --git a/test/sandbox/app/views/integration_examples/cached_untracked_component.html.erb b/test/sandbox/app/views/integration_examples/cached_untracked_component.html.erb new file mode 100644 index 000000000..eacbad1b0 --- /dev/null +++ b/test/sandbox/app/views/integration_examples/cached_untracked_component.html.erb @@ -0,0 +1,4 @@ +<% cache "cached-untracked-component-fragment" do %> + <%= render CacheableComponent.new(title: "cached") %> + <%= render UntrackedChildComponent.new %> +<% end %> diff --git a/test/sandbox/config/application.rb b/test/sandbox/config/application.rb index 65c9082fd..d01622174 100644 --- a/test/sandbox/config/application.rb +++ b/test/sandbox/config/application.rb @@ -52,6 +52,8 @@ class Application < Rails::Application end end +Rails.autoloaders.main.inflector.inflect("http_untracked_component" => "HTTPUntrackedComponent") + Sandbox::Application.config.secret_key_base = "foo" # Don't silence library backtraces in test reports diff --git a/test/sandbox/config/routes.rb b/test/sandbox/config/routes.rb index c29f6e4b3..d0d7c5073 100644 --- a/test/sandbox/config/routes.rb +++ b/test/sandbox/config/routes.rb @@ -28,6 +28,7 @@ get :cached_partial, to: "integration_examples#cached_partial" get :cached_component, to: "integration_examples#cached_component" get :cached_nested_component, to: "integration_examples#cached_nested_component" + get :cached_untracked_component, to: "integration_examples#cached_untracked_component" get :cache_block_component, to: "integration_examples#cache_block_component" get :inherited_sidecar, to: "integration_examples#inherited_sidecar" get :inherited_from_uncompilable_component, to: "integration_examples#inherited_from_uncompilable_component" diff --git a/test/sandbox/test/experimentally_cacheable_integration_test.rb b/test/sandbox/test/experimentally_cacheable_integration_test.rb index b1d152248..81c24545c 100644 --- a/test/sandbox/test/experimentally_cacheable_integration_test.rb +++ b/test/sandbox/test/experimentally_cacheable_integration_test.rb @@ -76,6 +76,38 @@ def test_cache_block_digest_is_unaffected_by_unrelated_components end end + def test_cache_block_is_invalidated_when_an_untracked_component_changes + get "/cached_untracked_component" + assert_select(".untracked-child", text: "untracked") + + before = fragment_digest_for("integration_examples/cached_untracked_component") + + modify_file "app/components/untracked_child_component.html.erb", "changed\n" do + clear_digest_cache + + refute_equal before, fragment_digest_for("integration_examples/cached_untracked_component") + with_new_cache do + get "/cached_untracked_component" + + assert_select(".untracked-child", text: "changed") + end + end + end + + def test_cache_block_is_invalidated_when_a_transitive_untracked_component_changes + get "/cached_untracked_component" + assert_select(".http-untracked", text: "HTTP") + + modify_file "app/components/http_untracked_component.html.erb", "changed\n" do + clear_digest_cache + with_new_cache do + get "/cached_untracked_component" + + assert_select(".http-untracked", text: "changed") + end + end + end + def test_renders_a_cache_block_held_by_a_component_template get "/cache_block_component" diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index ebf6cd10c..fd7bc3fd0 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -79,6 +79,12 @@ def test_cache_digest_raises_when_a_template_dependency_fails_to_load assert_equal "raising template dependency", error.message end + def test_cache_digest_raises_when_an_explicit_dependency_fails_to_load + error = assert_raises(RuntimeError) { CacheableRaisingExplicitDependencyComponent.cache_digest } + + assert_equal "raising Ruby dependency", error.message + end + def test_cache_digest_raises_when_a_digest_source_cannot_be_read error = assert_raises(Errno::EISDIR) { CacheableUnreadableDigestSourceComponent.cache_digest } @@ -117,6 +123,32 @@ def test_cache_digest_changes_when_a_child_component_ruby_file_changes ) { CacheableParentComponent.cache_digest } end + def test_cache_digest_changes_when_an_untracked_child_template_changes + assert_digest_changes( + "app/components/untracked_child_component.html.erb", + "changed\n" + ) { CacheableUntrackedParentComponent.cache_digest } + end + + def test_cache_digest_changes_when_an_untracked_child_ruby_file_changes + original = File.read(Rails.root.join("app/components/untracked_child_component.rb")) + + assert_digest_changes( + "app/components/untracked_child_component.rb", + original + "\n# a comment\n" + ) { CacheableUntrackedParentComponent.cache_digest } + end + + def test_cache_digest_changes_when_a_transitive_untracked_dependency_changes + refute_respond_to UntrackedChildComponent, :__vc_cacheable? + refute_respond_to HTTPUntrackedComponent, :__vc_cacheable? + + assert_digest_changes( + "app/components/http_untracked_component.html.erb", + "changed\n" + ) { CacheableUntrackedParentComponent.cache_digest } + end + def test_cache_digest_changes_when_a_superclass_template_changes assert_digest_changes( "app/components/cacheable_component.html.erb", @@ -165,13 +197,56 @@ def test_declared_template_paths_are_left_alone assert_includes dependencies, "integration_examples/erb_partial" end - def test_declared_names_that_are_not_cacheable_components_are_left_alone + def test_declared_names_that_are_not_components_are_left_alone + assert_empty ViewComponent::CacheDigest.explicit_component_dependencies( + "# Template Dependency: NotAConstantAnywhere" + ) assert_empty ViewComponent::CacheDigest.explicit_component_dependencies( - "# Template Dependency: ErbComponent" + "# Template Dependency: ActiveSupport::Digest" ) assert_empty ViewComponent::CacheDigest.explicit_component_dependencies("no declarations here") end + def test_declared_components_resolve_without_opting_into_caching + refute_respond_to ErbComponent, :__vc_cacheable? + + with_registry("cacheable_component" => "CacheableComponent") do + template = build_template("<%# Template Dependency: ErbComponent %>") + dependencies = ActionView::DependencyTracker.find_dependencies("test/template", template, []) + + assert_includes dependencies, "view_component/cache_digest/erb_component" + refute_includes dependencies, "ErbComponent" + assert_equal ErbComponent, ViewComponent::CacheDigest.component_for("view_component/cache_digest/erb_component") + end + end + + def test_cache_digest_changes_when_a_declared_untracked_component_changes + assert_digest_changes( + "app/components/erb_component.html.erb", + "
changed
\n" + ) { CacheablePlainDependencyComponent.cache_digest } + end + + def test_declared_components_register_acronym_names_and_custom_virtual_paths + with_registry("cacheable_component" => "CacheableComponent") do + dependencies = ViewComponent::CacheDigest.explicit_component_dependencies( + "# Template Dependency: HTTPUntrackedComponent\n# Template Dependency: UntrackedChildComponent" + ) + + assert_equal( + [ + ["HTTPUntrackedComponent", "view_component/cache_digest/http_untracked_component"], + ["UntrackedChildComponent", "view_component/cache_digest/custom/untracked_child"] + ], + dependencies + ) + assert_equal HTTPUntrackedComponent, + ViewComponent::CacheDigest.component_for("view_component/cache_digest/http_untracked_component") + assert_equal UntrackedChildComponent, + ViewComponent::CacheDigest.component_for("view_component/cache_digest/custom/untracked_child") + end + end + # Components rendered from an inline template are invisible to Action View's # trackers, which only read template files. def test_cache_digest_changes_when_a_child_of_an_inline_template_changes @@ -505,8 +580,47 @@ def test_dependencies_are_found_for_component_renders ) end - def test_dependencies_ignore_components_that_did_not_opt_in - assert_empty ViewComponent::CacheDigest.dependencies_in(build_template("<%= render ErbComponent.new(message: 'a') %>")) + def test_dependencies_are_found_for_components_that_did_not_opt_in + assert_equal( + ["view_component/cache_digest/erb_component"], + ViewComponent::CacheDigest.dependencies_in(build_template("<%= render ErbComponent.new(message: 'a') %>")) + ) + end + + def test_discovered_components_register_acronym_names_and_custom_virtual_paths + with_registry("cacheable_component" => "CacheableComponent") do + dependencies = ViewComponent::CacheDigest.dependencies_in( + build_template("<%= render HTTPUntrackedComponent.new %><%= render UntrackedChildComponent.new %>") + ) + + assert_equal( + ["view_component/cache_digest/http_untracked_component", "view_component/cache_digest/custom/untracked_child"], + dependencies + ) + assert_equal "HTTPUntrackedComponent", ViewComponent::CacheDigest.registry["http_untracked_component"] + assert_equal "UntrackedChildComponent", ViewComponent::CacheDigest.registry["custom/untracked_child"] + assert_equal HTTPUntrackedComponent, + ViewComponent::CacheDigest.component_for("view_component/cache_digest/http_untracked_component") + assert_equal UntrackedChildComponent, + ViewComponent::CacheDigest.component_for("view_component/cache_digest/custom/untracked_child") + refute_respond_to HTTPUntrackedComponent, :__vc_caches_output? + refute_includes UntrackedChildComponent.ancestors, ViewComponent::ExperimentallyCacheable + end + end + + def test_dependencies_ignore_constants_that_are_not_components + assert_empty ViewComponent::CacheDigest.dependencies_in(build_template("<%= render IntegrationExamplesController %>")) + end + + def test_dependency_discovery_does_not_enable_caching_without_opt_in + with_registry({}) do + source = "<%# Template Dependency: UntrackedChildComponent %><%= render HTTPUntrackedComponent.new %>" + + assert_empty ViewComponent::CacheDigest.dependencies_in(build_template(source)) + assert_empty ViewComponent::CacheDigest.component_paths_in(source) + assert_empty ViewComponent::CacheDigest.explicit_component_dependencies(source) + assert_empty ViewComponent::CacheDigest.registry + end end def test_resolver_is_identified_by_class @@ -530,6 +644,15 @@ def test_install_is_idempotent private + def with_registry(entries) + saved = ViewComponent::CacheDigest.registry.dup + ViewComponent::CacheDigest.registry.replace(entries) + yield + ensure + ViewComponent::CacheDigest.registry.replace(saved) + clear_digest_cache + end + # `with_new_cache` compiles components against whatever is on disk, then # restores the previous compile cache on exit. A component compiled while its # template was modified is therefore still registered as compiled, and keeps diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index 6e53e8749..9a119688a 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -581,7 +581,7 @@ def test_raise_error_when_variant_template_files_collide assert_includes( error.message, - "Colliding templates 'mini-watch' and 'mini__watch' found in VariantTemplatesCollisionComponent." \ + "Colliding templates 'mini-watch' and 'mini__watch' found in VariantTemplatesCollisionComponent." ) end