diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f48c475aa..df9bdfb8b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -28,6 +28,10 @@ nav_order: 5 ## 3.16.0 +* Fix development mode race condition that caused an invalid duplicate template error. + + *Blake Williams* + * Add template information to multiple template error messages. *Joel Hawksley* diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 8e6163acc..70e39516a 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -30,7 +30,12 @@ def compile(raise_errors: false, force: false) @component.superclass.compile(raise_errors: raise_errors) end - return if gather_template_errors(raise_errors).any? + if template_errors.present? + raise TemplateError.new(template_errors) if raise_errors + + # this return is load bearing, and prevents the component from being considered "compiled?" + return false + end if raise_errors @component.validate_initialization_parameters! @@ -98,70 +103,70 @@ def render_template_for(variant = nil, format = nil) end end - def gather_template_errors(raise_errors) - errors = [] + def template_errors + @_template_errors ||= begin + errors = [] - errors << "Couldn't find a template file or inline render method for #{@component}." if @templates.empty? + errors << "Couldn't find a template file or inline render method for #{@component}." if @templates.empty? - # We currently allow components to have both an inline call method and a template for a variant, with the - # inline call method overriding the template. We should aim to change this in v4 to instead - # raise an error. - @templates.reject(&:inline_call?) - .map { |template| [template.variant, template.format] } - .tally - .select { |_, count| count > 1 } - .each do |tally| - variant, this_format = tally.first + # We currently allow components to have both an inline call method and a template for a variant, with the + # inline call method overriding the template. We should aim to change this in v4 to instead + # raise an error. + @templates.reject(&:inline_call?) + .map { |template| [template.variant, template.format] } + .tally + .select { |_, count| count > 1 } + .each do |tally| + variant, this_format = tally.first - variant_string = " for variant `#{variant}`" if variant.present? - - errors << "More than one #{this_format.upcase} template found#{variant_string} for #{@component}. " - end + variant_string = " for variant `#{variant}`" if variant.present? - default_template_types = @templates.each_with_object(Set.new) do |template, memo| - next if template.variant + errors << "More than one #{this_format.upcase} template found#{variant_string} for #{@component}. " + end - memo << :template_file if !template.inline_call? - memo << :inline_render if template.inline_call? && template.defined_on_self? + default_template_types = @templates.each_with_object(Set.new) do |template, memo| + next if template.variant - memo - end + memo << :template_file if !template.inline_call? + memo << :inline_render if template.inline_call? && template.defined_on_self? - if default_template_types.length > 1 - errors << - "Template file and inline render method found for #{@component}. " \ - "There can only be a template file or inline render method per component." - end + memo + end - # If a template has inline calls, they can conflict with template files the component may use - # to render. This attempts to catch and raise that issue before run time. For example, - # `def render_mobile` would conflict with a sidecar template of `component.html+mobile.erb` - duplicate_template_file_and_inline_call_variants = - @templates.reject(&:inline_call?).map(&:variant) & - @templates.select { _1.inline_call? && _1.defined_on_self? }.map(&:variant) - - unless duplicate_template_file_and_inline_call_variants.empty? - count = duplicate_template_file_and_inline_call_variants.count - - errors << - "Template #{"file".pluralize(count)} and inline render #{"method".pluralize(count)} " \ - "found for #{"variant".pluralize(count)} " \ - "#{duplicate_template_file_and_inline_call_variants.map { |v| "'#{v}'" }.to_sentence} " \ - "in #{@component}. There can only be a template file or inline render method per variant." - end + if default_template_types.length > 1 + errors << + "Template file and inline render method found for #{@component}. " \ + "There can only be a template file or inline render method per component." + end - @templates.select(&:variant).each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |template, memo| - memo[template.normalized_variant_name] << template.variant - memo - end.each do |_, variant_names| - next unless variant_names.length > 1 + # If a template has inline calls, they can conflict with template files the component may use + # to render. This attempts to catch and raise that issue before run time. For example, + # `def render_mobile` would conflict with a sidecar template of `component.html+mobile.erb` + duplicate_template_file_and_inline_call_variants = + @templates.reject(&:inline_call?).map(&:variant) & + @templates.select { _1.inline_call? && _1.defined_on_self? }.map(&:variant) + + unless duplicate_template_file_and_inline_call_variants.empty? + count = duplicate_template_file_and_inline_call_variants.count + + errors << + "Template #{"file".pluralize(count)} and inline render #{"method".pluralize(count)} " \ + "found for #{"variant".pluralize(count)} " \ + "#{duplicate_template_file_and_inline_call_variants.map { |v| "'#{v}'" }.to_sentence} " \ + "in #{@component}. There can only be a template file or inline render method per variant." + end - errors << "Colliding templates #{variant_names.sort.map { |v| "'#{v}'" }.to_sentence} found in #{@component}." - end + @templates.select(&:variant).each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |template, memo| + memo[template.normalized_variant_name] << template.variant + memo + end.each do |_, variant_names| + next unless variant_names.length > 1 - raise TemplateError.new(errors, @templates) if errors.any? && raise_errors + errors << "Colliding templates #{variant_names.sort.map { |v| "'#{v}'" }.to_sentence} found in #{@component}." + end - errors + errors + end end def gather_templates diff --git a/lib/view_component/errors.rb b/lib/view_component/errors.rb index 5b59f6db9..e8cb8b10e 100644 --- a/lib/view_component/errors.rb +++ b/lib/view_component/errors.rb @@ -20,12 +20,6 @@ class TemplateError < StandardError def initialize(errors, templates = nil) message = errors.join("\n") - if templates - message << "\n" - message << "Templates:\n" - message << templates.map(&:inspect).join("\n") - end - super(message) end end diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index b06bcf118..afd4e1ca7 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -512,11 +512,6 @@ def test_raise_error_when_variant_template_file_and_inline_variant_call_exist end end - assert_includes( - error.message, - "ViewComponent::Template:" - ) - assert_includes( error.message, "Template file and inline render method found for variant 'phone' in " \