Skip to content

Commit d64933f

Browse files
authored
Merge pull request #2770 from ViewComponent/joelhawksley-fix-interpolated-render-dependencies
Fix unresolvable interpolated render dependencies
2 parents 00042b8 + c5fedb1 commit d64933f

5 files changed

Lines changed: 109 additions & 4 deletions

File tree

‎docs/CHANGELOG.md‎

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

1111
## main
1212

13+
* Avoid missing-template digest errors for unresolvable interpolated render paths in components and included helpers.
14+
15+
*Joel Hawksley*
16+
1317
* Cover shared helper dependencies in the boot benchmark and add a cold cache digest benchmark to the CI performance check.
1418

1519
*Joel Hawksley*

‎lib/view_component/cache_digest.rb‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,20 +457,29 @@ def partial_paths_in(source, name)
457457
# Dynamic renders can return components without naming their classes,
458458
# such as `render UI.button`. Keep Action View's guess only if it resolves,
459459
# without executing application methods to discover what they return.
460+
# Interpolated paths are discarded if Rails cannot expand their wildcards.
460461
def unresolved_partials_rendered_by(calls, name, view_paths)
461-
return [] unless view_paths
462-
463462
targets = calls.flat_map { |call| render_targets(call) }
464463
literal, dynamic = targets.partition do |target|
465464
target.is_a?(Prism::StringNode) || target.is_a?(Prism::InterpolatedStringNode)
466465
end
467-
return [] if dynamic.empty?
466+
interpolated = literal.grep(Prism::InterpolatedStringNode)
467+
unresolved = []
468+
if interpolated.any?
469+
paths = partial_paths_in(interpolated.map { |target| "render(#{target.slice})" }.join("\n"), name)
470+
# Rails expands only directory wildcards with a literal prefix.
471+
# Older parsers can return truncated paths without any wildcard.
472+
unresolved = paths.reject { |path| path.match?(%r{\A[^*]+/\*\z}) }
473+
static = literal.grep(Prism::StringNode)
474+
unresolved -= partial_paths_in(static.map { |target| "render(#{target.slice})" }.join("\n"), name)
475+
end
476+
return unresolved unless view_paths && dynamic.any?
468477

469478
inferred = partial_paths_in(dynamic.map { |target| "render(#{target.slice})" }.join("\n"), name)
470479
declared = partial_paths_in(literal.map { |target| "render(#{target.slice})" }.join("\n"), name)
471480
finder = ActionView::LookupContext.new(view_paths, variants: :any, locale: I18n.available_locales)
472481

473-
inferred.reject do |path|
482+
unresolved | inferred.reject do |path|
474483
declared.include?(path) || finder.exists?(path, [], true)
475484
end
476485
end

‎lib/view_component/cache_digest/dependency_tracking.rb‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ def find_dependencies(name, template, view_paths = nil)
3535

3636
component_paths, inferred_partials =
3737
CacheDigest.template_renders_in(CacheDigest.compiled_source(template), name, view_paths: view_paths)
38+
# Older ERB trackers omit the underscore on unqualified partial paths.
39+
inferred_partials |= inferred_partials.map { |path| path.delete_prefix("_") }
3840
declared = source.scan(CacheDigest::EXPLICIT_DEPENDENCY).flatten
3941

4042
(dependencies - (inferred_partials - declared) + component_paths).uniq

‎test/sandbox/test/cacheable_test.rb‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,40 @@ def test_existing_dynamic_partial_dependencies_are_kept
11371137
)
11381138
end
11391139

1140+
def test_unresolvable_interpolated_partial_dependencies_are_discarded
1141+
sources = [
1142+
"render(\"\#{path}/\#{association}_fields\")",
1143+
"render partial: \"\#{path}/\#{association}_fields\"",
1144+
"render_to_string \"\#{path}/\#{association}_fields\"",
1145+
"render \"buttons/\#{kind}_fields\"",
1146+
"render \"\#{path}/\#{name}\"",
1147+
"render \"\#{kind}_fields\""
1148+
]
1149+
1150+
each_render_tracker do
1151+
sources.each do |source|
1152+
dependencies = ActionView::DependencyTracker.find_dependencies(
1153+
"test/template", build_template("<%= #{source} %>"), []
1154+
)
1155+
1156+
# Older ERB trackers leave interpolation markers, which the digestor skips.
1157+
assert_empty dependencies.reject { |path| path.include?("#") }, source
1158+
[nil, []].each do |view_paths|
1159+
assert_equal [[], []], ViewComponent::CacheDigest.renders_in(source, "test/template", view_paths: view_paths), source
1160+
end
1161+
end
1162+
end
1163+
end
1164+
1165+
def test_literal_wildcards_are_not_discarded_with_matching_interpolated_paths
1166+
source = "render \"buttons/*_fields\"; render \"buttons/\#{kind}_fields\""
1167+
1168+
assert_equal(
1169+
[[], ["buttons/*_fields"]],
1170+
ViewComponent::CacheDigest.renders_in(source, "test/template", view_paths: [])
1171+
)
1172+
end
1173+
11401174
def test_existing_dynamic_partials_still_change_the_digest
11411175
digests = %w[before after].map do |content|
11421176
clear_digest_cache

‎test/sandbox/test/template_regression_test.rb‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,62 @@ def test_render_parent_preserves_child_fragment_cache_identity
170170
end
171171
end
172172

173+
def test_precomputation_does_not_log_unresolvable_interpolated_helper_dependencies
174+
with_component_files(
175+
"interpolated_helper.rb" => <<~'RUBY',
176+
module TemplateRegressionFixtures::InterpolatedHelper
177+
def nested_fields(path, association)
178+
render("#{path}/#{association}_fields")
179+
end
180+
end
181+
RUBY
182+
"base_component.rb" => <<~RUBY,
183+
class TemplateRegressionFixtures::BaseComponent < ViewComponent::Base
184+
include TemplateRegressionFixtures::InterpolatedHelper
185+
end
186+
RUBY
187+
"greeting_component.rb" => "class TemplateRegressionFixtures::GreetingComponent < TemplateRegressionFixtures::BaseComponent; end",
188+
"greeting_component.html.erb" => "<p>Hello</p>",
189+
"control_component.rb" => "class TemplateRegressionFixtures::ControlComponent < ViewComponent::Base; end",
190+
"control_component.html.erb" => "<%= render TemplateRegressionFixtures::GreetingComponent.new %>"
191+
) do
192+
components = [TemplateRegressionFixtures::GreetingComponent, TemplateRegressionFixtures::ControlComponent]
193+
components.each(&:__vc_compile)
194+
195+
ActionView::Digestor.logger.stub(:error, ->(message) { flunk message }) do
196+
ViewComponent::CacheDigest.precompute!(components)
197+
end
198+
199+
refute_includes components.first.__vc_cache_digest_source, "*/*_fields"
200+
assert_includes components.last.__vc_cache_digest_source,
201+
ViewComponent::CacheDigest.virtual_path_for(components.first)
202+
end
203+
end
204+
205+
def test_supported_interpolated_partial_edits_change_component_digest
206+
parser = ViewComponent::CacheDigest::RENDER_PARSER.new("test/template", "render \"buttons/\#{kind}\"")
207+
unless parser.render_calls.any? { |path| path.gsub(%r{/_}, "/") == "buttons/*" }
208+
skip "Action View's render parser does not support interpolated directory wildcards"
209+
end
210+
211+
with_component_files(
212+
"wildcard_component.rb" => "class TemplateRegressionFixtures::WildcardComponent < ViewComponent::Base; end",
213+
"wildcard_component.html.erb" => "<%= render \"buttons/\#{kind}\" %>",
214+
"views/buttons/_button.html.erb" => "original partial"
215+
) do |directory|
216+
component = TemplateRegressionFixtures::WildcardComponent
217+
component.__vc_cache_digest_source = ViewComponent::CacheDigest::Resolver.instance.source_for(component).freeze
218+
resolver = ActionView::FileSystemResolver.new(File.join(directory, "views"))
219+
before = component.cache_digest(finder: ActionView::LookupContext.new([resolver]))
220+
221+
File.write(File.join(directory, "views/buttons/_button.html.erb"), "changed partial")
222+
resolver.clear_cache
223+
clear_digest_cache
224+
225+
refute_equal before, component.cache_digest(finder: ActionView::LookupContext.new([resolver]))
226+
end
227+
end
228+
173229
def test_view_fragments_track_components_assigned_to_locals
174230
with_component_files(
175231
"leaf_component.rb" => "class TemplateRegressionFixtures::LeafComponent < ViewComponent::Base; end",

0 commit comments

Comments
 (0)