Skip to content

Commit b085ece

Browse files
erikaxelclaude
andcommitted
Resolve declared component dependencies that never opted in
`# Template Dependency: SomeComponent` is the escape hatch for component renders static analysis can't see -- a class held in a local variable, or reached through a helper module. `explicit_component_dependencies` translated the declared class name into the synthetic path the component is digested under, but only for classes `constantize_component` accepts, which means only ones that already include `ExperimentallyCacheable`. For anything else the raw constant name stayed in the dependency list and was handed to the Digestor as a template path. Nothing resolves there, so the declaration silently did nothing and the only signal was `Couldn't find template for digesting: SomeComponent` in the log. That defeats the point of the hatch: the dependencies you need it for are exactly the ones whose target you may not control or want to modify. Resolve any `ViewComponent::Base` descendant instead, and register it so the Resolver can synthesize a template for it -- being named by a declaration is itself the opt-in. `component_for` resolves through the same path, since the registry can now hold components that never included the module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2e6e02d commit b085ece

5 files changed

Lines changed: 79 additions & 6 deletions

File tree

‎docs/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ nav_order: 6
1010

1111
## main
1212

13+
* Resolve `# Template Dependency: SomeComponent` declarations naming a component that hasn't included `ViewComponent::ExperimentallyCacheable`.
14+
15+
The declaration was previously dropped: the class name was emitted to the Digestor verbatim, which looked for a template at that path, found none, and logged `Couldn't find template for digesting: SomeComponent`. That made the escape hatch unusable for exactly the dependencies it exists for, since a component reached through a local variable or a helper module is often one you don't control and can't add the module to. Naming a component in a declaration now digests it, opted in or not.
16+
17+
*Erik Axel Nielsen*
18+
1319
## 4.15.0
1420

1521
* Add experimental caching support, opt-in per component via `include ViewComponent::ExperimentallyCacheable`.

‎docs/guide/caching.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ The same works in a template, where the branch is often the more natural place f
187187
<%= render component.new(post: @post) %>
188188
```
189189

190-
Declared components must include `ViewComponent::ExperimentallyCacheable` themselves, since a component that hasn't opted in has no digest to depend on.
190+
A declared component doesn't have to include `ViewComponent::ExperimentallyCacheable` itself. The declaration alone gets its template, Ruby class, sidecar files, and superclasses digested, which matters because the components this escape hatch exists for are often ones you don't control.
191191

192192
## Caveats
193193

‎lib/view_component/cache_digest.rb‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def component_for(virtual_path)
9797
name = registry[virtual_path.delete_prefix("#{VIRTUAL_PATH_PREFIX}/")]
9898
return unless name
9999

100-
constantize_component(name)
100+
constantize_view_component(name)
101101
end
102102

103103
# Scan a template's source for renders of cacheable components.
@@ -168,6 +168,10 @@ def resolve_render_parser(parser)
168168
# that detail out of application code, so `SomeComponent` is translated
169169
# to the path the Digestor can resolve.
170170
#
171+
# Any component resolves here, not just ones that opted into caching. The
172+
# hatch exists for dependencies the source scanner can't see, which are
173+
# exactly the ones whose target the application may not control.
174+
#
171175
# @return [Array<Array(String, String)>] pairs of declared name and
172176
# synthetic virtual path
173177
def explicit_component_dependencies(source)
@@ -176,8 +180,18 @@ def explicit_component_dependencies(source)
176180
source.scan(EXPLICIT_DEPENDENCY).flatten.uniq.filter_map do |declared|
177181
next unless /\A(?:::)?[A-Z]/.match?(declared)
178182

179-
component = constantize_component(declared)
180-
[declared, virtual_path_for(component)] if component
183+
component = constantize_view_component(declared)
184+
next unless component
185+
186+
virtual_path = virtual_path_for(component)
187+
next unless virtual_path
188+
189+
# Being named by a declaration is what makes a component resolvable:
190+
# the Resolver synthesizes templates from the registry, so a component
191+
# that never opted in has to be added to it before the Digestor asks.
192+
register(component)
193+
194+
[declared, virtual_path]
181195
end
182196
end
183197

@@ -238,6 +252,20 @@ def constantize_component(constant_name)
238252
# Never let digest computation break rendering.
239253
nil
240254
end
255+
256+
# Resolve a constant name to a component, whether or not it opted into
257+
# caching.
258+
#
259+
# @return [Class, nil]
260+
def constantize_view_component(constant_name)
261+
component = constant_name.safe_constantize
262+
return unless component.is_a?(Class) && component < ViewComponent::Base
263+
264+
component
265+
rescue
266+
# Never let digest computation break rendering.
267+
nil
268+
end
241269
end
242270

243271
# Resolved once at load time rather than memoized, so no class-level state
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# frozen_string_literal: true
2+
3+
# Renders a component that static analysis can't see and that hasn't opted into
4+
# caching, declared with the `# Template Dependency:` escape hatch.
5+
class CacheablePlainDependencyComponent < ViewComponent::Base
6+
include ViewComponent::ExperimentallyCacheable
7+
8+
# Template Dependency: ErbComponent
9+
10+
def initialize(component: ErbComponent)
11+
@component = component
12+
end
13+
14+
def call
15+
render @component.new(message: "plain")
16+
end
17+
end

‎test/sandbox/test/experimentally_cacheable_test.rb‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,35 @@ def test_declared_template_paths_are_left_alone
121121
assert_includes dependencies, "integration_examples/erb_partial"
122122
end
123123

124-
def test_declared_names_that_are_not_cacheable_components_are_left_alone
124+
def test_declared_names_that_are_not_components_are_left_alone
125125
assert_empty ViewComponent::CacheDigest.explicit_component_dependencies(
126-
"# Template Dependency: ErbComponent"
126+
"# Template Dependency: NotAConstantAnywhere"
127+
)
128+
assert_empty ViewComponent::CacheDigest.explicit_component_dependencies(
129+
"# Template Dependency: ActiveSupport::Digest"
127130
)
128131
assert_empty ViewComponent::CacheDigest.explicit_component_dependencies("no declarations here")
129132
end
130133

134+
# The hatch exists for dependencies static analysis can't see, which are
135+
# exactly the ones whose target an application may not control. Declaring a
136+
# component is the opt-in, so the target needn't include the module itself.
137+
def test_declared_components_resolve_without_opting_into_caching
138+
refute_respond_to ErbComponent, :__vc_cacheable?
139+
140+
assert_equal(
141+
[["ErbComponent", "view_component/cache_digest/erb_component"]],
142+
ViewComponent::CacheDigest.explicit_component_dependencies("# Template Dependency: ErbComponent")
143+
)
144+
end
145+
146+
def test_cache_digest_changes_when_a_component_declared_without_opting_in_changes
147+
assert_digest_changes(
148+
"app/components/erb_component.html.erb",
149+
"<div>changed</div>\n"
150+
) { CacheablePlainDependencyComponent.cache_digest }
151+
end
152+
131153
# Components rendered from an inline template are invisible to Action View's
132154
# trackers, which only read template files.
133155
def test_cache_digest_changes_when_a_child_of_an_inline_template_changes

0 commit comments

Comments
 (0)