From 698e9917a2ab3ba9fb5372c8179060c22e5ebd69 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 23 Aug 2024 13:19:31 -0600 Subject: [PATCH 001/151] add test case --- .../multiple_formats_component.css.erb | 1 + .../multiple_formats_component.html.erb | 1 + .../multiple_formats_component.json.erb | 1 + .../components/multiple_formats_component.rb | 4 ++++ .../integration_examples_controller.rb | 4 ++++ test/sandbox/config/routes.rb | 1 + test/sandbox/test/integration_test.rb | 18 ++++++++++++++++++ 7 files changed, 30 insertions(+) create mode 100644 test/sandbox/app/components/multiple_formats_component.css.erb create mode 100644 test/sandbox/app/components/multiple_formats_component.html.erb create mode 100644 test/sandbox/app/components/multiple_formats_component.json.erb create mode 100644 test/sandbox/app/components/multiple_formats_component.rb diff --git a/test/sandbox/app/components/multiple_formats_component.css.erb b/test/sandbox/app/components/multiple_formats_component.css.erb new file mode 100644 index 000000000..1ce0cba5f --- /dev/null +++ b/test/sandbox/app/components/multiple_formats_component.css.erb @@ -0,0 +1 @@ +Hello, CSS! \ No newline at end of file diff --git a/test/sandbox/app/components/multiple_formats_component.html.erb b/test/sandbox/app/components/multiple_formats_component.html.erb new file mode 100644 index 000000000..4d6b75bea --- /dev/null +++ b/test/sandbox/app/components/multiple_formats_component.html.erb @@ -0,0 +1 @@ +Hello, HTML! \ No newline at end of file diff --git a/test/sandbox/app/components/multiple_formats_component.json.erb b/test/sandbox/app/components/multiple_formats_component.json.erb new file mode 100644 index 000000000..82fe31da1 --- /dev/null +++ b/test/sandbox/app/components/multiple_formats_component.json.erb @@ -0,0 +1 @@ +Hello, JSON! \ No newline at end of file diff --git a/test/sandbox/app/components/multiple_formats_component.rb b/test/sandbox/app/components/multiple_formats_component.rb new file mode 100644 index 000000000..d6af286c1 --- /dev/null +++ b/test/sandbox/app/components/multiple_formats_component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class MultipleFormatsComponent < ViewComponent::Base +end diff --git a/test/sandbox/app/controllers/integration_examples_controller.rb b/test/sandbox/app/controllers/integration_examples_controller.rb index c7dfad3ba..cfdbb9a86 100644 --- a/test/sandbox/app/controllers/integration_examples_controller.rb +++ b/test/sandbox/app/controllers/integration_examples_controller.rb @@ -71,4 +71,8 @@ def unsafe_preamble_component def unsafe_postamble_component render(UnsafePostambleComponent.new) end + + def multiple_formats_component + render(MultipleFormatsComponent.new) + end end diff --git a/test/sandbox/config/routes.rb b/test/sandbox/config/routes.rb index 072cf9687..545acf639 100644 --- a/test/sandbox/config/routes.rb +++ b/test/sandbox/config/routes.rb @@ -32,6 +32,7 @@ get :unsafe_component, to: "integration_examples#unsafe_component" get :unsafe_preamble_component, to: "integration_examples#unsafe_preamble_component" get :unsafe_postamble_component, to: "integration_examples#unsafe_postamble_component" + get :multiple_formats_component, to: "integration_examples#multiple_formats_component" post :create, to: "integration_examples#create" constraints(lambda { |request| request.env["warden"].authenticate! }) do diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index c024deabf..3584bb8ad 100644 --- a/test/sandbox/test/integration_test.rb +++ b/test/sandbox/test/integration_test.rb @@ -752,4 +752,22 @@ def test_unsafe_postamble_component "Rendering UnsafePostambleComponent did not emit an HTML safety warning" ) end + + def test_renders_multiple_format_component_as_html + get "/multiple_formats_component" + + assert_includes response.body, "Hello, HTML!" + end + + def test_renders_multiple_format_component_as_json + get "/multiple_formats_component.json" + + assert_includes response.body, "Hello, JSON!" + end + + def test_renders_multiple_format_component_as_css + get "/multiple_formats_component.css" + + assert_includes response.body, "Hello, CSS!" + end end From 71f4adc881e2b0fc343cf2578e2fcad304b6b5fe Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 23 Aug 2024 16:06:22 -0600 Subject: [PATCH 002/151] two tests remaining --- lib/view_component/base.rb | 12 ++++++------ lib/view_component/compiler.rb | 29 ++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 905bdf13e..dafd398c1 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -107,7 +107,7 @@ def render_in(view_context, &block) if render? # Avoid allocating new string when output_preamble and output_postamble are blank - rendered_template = safe_render_template_for(@__vc_variant).to_s + rendered_template = safe_render_template_for(@__vc_variant, request.present? ? request.format.to_sym : nil).to_s if output_preamble.blank? && output_postamble.blank? rendered_template @@ -330,11 +330,11 @@ def maybe_escape_html(text) end end - def safe_render_template_for(variant) + def safe_render_template_for(variant, format = nil) if compiler.renders_template_for_variant?(variant) - render_template_for(variant) + render_template_for(variant, format) else - maybe_escape_html(render_template_for(variant)) do + maybe_escape_html(render_template_for(variant, format)) do Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.") end end @@ -519,12 +519,12 @@ def inherited(child) # meaning it will not be called for any children and thus not compile their templates. if !child.instance_methods(false).include?(:render_template_for) && !child.compiled? child.class_eval <<~RUBY, __FILE__, __LINE__ + 1 - def render_template_for(variant = nil) + def render_template_for(variant = nil, format = nil) # Force compilation here so the compiler always redefines render_template_for. # This is mostly a safeguard to prevent infinite recursion. self.class.compile(raise_errors: true, force: true) # .compile replaces this method; call the new one - render_template_for(variant) + render_template_for(variant, format) end RUBY end diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 384741f60..d45dcb83e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -61,14 +61,14 @@ def call component_class.silence_redefinition_of_method("render_template_for") component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def render_template_for(variant = nil) + def render_template_for(variant = nil, format = nil) _call_#{safe_class_name} end RUBY end else templates.each do |template| - method_name = call_method_name(template[:variant]) + method_name = call_method_name(template[:variant], template[:format]) @variants_rendering_templates << template[:variant] redefinition_lock.synchronize do @@ -127,7 +127,7 @@ def define_render_template_for redefinition_lock.synchronize do component_class.silence_redefinition_of_method(:render_template_for) component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def render_template_for(variant = nil) + def render_template_for(variant = nil, format = nil) #{body} end RUBY @@ -147,7 +147,7 @@ def template_errors errors << "Couldn't find a template file or inline render method for #{component_class}." end - if templates.count { |template| template[:variant].nil? } > 1 + if templates.map { |template| "#{template[:variant].inspect}_#{template[:format]}" }.tally.any? { |_, count| count > 1 } errors << "More than one template found for #{component_class}. " \ "There can only be one default template file per component." @@ -213,6 +213,7 @@ def templates pieces = File.basename(path).split(".") memo << { path: path, + format: pieces[1..-2].join(".").split("+").first&.to_sym, variant: pieces[1..-2].join(".").split("+").second&.to_sym, handler: pieces.last } @@ -239,6 +240,10 @@ def inline_calls_defined_on_self @inline_calls_defined_on_self ||= component_class.instance_methods(false).grep(/^call(_|$)/) end + def formats + @__vc_variants = (templates.map { |template| template[:format] }).compact.uniq + end + def variants @__vc_variants = ( templates.map { |template| template[:variant] } + variants_from_inline_calls(inline_calls) @@ -283,12 +288,18 @@ def compile_template(template, handler) # :nocov: end - def call_method_name(variant) - if variant.present? && variants.include?(variant) - "call_#{normalized_variant_name(variant)}" - else - "call" + def call_method_name(variant, format = nil) + out = +"call" + + if variant.present? + out << "_#{normalized_variant_name(variant)}" end + + if format.present? && format != :html && formats.length > 1 + out << "_#{format}" + end + + out end def normalized_variant_name(variant) From d356ce8334422105be2baf38697d9f19452d4116 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 23 Aug 2024 16:12:35 -0600 Subject: [PATCH 003/151] first time all green --- lib/view_component/compiler.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index d45dcb83e..c1bb0f435 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -106,6 +106,13 @@ def renders_template_for_variant?(variant) attr_reader :component_class, :redefinition_lock def define_render_template_for + template_elsifs = templates.map do |template| + safe_name = "_call_variant_#{normalized_variant_name(template[:variant])}_#{template[:format]}_#{safe_class_name}" + component_class.define_method(safe_name, component_class.instance_method(call_method_name(template[:variant], template[:format]))) + + "elsif variant == #{template[:variant].inspect} && format == #{template[:format].inspect}\n #{safe_name}" + end.join("\n") + variant_elsifs = variants.compact.uniq.map do |variant| safe_name = "_call_variant_#{normalized_variant_name(variant)}_#{safe_class_name}" component_class.define_method(safe_name, component_class.instance_method(call_method_name(variant))) @@ -116,7 +123,9 @@ def define_render_template_for component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call)) body = <<-RUBY - if variant.nil? + if false + #{template_elsifs} + elsif variant.nil? _call_#{safe_class_name} #{variant_elsifs} else From 10d68ee2cf299fc82849dd2c7c24e71125bd4150 Mon Sep 17 00:00:00 2001 From: GitHub Actions Bot <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 26 Aug 2024 14:23:39 +0000 Subject: [PATCH 004/151] Fix final line endings --- test/sandbox/app/components/multiple_formats_component.css.erb | 2 +- test/sandbox/app/components/multiple_formats_component.html.erb | 2 +- test/sandbox/app/components/multiple_formats_component.json.erb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/sandbox/app/components/multiple_formats_component.css.erb b/test/sandbox/app/components/multiple_formats_component.css.erb index 1ce0cba5f..c48de162a 100644 --- a/test/sandbox/app/components/multiple_formats_component.css.erb +++ b/test/sandbox/app/components/multiple_formats_component.css.erb @@ -1 +1 @@ -Hello, CSS! \ No newline at end of file +Hello, CSS! diff --git a/test/sandbox/app/components/multiple_formats_component.html.erb b/test/sandbox/app/components/multiple_formats_component.html.erb index 4d6b75bea..b18211d66 100644 --- a/test/sandbox/app/components/multiple_formats_component.html.erb +++ b/test/sandbox/app/components/multiple_formats_component.html.erb @@ -1 +1 @@ -Hello, HTML! \ No newline at end of file +Hello, HTML! diff --git a/test/sandbox/app/components/multiple_formats_component.json.erb b/test/sandbox/app/components/multiple_formats_component.json.erb index 82fe31da1..2803e7206 100644 --- a/test/sandbox/app/components/multiple_formats_component.json.erb +++ b/test/sandbox/app/components/multiple_formats_component.json.erb @@ -1 +1 @@ -Hello, JSON! \ No newline at end of file +Hello, JSON! From 229aaaba413ebb31f893a4619ae2d39b7ad33a9f Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 09:12:15 -0600 Subject: [PATCH 005/151] add docs, changelog, test helpers --- docs/CHANGELOG.md | 12 ++++++++++ docs/guide/testing.md | 13 +++++++++++ lib/view_component/test_helpers.rb | 23 +++++++++++++++++++ .../multiple_formats_component.json.erb | 1 - .../multiple_formats_component.json.jbuilder | 1 + test/sandbox/test/integration_test.rb | 2 +- test/sandbox/test/rendering_test.rb | 10 +++++++- 7 files changed, 59 insertions(+), 3 deletions(-) delete mode 100644 test/sandbox/app/components/multiple_formats_component.json.erb create mode 100644 test/sandbox/app/components/multiple_formats_component.json.jbuilder diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index dc03cd061..6bf22a708 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,18 @@ nav_order: 5 ## main +* Add support for request formats. + + *Joel Hawksley* + +* Add `rendered_json` test helper. + + *Joel Hawksley* + +* Add `with_format` test helper. + + *Joel Hawksley* + * Defer to built-in caching for language environment setup, rather than manually using `actions/cache` in CI. *Simon Fish* diff --git a/docs/guide/testing.md b/docs/guide/testing.md index 363a972b1..30935fbe4 100644 --- a/docs/guide/testing.md +++ b/docs/guide/testing.md @@ -135,6 +135,19 @@ def test_render_component_for_tablet end ``` +## Request formats + +Use the `with_format` helper to test specific request formats: + +```ruby +def test_render_component_as_json + with_format :json do + render_inline(MultipleFormatsComponent.new) + + assert_equal(rendered_json["hello"], "world") + end +end + ## Configuring the controller used in tests Since 2.27.0 diff --git a/lib/view_component/test_helpers.rb b/lib/view_component/test_helpers.rb index 04809bf1c..097d2cc38 100644 --- a/lib/view_component/test_helpers.rb +++ b/lib/view_component/test_helpers.rb @@ -63,6 +63,16 @@ def render_inline(component, **args, &block) Nokogiri::HTML.fragment(@rendered_content) end + # `JSON.parse`-d component output. + # + # ```ruby + # render_inline(MyJsonComponent.new) + # assert_equal(rendered_json["hello"], "world") + # ``` + def rendered_json + JSON.parse(rendered_content) + end + # Render a preview inline. Internally sets `page` to be a `Capybara::Node::Simple`, # allowing for Capybara assertions to be used: # @@ -155,6 +165,19 @@ def with_controller_class(klass) @vc_test_controller = old_controller end + # Set format of the current request + # + # ```ruby + # with_format(:json) do + # render_inline(MyComponent.new) + # end + # ``` + # + # @param format [Symbol] The format to be set for the provided block. + def with_format(format) + with_request_url("/", format: format) { yield } + end + # Set the URL of the current request (such as when using request-dependent path helpers): # # ```ruby diff --git a/test/sandbox/app/components/multiple_formats_component.json.erb b/test/sandbox/app/components/multiple_formats_component.json.erb deleted file mode 100644 index 2803e7206..000000000 --- a/test/sandbox/app/components/multiple_formats_component.json.erb +++ /dev/null @@ -1 +0,0 @@ -Hello, JSON! diff --git a/test/sandbox/app/components/multiple_formats_component.json.jbuilder b/test/sandbox/app/components/multiple_formats_component.json.jbuilder new file mode 100644 index 000000000..8a54a3de6 --- /dev/null +++ b/test/sandbox/app/components/multiple_formats_component.json.jbuilder @@ -0,0 +1 @@ +json.hello "world" \ No newline at end of file diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index e115a7cce..70bb11c6b 100644 --- a/test/sandbox/test/integration_test.rb +++ b/test/sandbox/test/integration_test.rb @@ -779,7 +779,7 @@ def test_renders_multiple_format_component_as_html def test_renders_multiple_format_component_as_json get "/multiple_formats_component.json" - assert_includes response.body, "Hello, JSON!" + assert_equal response.body, "{\"hello\":\"world\"}" end def test_renders_multiple_format_component_as_css diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index a478cbeea..c18f7e19d 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -151,7 +151,7 @@ def test_renders_haml_template end def test_render_jbuilder_template - with_request_url("/", format: :json) do + with_format(:json) do render_inline(JbuilderComponent.new(message: "bar")) { "foo" } end @@ -1195,4 +1195,12 @@ def test_use_helpers_macros_with_named_prefix assert_selector ".helper__named-prefix-message", text: "Hello macro named prefix helper method" end + + def test_with_format + with_format(:json) do + render_inline(MultipleFormatsComponent.new) + + assert_equal(rendered_json["hello"], "world") + end + end end From 9560ca099a613f0c4362430c2c2bd73c53a4deba Mon Sep 17 00:00:00 2001 From: GitHub Actions Bot <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 26 Aug 2024 15:12:53 +0000 Subject: [PATCH 006/151] Fix final line endings --- .../app/components/multiple_formats_component.json.jbuilder | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/sandbox/app/components/multiple_formats_component.json.jbuilder b/test/sandbox/app/components/multiple_formats_component.json.jbuilder index 8a54a3de6..880598ac2 100644 --- a/test/sandbox/app/components/multiple_formats_component.json.jbuilder +++ b/test/sandbox/app/components/multiple_formats_component.json.jbuilder @@ -1 +1 @@ -json.hello "world" \ No newline at end of file +json.hello "world" From 9640e676fda9c3270b9296eaaa604eda719c469c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 10:56:49 -0600 Subject: [PATCH 007/151] streamline compiler method generation --- lib/view_component/compiler.rb | 68 +++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index c1bb0f435..84f35baed 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -106,32 +106,64 @@ def renders_template_for_variant?(variant) attr_reader :component_class, :redefinition_lock def define_render_template_for - template_elsifs = templates.map do |template| - safe_name = "_call_variant_#{normalized_variant_name(template[:variant])}_#{template[:format]}_#{safe_class_name}" + branches = [] + + templates.each do |template| + safe_name = +"_call" + variant_name = normalized_variant_name(template[:variant]) + safe_name << "_#{variant_name}" if variant_name.present? + safe_name << "_#{template[:format]}" if template[:format].present? && template[:format] != :html + safe_name << "_#{safe_class_name}" + component_class.define_method(safe_name, component_class.instance_method(call_method_name(template[:variant], template[:format]))) - "elsif variant == #{template[:variant].inspect} && format == #{template[:format].inspect}\n #{safe_name}" - end.join("\n") + format_conditional = + if template[:format] == :html + "(format == :html || format.nil?)" + else + "format == #{template[:format].inspect}" + end + + variant_conditional = + if template[:variant].nil? + "variant.nil?" + else + "variant&.to_sym == :'#{template[:variant]}'" + end + + branches << ["#{variant_conditional} && #{format_conditional}", safe_name] + end - variant_elsifs = variants.compact.uniq.map do |variant| - safe_name = "_call_variant_#{normalized_variant_name(variant)}_#{safe_class_name}" + variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| + safe_name = "_call_#{normalized_variant_name(variant)}_#{safe_class_name}" component_class.define_method(safe_name, component_class.instance_method(call_method_name(variant))) - "elsif variant.to_sym == :'#{variant}'\n #{safe_name}" - end.join("\n") + branches << ["variant&.to_sym == :'#{variant}'", safe_name] + end - component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call)) + default_method_name = "_call_#{safe_class_name}" - body = <<-RUBY - if false - #{template_elsifs} - elsif variant.nil? - _call_#{safe_class_name} - #{variant_elsifs} - else - _call_#{safe_class_name} + component_class.define_method(:"#{default_method_name}", component_class.instance_method(:call)) + + # Just use default method name if no conditional branches or if there is a single + # conditional branch that just calls the default method_name + if branches.empty? || (branches.length == 1 && branches[0].last == default_method_name) + body = default_method_name + else + body = +"" + + branches.each_with_index do |(conditional, method_body), index| + next if method_body == default_method_name + + if index == 0 + body << "if #{conditional}\n #{method_body}\n" + else + body << "elsif #{conditional}\n #{method_body}\n" + end end - RUBY + + body << "else\n #{default_method_name}\nend" + end redefinition_lock.synchronize do component_class.silence_redefinition_of_method(:render_template_for) From 4bacfd9303ee49246197cfdc77511b6343198b3b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 11:07:07 -0600 Subject: [PATCH 008/151] simplification --- lib/view_component/compiler.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 84f35baed..1176a7f38 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -155,11 +155,7 @@ def define_render_template_for branches.each_with_index do |(conditional, method_body), index| next if method_body == default_method_name - if index == 0 - body << "if #{conditional}\n #{method_body}\n" - else - body << "elsif #{conditional}\n #{method_body}\n" - end + body << "#{index == 0 ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end body << "else\n #{default_method_name}\nend" From eee9aec574dd19e79accd3c3817fff1f23891697 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 11:08:40 -0600 Subject: [PATCH 009/151] refactor to remove index usage --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 1176a7f38..ae3516f32 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -152,10 +152,10 @@ def define_render_template_for else body = +"" - branches.each_with_index do |(conditional, method_body), index| + branches.each do |conditional, method_body| next if method_body == default_method_name - body << "#{index == 0 ? "if" : "elsif"} #{conditional}\n #{method_body}\n" + body << "#{!body.present? ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end body << "else\n #{default_method_name}\nend" From 1331e4136e46f981e2fe8159108de6fe37e6107e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 11:14:01 -0600 Subject: [PATCH 010/151] clearer control flow --- lib/view_component/compiler.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index ae3516f32..93b7d83cc 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -107,6 +107,7 @@ def renders_template_for_variant?(variant) def define_render_template_for branches = [] + default_method_name = "_call_#{safe_class_name}" templates.each do |template| safe_name = +"_call" @@ -115,7 +116,16 @@ def define_render_template_for safe_name << "_#{template[:format]}" if template[:format].present? && template[:format] != :html safe_name << "_#{safe_class_name}" - component_class.define_method(safe_name, component_class.instance_method(call_method_name(template[:variant], template[:format]))) + if safe_name == default_method_name + next + else + component_class.define_method( + safe_name, + component_class.instance_method( + call_method_name(template[:variant], template[:format]) + ) + ) + end format_conditional = if template[:format] == :html @@ -141,8 +151,6 @@ def define_render_template_for branches << ["variant&.to_sym == :'#{variant}'", safe_name] end - default_method_name = "_call_#{safe_class_name}" - component_class.define_method(:"#{default_method_name}", component_class.instance_method(:call)) # Just use default method name if no conditional branches or if there is a single @@ -153,8 +161,6 @@ def define_render_template_for body = +"" branches.each do |conditional, method_body| - next if method_body == default_method_name - body << "#{!body.present? ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end From ac048cb0b29ccbfc6e9f4d245afb9d3a44c81da5 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 11:20:46 -0600 Subject: [PATCH 011/151] lint --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 93b7d83cc..bc5e1d852 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -161,7 +161,7 @@ def define_render_template_for body = +"" branches.each do |conditional, method_body| - body << "#{!body.present? ? "if" : "elsif"} #{conditional}\n #{method_body}\n" + body << "#{(!body.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end body << "else\n #{default_method_name}\nend" From 7bbba5180abc2a2a301ebcdc18a5793570be69d7 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 12:46:32 -0600 Subject: [PATCH 012/151] md lint --- docs/guide/testing.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/guide/testing.md b/docs/guide/testing.md index 30935fbe4..dbff2646d 100644 --- a/docs/guide/testing.md +++ b/docs/guide/testing.md @@ -147,6 +147,7 @@ def test_render_component_as_json assert_equal(rendered_json["hello"], "world") end end +``` ## Configuring the controller used in tests From c44b5b343b3804476cb226c3f51e772ec520188a Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Aug 2024 16:52:36 -0600 Subject: [PATCH 013/151] remove remaining hardcoded formats --- lib/view_component/base.rb | 18 ------------------ lib/view_component/collection.rb | 7 ++++++- lib/view_component/compiler.rb | 25 ++++++++++++++++++------- lib/view_component/instrumentation.rb | 2 +- test/sandbox/test/integration_test.rb | 2 +- 5 files changed, 26 insertions(+), 28 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index dafd398c1..193395679 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -500,13 +500,6 @@ def with_collection(collection, **args) Collection.new(self, collection, **args) end - # Provide identifier for ActionView template annotations - # - # @private - def short_identifier - @short_identifier ||= defined?(Rails.root) ? source_location.sub("#{Rails.root}/", "") : source_location - end - # @private def inherited(child) # Compile so child will inherit compiled `call_*` template methods that @@ -586,17 +579,6 @@ def compiler @__vc_compiler ||= Compiler.new(self) end - # we'll eventually want to update this to support other types - # @private - def type - "text/html" - end - - # @private - def format - :html - end - # @private def identifier source_location diff --git a/lib/view_component/collection.rb b/lib/view_component/collection.rb index e602f46c9..e4082dc0d 100644 --- a/lib/view_component/collection.rb +++ b/lib/view_component/collection.rb @@ -7,7 +7,6 @@ class Collection include Enumerable attr_reader :component - delegate :format, to: :component delegate :size, to: :@collection attr_accessor :__vc_original_view_context @@ -41,6 +40,12 @@ def each(&block) components.each(&block) end + # Rails expects us to define `format` on all renderables, + # but we do not know the `format` of a ViewComponent until runtime. + def format + nil + end + private def initialize(component, object, **options) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index bc5e1d852..90e4eefcd 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -76,7 +76,7 @@ def render_template_for(variant = nil, format = nil) # rubocop:disable Style/EvalWithLocation component_class.class_eval <<-RUBY, template[:path], 0 def #{method_name} - #{compiled_template(template[:path])} + #{compiled_template(template[:path], template[:format])} end RUBY # rubocop:enable Style/EvalWithLocation @@ -306,25 +306,36 @@ def compiled_inline_template(template) compile_template(template, handler) end - def compiled_template(file_path) + def compiled_template(file_path, format) handler = ActionView::Template.handler_for_extension(File.extname(file_path).delete(".")) template = File.read(file_path) - compile_template(template, handler) + compile_template(template, handler, file_path, format) end - def compile_template(template, handler) + def compile_template(template, handler, identifier = component_class.source_location, format = :html) template.rstrip! if component_class.strip_trailing_whitespace? + short_identifier = defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier + type = ActionView::Template::Types[format] + if handler.method(:call).parameters.length > 1 - handler.call(component_class, template) + handler.call( + OpenStruct.new( + format: format, + identifier: identifier, + short_identifier: short_identifier, + type: type + ), + template + ) # :nocov: else handler.call( OpenStruct.new( source: template, - identifier: component_class.identifier, - type: component_class.type + identifier: identifier, + type: type ) ) end diff --git a/lib/view_component/instrumentation.rb b/lib/view_component/instrumentation.rb index d63efd283..41a37ba3e 100644 --- a/lib/view_component/instrumentation.rb +++ b/lib/view_component/instrumentation.rb @@ -13,7 +13,7 @@ def render_in(view_context, &block) notification_name, { name: self.class.name, - identifier: self.class.identifier + identifier: self.class.source_location } ) do super diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index 70bb11c6b..21139169d 100644 --- a/test/sandbox/test/integration_test.rb +++ b/test/sandbox/test/integration_test.rb @@ -19,7 +19,7 @@ def test_rendering_component_with_template_annotations_enabled get "/" assert_response :success - assert_includes response.body, "BEGIN app/components/erb_component.rb" + assert_includes response.body, "BEGIN app/components/erb_component.html.erb" assert_select("div", "Foo\n bar") end From e46c231f797dfdf52dc5588dec073726c85ede09 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 27 Aug 2024 12:06:55 -0600 Subject: [PATCH 014/151] Update lib/view_component/base.rb Co-authored-by: Blake Williams --- lib/view_component/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 193395679..9effabe3b 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -107,7 +107,7 @@ def render_in(view_context, &block) if render? # Avoid allocating new string when output_preamble and output_postamble are blank - rendered_template = safe_render_template_for(@__vc_variant, request.present? ? request.format.to_sym : nil).to_s + rendered_template = safe_render_template_for(@__vc_variant, request&.format&.to_sym).to_s if output_preamble.blank? && output_postamble.blank? rendered_template From 6fb636aae7a8f4950f1cb8bae86bf959523a7339 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 27 Aug 2024 12:37:08 -0600 Subject: [PATCH 015/151] remove unnecessary `inspect` --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 90e4eefcd..36426bf00 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -190,7 +190,7 @@ def template_errors errors << "Couldn't find a template file or inline render method for #{component_class}." end - if templates.map { |template| "#{template[:variant].inspect}_#{template[:format]}" }.tally.any? { |_, count| count > 1 } + if templates.map { |template| "#{template[:variant]}_#{template[:format]}" }.tally.any? { |_, count| count > 1 } errors << "More than one template found for #{component_class}. " \ "There can only be one default template file per component." From 87978c86ecbe8c8625cabe3b520b26c1898793a9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 27 Aug 2024 12:50:41 -0600 Subject: [PATCH 016/151] remove unused `identifier` --- lib/view_component/base.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 9effabe3b..b5f25f25a 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -579,11 +579,6 @@ def compiler @__vc_compiler ||= Compiler.new(self) end - # @private - def identifier - source_location - end - # Set the parameter name used when rendering elements of a collection ([documentation](/guide/collections)): # # ```ruby From b55a94a32b40e005ba2b9be1fa081a43bca1b9e9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 27 Aug 2024 12:58:04 -0600 Subject: [PATCH 017/151] inline single-use method --- lib/view_component/base.rb | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index b5f25f25a..8cf22999e 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -107,7 +107,14 @@ def render_in(view_context, &block) if render? # Avoid allocating new string when output_preamble and output_postamble are blank - rendered_template = safe_render_template_for(@__vc_variant, request&.format&.to_sym).to_s + rendered_template = + if compiler.renders_template_for_variant?(@__vc_variant) + render_template_for(@__vc_variant, request&.format&.to_sym) + else + maybe_escape_html(render_template_for(@__vc_variant, request&.format.to_sym)) do + Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.") + end + end.to_s if output_preamble.blank? && output_postamble.blank? rendered_template @@ -330,16 +337,6 @@ def maybe_escape_html(text) end end - def safe_render_template_for(variant, format = nil) - if compiler.renders_template_for_variant?(variant) - render_template_for(variant, format) - else - maybe_escape_html(render_template_for(variant, format)) do - Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.") - end - end - end - def safe_output_preamble maybe_escape_html(output_preamble) do Kernel.warn("WARNING: The #{self.class} component was provided an HTML-unsafe preamble. The preamble will be automatically escaped, but you may want to investigate.") From 61a6d6193977b568af5c75ee42a226a9d292ad9f Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 27 Aug 2024 13:10:07 -0600 Subject: [PATCH 018/151] add safe navigation --- lib/view_component/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 8cf22999e..9e7ad54bc 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -111,7 +111,7 @@ def render_in(view_context, &block) if compiler.renders_template_for_variant?(@__vc_variant) render_template_for(@__vc_variant, request&.format&.to_sym) else - maybe_escape_html(render_template_for(@__vc_variant, request&.format.to_sym)) do + maybe_escape_html(render_template_for(@__vc_variant, request&.format&.to_sym)) do Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.") end end.to_s From e77aecde4ee53cec652f349e6dab6695290025e0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Sep 2024 12:48:29 -0600 Subject: [PATCH 019/151] consolidate template collision error messages to include format --- lib/view_component/compiler.rb | 24 ++++++++---------------- lib/view_component/errors.rb | 2 +- test/sandbox/test/rendering_test.rb | 11 ++++++++--- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 36426bf00..aa6b71959 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -190,24 +190,16 @@ def template_errors errors << "Couldn't find a template file or inline render method for #{component_class}." end - if templates.map { |template| "#{template[:variant]}_#{template[:format]}" }.tally.any? { |_, count| count > 1 } - errors << - "More than one template found for #{component_class}. " \ - "There can only be one default template file per component." - end + templates. + map { |template| [template[:variant], template[:format]] }. + tally. + select { |_, count| count > 1 }. + each do |tally| + variant, this_format = tally[0] - invalid_variants = - templates - .group_by { |template| template[:variant] } - .map { |variant, grouped| variant if grouped.length > 1 } - .compact - .sort + variant_string = " for variant #{variant}" if variant.present? - unless invalid_variants.empty? - errors << - "More than one template found for #{"variant".pluralize(invalid_variants.count)} " \ - "#{invalid_variants.map { |v| "'#{v}'" }.to_sentence} in #{component_class}. " \ - "There can only be one template file per variant." + errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component_class}. " end if templates.find { |template| template[:variant].nil? } && inline_calls_defined_on_self.include?(:call) diff --git a/lib/view_component/errors.rb b/lib/view_component/errors.rb index f2d5c1247..79eda652e 100644 --- a/lib/view_component/errors.rb +++ b/lib/view_component/errors.rb @@ -18,7 +18,7 @@ def initialize(klass_name) class TemplateError < StandardError def initialize(errors) - super(errors.join(", ")) + super(errors.join("\n")) end end diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index c18f7e19d..3577138f5 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -459,7 +459,7 @@ def test_raises_error_when_more_than_one_sidecar_template_is_present render_inline(TooManySidecarFilesComponent.new) end - assert_includes error.message, "More than one template found for TooManySidecarFilesComponent." + assert_includes error.message, "More than one HTML template found for TooManySidecarFilesComponent." end def test_raises_error_when_more_than_one_sidecar_template_for_a_variant_is_present @@ -470,7 +470,12 @@ def test_raises_error_when_more_than_one_sidecar_template_for_a_variant_is_prese assert_includes( error.message, - "More than one template found for variants 'test' and 'testing' in TooManySidecarFilesForVariantComponent" + "More than one HTML template found for variant test for TooManySidecarFilesForVariantComponent" + ) + + assert_includes( + error.message, + "More than one HTML template found for variant testing for TooManySidecarFilesForVariantComponent" ) end @@ -538,7 +543,7 @@ def test_raise_error_when_template_file_and_sidecar_directory_template_exist assert_includes( error.message, - "More than one template found for TemplateAndSidecarDirectoryTemplateComponent." + "More than one HTML template found for TemplateAndSidecarDirectoryTemplateComponent." ) end From 9777f38de85651ba0727b22f63d2b44950e0c09e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 08:59:19 -0600 Subject: [PATCH 020/151] add backticks around variant name in error --- lib/view_component/compiler.rb | 2 +- test/sandbox/test/rendering_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index aa6b71959..fbc9b22a3 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -197,7 +197,7 @@ def template_errors each do |tally| variant, this_format = tally[0] - variant_string = " for variant #{variant}" if variant.present? + variant_string = " for variant `#{variant}`" if variant.present? errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component_class}. " end diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index 3577138f5..acac88fca 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -470,12 +470,12 @@ def test_raises_error_when_more_than_one_sidecar_template_for_a_variant_is_prese assert_includes( error.message, - "More than one HTML template found for variant test for TooManySidecarFilesForVariantComponent" + "More than one HTML template found for variant `test` for TooManySidecarFilesForVariantComponent" ) assert_includes( error.message, - "More than one HTML template found for variant testing for TooManySidecarFilesForVariantComponent" + "More than one HTML template found for variant `testing` for TooManySidecarFilesForVariantComponent" ) end From 067f09f336ea3609875f658146b53c0579c2fd66 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:30:10 -0600 Subject: [PATCH 021/151] use one line syntax --- lib/view_component/compiler.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index fbc9b22a3..4674ad919 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -336,15 +336,8 @@ def compile_template(template, handler, identifier = component_class.source_loca def call_method_name(variant, format = nil) out = +"call" - - if variant.present? - out << "_#{normalized_variant_name(variant)}" - end - - if format.present? && format != :html && formats.length > 1 - out << "_#{format}" - end - + out << "_#{normalized_variant_name(variant)}" if variant.present? + out << "_#{format}" if format.present? && format != :html && formats.length > 1 out end From b0aaac87b32d8dca29cbb0719a111299f0807837 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:32:34 -0600 Subject: [PATCH 022/151] no need to return false --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 4674ad919..f52f878ca 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -36,7 +36,7 @@ def compile(raise_errors: false, force: false) if template_errors.present? raise TemplateError.new(template_errors) if raise_errors - return false + return end if raise_errors From ce79fe43bda7e269136c00008a65d2fa5587baf2 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:34:57 -0600 Subject: [PATCH 023/151] inline single-use method --- lib/view_component/compiler.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index f52f878ca..e32de9165 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -31,7 +31,9 @@ def compile(raise_errors: false, force: false) return if compiled? && !force return if component_class == ViewComponent::Base - component_class.superclass.compile(raise_errors: raise_errors) if should_compile_superclass? + if development? && templates.empty? && !has_inline_template? && !call_defined? + component_class.superclass.compile(raise_errors: raise_errors) + end if template_errors.present? raise TemplateError.new(template_errors) if raise_errors @@ -349,10 +351,6 @@ def safe_class_name @safe_class_name ||= component_class.name.underscore.gsub("/", "__") end - def should_compile_superclass? - development? && templates.empty? && !has_inline_template? && !call_defined? - end - def call_defined? component_class.instance_methods(false).include?(:call) || component_class.private_instance_methods(false).include?(:call) From b327ac26d80c2ab705df0ec6ffae652848c6cf83 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:37:30 -0600 Subject: [PATCH 024/151] remove offhand comment that would be better in the compiler, if at all --- lib/view_component/base.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 9e7ad54bc..cdcd92968 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -562,10 +562,6 @@ def ensure_compiled compile unless compiled? end - # Compile templates to instance methods, assuming they haven't been compiled already. - # - # Do as much work as possible in this step, as doing so reduces the amount - # of work done each time a component is rendered. # @private def compile(raise_errors: false, force: false) compiler.compile(raise_errors: raise_errors, force: force) From e3aeb397c111cbb2c5d3a34923d194f7c425c2ac Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:41:59 -0600 Subject: [PATCH 025/151] inline barely used variable --- lib/view_component/compiler.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index e32de9165..59c33f922 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -47,14 +47,12 @@ def compile(raise_errors: false, force: false) end if has_inline_template? - template = component_class.inline_template - redefinition_lock.synchronize do component_class.silence_redefinition_of_method("call") # rubocop:disable Style/EvalWithLocation - component_class.class_eval <<-RUBY, template.path, template.lineno + component_class.class_eval <<-RUBY, component_class.inline_template.path, component_class.inline_template.lineno def call - #{compiled_inline_template(template)} + #{compiled_inline_template(component_class.inline_template)} end RUBY # rubocop:enable Style/EvalWithLocation From 54bd5cdf12a7203c57e1578607ef30806286424f Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:47:17 -0600 Subject: [PATCH 026/151] consolidate safe_class_name into default_method_name --- lib/view_component/compiler.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 59c33f922..122c1aa6f 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -57,12 +57,12 @@ def call RUBY # rubocop:enable Style/EvalWithLocation - component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call)) + component_class.define_method(default_method_name, component_class.instance_method(:call)) component_class.silence_redefinition_of_method("render_template_for") component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) - _call_#{safe_class_name} + #{default_method_name} end RUBY end @@ -107,14 +107,12 @@ def renders_template_for_variant?(variant) def define_render_template_for branches = [] - default_method_name = "_call_#{safe_class_name}" templates.each do |template| - safe_name = +"_call" + safe_name = +default_method_name.to_s variant_name = normalized_variant_name(template[:variant]) safe_name << "_#{variant_name}" if variant_name.present? safe_name << "_#{template[:format]}" if template[:format].present? && template[:format] != :html - safe_name << "_#{safe_class_name}" if safe_name == default_method_name next @@ -145,13 +143,13 @@ def define_render_template_for end variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| - safe_name = "_call_#{normalized_variant_name(variant)}_#{safe_class_name}" + safe_name = "#{default_method_name}_#{normalized_variant_name(variant)}" component_class.define_method(safe_name, component_class.instance_method(call_method_name(variant))) branches << ["variant&.to_sym == :'#{variant}'", safe_name] end - component_class.define_method(:"#{default_method_name}", component_class.instance_method(:call)) + component_class.define_method(default_method_name, component_class.instance_method(:call)) # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name @@ -345,8 +343,8 @@ def normalized_variant_name(variant) variant.to_s.gsub("-", "__").gsub(".", "___") end - def safe_class_name - @safe_class_name ||= component_class.name.underscore.gsub("/", "__") + def default_method_name + @default_method_name ||= "_call_#{component_class.name.underscore.gsub("/", "__")}".to_sym end def call_defined? From bd165724b0f7d0354bf092513ebeafe48261f996 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Sep 2024 12:51:08 -0600 Subject: [PATCH 027/151] use shorter `component` instead of `component_class` --- lib/view_component/compiler.rb | 84 +++++++++++++++++----------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 122c1aa6f..a3883936f 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -13,14 +13,14 @@ class Compiler class_attribute :mode, default: PRODUCTION_MODE - def initialize(component_class) - @component_class = component_class + def initialize(component) + @component = component @redefinition_lock = Mutex.new @variants_rendering_templates = Set.new end def compiled? - CompileCache.compiled?(component_class) + CompileCache.compiled?(component) end def development? @@ -29,10 +29,10 @@ def development? def compile(raise_errors: false, force: false) return if compiled? && !force - return if component_class == ViewComponent::Base + return if component == ViewComponent::Base if development? && templates.empty? && !has_inline_template? && !call_defined? - component_class.superclass.compile(raise_errors: raise_errors) + component.superclass.compile(raise_errors: raise_errors) end if template_errors.present? @@ -42,25 +42,25 @@ def compile(raise_errors: false, force: false) end if raise_errors - component_class.validate_initialization_parameters! - component_class.validate_collection_parameter! + component.validate_initialization_parameters! + component.validate_collection_parameter! end if has_inline_template? redefinition_lock.synchronize do - component_class.silence_redefinition_of_method("call") + component.silence_redefinition_of_method("call") # rubocop:disable Style/EvalWithLocation - component_class.class_eval <<-RUBY, component_class.inline_template.path, component_class.inline_template.lineno + component.class_eval <<-RUBY, component.inline_template.path, component.inline_template.lineno def call - #{compiled_inline_template(component_class.inline_template)} + #{compiled_inline_template(component.inline_template)} end RUBY # rubocop:enable Style/EvalWithLocation - component_class.define_method(default_method_name, component_class.instance_method(:call)) + component.define_method(default_method_name, component.instance_method(:call)) - component_class.silence_redefinition_of_method("render_template_for") - component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1 + component.silence_redefinition_of_method("render_template_for") + component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) #{default_method_name} end @@ -72,9 +72,9 @@ def render_template_for(variant = nil, format = nil) @variants_rendering_templates << template[:variant] redefinition_lock.synchronize do - component_class.silence_redefinition_of_method(method_name) + component.silence_redefinition_of_method(method_name) # rubocop:disable Style/EvalWithLocation - component_class.class_eval <<-RUBY, template[:path], 0 + component.class_eval <<-RUBY, template[:path], 0 def #{method_name} #{compiled_template(template[:path], template[:format])} end @@ -86,15 +86,15 @@ def #{method_name} define_render_template_for end - component_class.registered_slots.each do |slot_name, config| - config[:default_method] = component_class.instance_methods.find { |method_name| method_name == :"default_#{slot_name}" } + component.registered_slots.each do |slot_name, config| + config[:default_method] = component.instance_methods.find { |method_name| method_name == :"default_#{slot_name}" } - component_class.registered_slots[slot_name] = config + component.registered_slots[slot_name] = config end - component_class.build_i18n_backend + component.build_i18n_backend - CompileCache.register(component_class) + CompileCache.register(component) end def renders_template_for_variant?(variant) @@ -103,7 +103,7 @@ def renders_template_for_variant?(variant) private - attr_reader :component_class, :redefinition_lock + attr_reader :component, :redefinition_lock def define_render_template_for branches = [] @@ -117,9 +117,9 @@ def define_render_template_for if safe_name == default_method_name next else - component_class.define_method( + component.define_method( safe_name, - component_class.instance_method( + component.instance_method( call_method_name(template[:variant], template[:format]) ) ) @@ -144,12 +144,12 @@ def define_render_template_for variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| safe_name = "#{default_method_name}_#{normalized_variant_name(variant)}" - component_class.define_method(safe_name, component_class.instance_method(call_method_name(variant))) + component.define_method(safe_name, component.instance_method(call_method_name(variant))) branches << ["variant&.to_sym == :'#{variant}'", safe_name] end - component_class.define_method(default_method_name, component_class.instance_method(:call)) + component.define_method(default_method_name, component.instance_method(:call)) # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name @@ -166,8 +166,8 @@ def define_render_template_for end redefinition_lock.synchronize do - component_class.silence_redefinition_of_method(:render_template_for) - component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1 + component.silence_redefinition_of_method(:render_template_for) + component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) #{body} end @@ -176,7 +176,7 @@ def render_template_for(variant = nil, format = nil) end def has_inline_template? - component_class.respond_to?(:inline_template) && component_class.inline_template.present? + component.respond_to?(:inline_template) && component.inline_template.present? end def template_errors @@ -185,7 +185,7 @@ def template_errors errors = [] if (templates + inline_calls).empty? && !has_inline_template? - errors << "Couldn't find a template file or inline render method for #{component_class}." + errors << "Couldn't find a template file or inline render method for #{component}." end templates. @@ -197,12 +197,12 @@ def template_errors variant_string = " for variant `#{variant}`" if variant.present? - errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component_class}. " + errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " end if templates.find { |template| template[:variant].nil? } && inline_calls_defined_on_self.include?(:call) errors << - "Template file and inline render method found for #{component_class}. " \ + "Template file and inline render method found for #{component}. " \ "There can only be a template file or inline render method per component." end @@ -216,7 +216,7 @@ def template_errors "Template #{"file".pluralize(count)} and inline render #{"method".pluralize(count)} " \ "found for #{"variant".pluralize(count)} " \ "#{duplicate_template_file_and_inline_variant_calls.map { |v| "'#{v}'" }.to_sentence} " \ - "in #{component_class}. " \ + "in #{component}. " \ "There can only be a template file or inline render method per variant." end @@ -230,7 +230,7 @@ def template_errors unless colliding_variants.empty? errors << "Colliding templates #{colliding_variants.sort.map { |v| "'#{v}'" }.to_sentence} " \ - "found in #{component_class}." + "found in #{component}." end errors @@ -242,7 +242,7 @@ def templates begin extensions = ActionView::Template.template_handler_extensions - component_class.sidecar_files(extensions).each_with_object([]) do |path, memo| + component.sidecar_files(extensions).each_with_object([]) do |path, memo| pieces = File.basename(path).split(".") memo << { path: path, @@ -261,8 +261,8 @@ def inline_calls # finding inline calls view_component_ancestors = ( - component_class.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - - component_class.included_modules + component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - + component.included_modules ) view_component_ancestors.flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }.uniq @@ -270,7 +270,7 @@ def inline_calls end def inline_calls_defined_on_self - @inline_calls_defined_on_self ||= component_class.instance_methods(false).grep(/^call(_|$)/) + @inline_calls_defined_on_self ||= component.instance_methods(false).grep(/^call(_|$)/) end def formats @@ -303,8 +303,8 @@ def compiled_template(file_path, format) compile_template(template, handler, file_path, format) end - def compile_template(template, handler, identifier = component_class.source_location, format = :html) - template.rstrip! if component_class.strip_trailing_whitespace? + def compile_template(template, handler, identifier = component.source_location, format = :html) + template.rstrip! if component.strip_trailing_whitespace? short_identifier = defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier type = ActionView::Template::Types[format] @@ -344,12 +344,12 @@ def normalized_variant_name(variant) end def default_method_name - @default_method_name ||= "_call_#{component_class.name.underscore.gsub("/", "__")}".to_sym + @default_method_name ||= "_call_#{component.name.underscore.gsub("/", "__")}".to_sym end def call_defined? - component_class.instance_methods(false).include?(:call) || - component_class.private_instance_methods(false).include?(:call) + component.instance_methods(false).include?(:call) || + component.private_instance_methods(false).include?(:call) end end end From 6cc7f99ce544cdf88c8aa47d4fcfc76fe9d153fd Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:53:28 -0600 Subject: [PATCH 028/151] remove unnecessary conditional --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index a3883936f..c8243a710 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -112,7 +112,7 @@ def define_render_template_for safe_name = +default_method_name.to_s variant_name = normalized_variant_name(template[:variant]) safe_name << "_#{variant_name}" if variant_name.present? - safe_name << "_#{template[:format]}" if template[:format].present? && template[:format] != :html + safe_name << "_#{template[:format]}" if template[:format] != :html if safe_name == default_method_name next From ab90237209b585983cef312af63d996ec480a74d Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 10:56:07 -0600 Subject: [PATCH 029/151] remove unnecessary nesting --- lib/view_component/compiler.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index c8243a710..14bf6caf8 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -114,16 +114,14 @@ def define_render_template_for safe_name << "_#{variant_name}" if variant_name.present? safe_name << "_#{template[:format]}" if template[:format] != :html - if safe_name == default_method_name - next - else - component.define_method( - safe_name, - component.instance_method( - call_method_name(template[:variant], template[:format]) - ) + next if safe_name == default_method_name + + component.define_method( + safe_name, + component.instance_method( + call_method_name(template[:variant], template[:format]) ) - end + ) format_conditional = if template[:format] == :html From e09adf990a2e607e4460785784b0a54a65f0860f Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 11:55:23 -0600 Subject: [PATCH 030/151] inline single use method --- lib/view_component/compiler.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 14bf6caf8..348ecc3db 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -31,7 +31,12 @@ def compile(raise_errors: false, force: false) return if compiled? && !force return if component == ViewComponent::Base - if development? && templates.empty? && !has_inline_template? && !call_defined? + if ( + development? && + templates.empty? && + !has_inline_template? && + !(component.instance_methods(false).include?(:call) || component.private_instance_methods(false).include?(:call)) + ) component.superclass.compile(raise_errors: raise_errors) end @@ -344,10 +349,5 @@ def normalized_variant_name(variant) def default_method_name @default_method_name ||= "_call_#{component.name.underscore.gsub("/", "__")}".to_sym end - - def call_defined? - component.instance_methods(false).include?(:call) || - component.private_instance_methods(false).include?(:call) - end end end From ce15d3ff85f2bea39ae26dc0e4ef8ed42756b5f7 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 12:03:41 -0600 Subject: [PATCH 031/151] remove intermediate compiler methods --- lib/view_component/compiler.rb | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 348ecc3db..9d6d7fce9 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -54,10 +54,11 @@ def compile(raise_errors: false, force: false) if has_inline_template? redefinition_lock.synchronize do component.silence_redefinition_of_method("call") + # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, component.inline_template.path, component.inline_template.lineno def call - #{compiled_inline_template(component.inline_template)} + #{compiled_template(component.inline_template.source.dup, component.inline_template.language)} end RUBY # rubocop:enable Style/EvalWithLocation @@ -81,7 +82,7 @@ def render_template_for(variant = nil, format = nil) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], 0 def #{method_name} - #{compiled_template(template[:path], template[:format])} + #{compiled_template(File.read(template[:path]), File.extname(template[:path]).delete("."), template[:path], template[:format])} end RUBY # rubocop:enable Style/EvalWithLocation @@ -292,22 +293,9 @@ def variants_from_inline_calls(calls) end end - def compiled_inline_template(template) - handler = ActionView::Template.handler_for_extension(template.language) - template = template.source.dup - - compile_template(template, handler) - end - - def compiled_template(file_path, format) - handler = ActionView::Template.handler_for_extension(File.extname(file_path).delete(".")) - template = File.read(file_path) - - compile_template(template, handler, file_path, format) - end - - def compile_template(template, handler, identifier = component.source_location, format = :html) - template.rstrip! if component.strip_trailing_whitespace? + def compiled_template(template_source, extension, identifier = component.source_location, format = :html) + handler = ActionView::Template.handler_for_extension(extension) + template_source.rstrip! if component.strip_trailing_whitespace? short_identifier = defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier type = ActionView::Template::Types[format] @@ -320,13 +308,13 @@ def compile_template(template, handler, identifier = component.source_location, short_identifier: short_identifier, type: type ), - template + template_source ) # :nocov: else handler.call( OpenStruct.new( - source: template, + source: template_source, identifier: identifier, type: type ) From fcafca2ed465004b36c622d2a2e67792a4f704dc Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 12:06:26 -0600 Subject: [PATCH 032/151] InlineTemplate is now included by default --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 9d6d7fce9..e64dc94fb 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -180,7 +180,7 @@ def render_template_for(variant = nil, format = nil) end def has_inline_template? - component.respond_to?(:inline_template) && component.inline_template.present? + component.inline_template.present? end def template_errors From 6a351b49e1868a229a92c1259fa02e78dbef498d Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 14:42:45 -0600 Subject: [PATCH 033/151] move comment to be in correct location --- lib/view_component/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index cdcd92968..3c0513604 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -106,7 +106,6 @@ def render_in(view_context, &block) before_render if render? - # Avoid allocating new string when output_preamble and output_postamble are blank rendered_template = if compiler.renders_template_for_variant?(@__vc_variant) render_template_for(@__vc_variant, request&.format&.to_sym) @@ -116,6 +115,7 @@ def render_in(view_context, &block) end end.to_s + # Avoid allocating new string when output_preamble and output_postamble are blank if output_preamble.blank? && output_postamble.blank? rendered_template else From c8f65c204587c77433c29dfafd219d28af6037d6 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 15:27:31 -0600 Subject: [PATCH 034/151] start to make inline templates like other templates --- lib/view_component/compiler.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index e64dc94fb..c5332d13d 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -34,7 +34,6 @@ def compile(raise_errors: false, force: false) if ( development? && templates.empty? && - !has_inline_template? && !(component.instance_methods(false).include?(:call) || component.private_instance_methods(false).include?(:call)) ) component.superclass.compile(raise_errors: raise_errors) @@ -51,7 +50,7 @@ def compile(raise_errors: false, force: false) component.validate_collection_parameter! end - if has_inline_template? + if templates.any? { _1[:type] == :inline } redefinition_lock.synchronize do component.silence_redefinition_of_method("call") @@ -74,6 +73,8 @@ def render_template_for(variant = nil, format = nil) end else templates.each do |template| + next if template[:type] == :inline + method_name = call_method_name(template[:variant], template[:format]) @variants_rendering_templates << template[:variant] @@ -179,16 +180,12 @@ def render_template_for(variant = nil, format = nil) end end - def has_inline_template? - component.inline_template.present? - end - def template_errors @__vc_template_errors ||= begin errors = [] - if (templates + inline_calls).empty? && !has_inline_template? + if (templates + inline_calls).empty? errors << "Couldn't find a template file or inline render method for #{component}." end @@ -246,7 +243,7 @@ def templates begin extensions = ActionView::Template.template_handler_extensions - component.sidecar_files(extensions).each_with_object([]) do |path, memo| + templates = component.sidecar_files(extensions).each_with_object([]) do |path, memo| pieces = File.basename(path).split(".") memo << { path: path, @@ -255,6 +252,15 @@ def templates handler: pieces.last } end + + if component.inline_template.present? + templates << { + type: :inline, + handler: component.inline_template.language + } + end + + templates end end From 08c879865e3d2f2ccda564310b6210cec179ff62 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 15:34:17 -0600 Subject: [PATCH 035/151] continue to consolidate inline template logic --- lib/view_component/compiler.rb | 46 ++++++++++++++++------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index c5332d13d..55ae62a32 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -50,31 +50,29 @@ def compile(raise_errors: false, force: false) component.validate_collection_parameter! end - if templates.any? { _1[:type] == :inline } - redefinition_lock.synchronize do - component.silence_redefinition_of_method("call") - - # rubocop:disable Style/EvalWithLocation - component.class_eval <<-RUBY, component.inline_template.path, component.inline_template.lineno - def call - #{compiled_template(component.inline_template.source.dup, component.inline_template.language)} - end - RUBY - # rubocop:enable Style/EvalWithLocation + templates.each do |template| + if template[:type] == :inline + redefinition_lock.synchronize do + component.silence_redefinition_of_method("call") - component.define_method(default_method_name, component.instance_method(:call)) + # rubocop:disable Style/EvalWithLocation + component.class_eval <<-RUBY, component.inline_template.path, component.inline_template.lineno + def call + #{compiled_template(component.inline_template.source.dup, component.inline_template.language)} + end + RUBY + # rubocop:enable Style/EvalWithLocation - component.silence_redefinition_of_method("render_template_for") - component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def render_template_for(variant = nil, format = nil) - #{default_method_name} - end - RUBY - end - else - templates.each do |template| - next if template[:type] == :inline + component.define_method(default_method_name, component.instance_method(:call)) + component.silence_redefinition_of_method("render_template_for") + component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def render_template_for(variant = nil, format = nil) + #{default_method_name} + end + RUBY + end + else method_name = call_method_name(template[:variant], template[:format]) @variants_rendering_templates << template[:variant] @@ -89,10 +87,10 @@ def #{method_name} # rubocop:enable Style/EvalWithLocation end end - - define_render_template_for end + define_render_template_for if !templates.any? { _1[:type] == :inline } + component.registered_slots.each do |slot_name, config| config[:default_method] = component.instance_methods.find { |method_name| method_name == :"default_#{slot_name}" } From c2e00b2faf8c2fd0351bf1bdf6cc30d98ef4defc Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 15:38:09 -0600 Subject: [PATCH 036/151] inline templates are just templates --- lib/view_component/compiler.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 55ae62a32..42cfffe1d 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -56,9 +56,9 @@ def compile(raise_errors: false, force: false) component.silence_redefinition_of_method("call") # rubocop:disable Style/EvalWithLocation - component.class_eval <<-RUBY, component.inline_template.path, component.inline_template.lineno + component.class_eval <<-RUBY, template[:path], template[:lineno] def call - #{compiled_template(component.inline_template.source.dup, component.inline_template.language)} + #{compiled_template(template[:source], template[:handler])} end RUBY # rubocop:enable Style/EvalWithLocation @@ -254,6 +254,9 @@ def templates if component.inline_template.present? templates << { type: :inline, + lineno: component.inline_template.lineno, + path: component.inline_template.path, + source: component.inline_template.source.dup, handler: component.inline_template.language } end From fa46d92ffdc211b3f952c36f1ebac55f707dca4f Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 15:50:29 -0600 Subject: [PATCH 037/151] move lock to outside conditional --- lib/view_component/compiler.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 42cfffe1d..a17fa5d96 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -51,8 +51,8 @@ def compile(raise_errors: false, force: false) end templates.each do |template| - if template[:type] == :inline - redefinition_lock.synchronize do + redefinition_lock.synchronize do + if template[:type] == :inline component.silence_redefinition_of_method("call") # rubocop:disable Style/EvalWithLocation @@ -71,12 +71,10 @@ def render_template_for(variant = nil, format = nil) #{default_method_name} end RUBY - end - else - method_name = call_method_name(template[:variant], template[:format]) - @variants_rendering_templates << template[:variant] + else + method_name = call_method_name(template[:variant], template[:format]) + @variants_rendering_templates << template[:variant] - redefinition_lock.synchronize do component.silence_redefinition_of_method(method_name) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], 0 From 992c30d103a37ea9a67235bdeccc6c99014e7858 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 15:51:30 -0600 Subject: [PATCH 038/151] use same method_name definition --- lib/view_component/compiler.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index a17fa5d96..14b809ea5 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -52,12 +52,13 @@ def compile(raise_errors: false, force: false) templates.each do |template| redefinition_lock.synchronize do - if template[:type] == :inline - component.silence_redefinition_of_method("call") + method_name = call_method_name(template[:variant], template[:format]) + component.silence_redefinition_of_method(method_name) + if template[:type] == :inline # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] - def call + def #{method_name} #{compiled_template(template[:source], template[:handler])} end RUBY @@ -72,10 +73,8 @@ def render_template_for(variant = nil, format = nil) end RUBY else - method_name = call_method_name(template[:variant], template[:format]) @variants_rendering_templates << template[:variant] - component.silence_redefinition_of_method(method_name) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], 0 def #{method_name} From 85efee752989e7d196b32153a69110da64562859 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Aug 2024 15:52:49 -0600 Subject: [PATCH 039/151] all templates have line numbers --- lib/view_component/compiler.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 14b809ea5..2808c72f4 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -76,7 +76,7 @@ def render_template_for(variant = nil, format = nil) @variants_rendering_templates << template[:variant] # rubocop:disable Style/EvalWithLocation - component.class_eval <<-RUBY, template[:path], 0 + component.class_eval <<-RUBY, template[:path], template[:lineno] def #{method_name} #{compiled_template(File.read(template[:path]), File.extname(template[:path]).delete("."), template[:path], template[:format])} end @@ -241,7 +241,9 @@ def templates templates = component.sidecar_files(extensions).each_with_object([]) do |path, memo| pieces = File.basename(path).split(".") memo << { + type: :file, path: path, + lineno: 0, format: pieces[1..-2].join(".").split("+").first&.to_sym, variant: pieces[1..-2].join(".").split("+").second&.to_sym, handler: pieces.last From 18c78162b570af5b00083699743215a36eb411e9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 12:33:33 -0600 Subject: [PATCH 040/151] move towards single method definition path with source --- lib/view_component/compiler.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 2808c72f4..91740725e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -55,11 +55,13 @@ def compile(raise_errors: false, force: false) method_name = call_method_name(template[:variant], template[:format]) component.silence_redefinition_of_method(method_name) + source = template[:source] || File.read(template[:path]) + if template[:type] == :inline # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] def #{method_name} - #{compiled_template(template[:source], template[:handler])} + #{compiled_template(source, template[:handler])} end RUBY # rubocop:enable Style/EvalWithLocation @@ -78,7 +80,7 @@ def render_template_for(variant = nil, format = nil) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] def #{method_name} - #{compiled_template(File.read(template[:path]), File.extname(template[:path]).delete("."), template[:path], template[:format])} + #{compiled_template(source, File.extname(template[:path]).delete("."), template[:path], template[:format])} end RUBY # rubocop:enable Style/EvalWithLocation From 7cc3d2b31a608d289b619770b9f463fed3bdc671 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 12:35:03 -0600 Subject: [PATCH 041/151] template handler is already present, no need to re-calculate --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 91740725e..212bb3975 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -80,7 +80,7 @@ def render_template_for(variant = nil, format = nil) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] def #{method_name} - #{compiled_template(source, File.extname(template[:path]).delete("."), template[:path], template[:format])} + #{compiled_template(source, template[:handler], template[:path], template[:format])} end RUBY # rubocop:enable Style/EvalWithLocation From c0c078720153f0985f2d5b644380d758a370c681 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 12:39:09 -0600 Subject: [PATCH 042/151] just pass the template around instead of four params --- lib/view_component/compiler.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 212bb3975..89cf908fa 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -55,13 +55,11 @@ def compile(raise_errors: false, force: false) method_name = call_method_name(template[:variant], template[:format]) component.silence_redefinition_of_method(method_name) - source = template[:source] || File.read(template[:path]) - if template[:type] == :inline # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] def #{method_name} - #{compiled_template(source, template[:handler])} + #{compiled_template(template)} end RUBY # rubocop:enable Style/EvalWithLocation @@ -80,7 +78,7 @@ def render_template_for(variant = nil, format = nil) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] def #{method_name} - #{compiled_template(source, template[:handler], template[:path], template[:format])} + #{compiled_template(template)} end RUBY # rubocop:enable Style/EvalWithLocation @@ -301,18 +299,19 @@ def variants_from_inline_calls(calls) end end - def compiled_template(template_source, extension, identifier = component.source_location, format = :html) - handler = ActionView::Template.handler_for_extension(extension) + def compiled_template(template) + handler = ActionView::Template.handler_for_extension(template[:handler]) + template_source = template[:source] || File.read(template[:path]) template_source.rstrip! if component.strip_trailing_whitespace? - short_identifier = defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier - type = ActionView::Template::Types[format] + short_identifier = defined?(Rails.root) ? template[:path].sub("#{Rails.root}/", "") : template[:path] + type = ActionView::Template::Types[template[:format]] if handler.method(:call).parameters.length > 1 handler.call( OpenStruct.new( - format: format, - identifier: identifier, + format: template[:format], + identifier: template[:path], short_identifier: short_identifier, type: type ), @@ -323,7 +322,7 @@ def compiled_template(template_source, extension, identifier = component.source_ handler.call( OpenStruct.new( source: template_source, - identifier: identifier, + identifier: template[:path], type: type ) ) From dcfe939cb7785bc0f2b14c50866610dc9c1a8668 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 12:40:17 -0600 Subject: [PATCH 043/151] use single class_eval to define template methods --- lib/view_component/compiler.rb | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 89cf908fa..a6cc18e55 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -55,15 +55,15 @@ def compile(raise_errors: false, force: false) method_name = call_method_name(template[:variant], template[:format]) component.silence_redefinition_of_method(method_name) - if template[:type] == :inline - # rubocop:disable Style/EvalWithLocation - component.class_eval <<-RUBY, template[:path], template[:lineno] - def #{method_name} - #{compiled_template(template)} - end - RUBY - # rubocop:enable Style/EvalWithLocation + # rubocop:disable Style/EvalWithLocation + component.class_eval <<-RUBY, template[:path], template[:lineno] + def #{method_name} + #{compiled_template(template)} + end + RUBY + # rubocop:enable Style/EvalWithLocation + if template[:type] == :inline component.define_method(default_method_name, component.instance_method(:call)) component.silence_redefinition_of_method("render_template_for") @@ -74,14 +74,6 @@ def render_template_for(variant = nil, format = nil) RUBY else @variants_rendering_templates << template[:variant] - - # rubocop:disable Style/EvalWithLocation - component.class_eval <<-RUBY, template[:path], template[:lineno] - def #{method_name} - #{compiled_template(template)} - end - RUBY - # rubocop:enable Style/EvalWithLocation end end end From 1e8715d0f9c398a39e953b9a5f0a26a0cd265a63 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 12:43:48 -0600 Subject: [PATCH 044/151] remove unnecessary skipping of iterator --- lib/view_component/compiler.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index a6cc18e55..8af85990a 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -108,8 +108,6 @@ def define_render_template_for safe_name << "_#{variant_name}" if variant_name.present? safe_name << "_#{template[:format]}" if template[:format] != :html - next if safe_name == default_method_name - component.define_method( safe_name, component.instance_method( From c4c06e93a08b5a4b23a676716563fc4cab61aebc Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 12:45:29 -0600 Subject: [PATCH 045/151] make template hashes look the same --- lib/view_component/compiler.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 8af85990a..539e91962 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -235,6 +235,7 @@ def templates path: path, lineno: 0, format: pieces[1..-2].join(".").split("+").first&.to_sym, + source: nil, variant: pieces[1..-2].join(".").split("+").second&.to_sym, handler: pieces.last } @@ -243,9 +244,11 @@ def templates if component.inline_template.present? templates << { type: :inline, - lineno: component.inline_template.lineno, path: component.inline_template.path, + lineno: component.inline_template.lineno, + format: nil, source: component.inline_template.source.dup, + variant: nil, handler: component.inline_template.language } end From fc49c1cff8a85f8708b6577ad38bc913d313eea7 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 12:59:26 -0600 Subject: [PATCH 046/151] move default slot registration to Slotable --- lib/view_component/compiler.rb | 7 +------ lib/view_component/slotable.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 539e91962..5867a50d2 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -80,12 +80,7 @@ def render_template_for(variant = nil, format = nil) define_render_template_for if !templates.any? { _1[:type] == :inline } - component.registered_slots.each do |slot_name, config| - config[:default_method] = component.instance_methods.find { |method_name| method_name == :"default_#{slot_name}" } - - component.registered_slots[slot_name] = config - end - + component.register_default_slots component.build_i18n_backend CompileCache.register(component) diff --git a/lib/view_component/slotable.rb b/lib/view_component/slotable.rb index c537d4828..a8c298965 100644 --- a/lib/view_component/slotable.rb +++ b/lib/view_component/slotable.rb @@ -268,6 +268,15 @@ def register_polymorphic_slot(slot_name, types, collection:) } end + # Called by the compiler, as instance methods are not defined when slots are first registered + def register_default_slots + registered_slots.each do |slot_name, config| + config[:default_method] = instance_methods.find { |method_name| method_name == :"default_#{slot_name}" } + + registered_slots[slot_name] = config + end + end + private def register_slot(slot_name, **kwargs) From ba825d883d008fefce10f2a9a77f56514f8ba96e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:13:18 -0600 Subject: [PATCH 047/151] define render_template_for in one place --- lib/view_component/compiler.rb | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 5867a50d2..f1054bfab 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -63,22 +63,11 @@ def #{method_name} RUBY # rubocop:enable Style/EvalWithLocation - if template[:type] == :inline - component.define_method(default_method_name, component.instance_method(:call)) - - component.silence_redefinition_of_method("render_template_for") - component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def render_template_for(variant = nil, format = nil) - #{default_method_name} - end - RUBY - else - @variants_rendering_templates << template[:variant] - end + @variants_rendering_templates << template[:variant] if template[:type] == :file end end - define_render_template_for if !templates.any? { _1[:type] == :inline } + define_render_template_for component.register_default_slots component.build_i18n_backend @@ -97,6 +86,19 @@ def renders_template_for_variant?(variant) def define_render_template_for branches = [] + if templates.any? { _1[:type] == :inline } + component.define_method(default_method_name, component.instance_method(:call)) + + component.silence_redefinition_of_method("render_template_for") + component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def render_template_for(variant = nil, format = nil) + #{default_method_name} + end + RUBY + + return + end + templates.each do |template| safe_name = +default_method_name.to_s variant_name = normalized_variant_name(template[:variant]) From c3d6a37840054d159b1f96f8f075c6e8613f7f5d Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:25:57 -0600 Subject: [PATCH 048/151] remove duplicated method name construction --- lib/view_component/compiler.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index f1054bfab..03823326d 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -100,10 +100,7 @@ def render_template_for(variant = nil, format = nil) end templates.each do |template| - safe_name = +default_method_name.to_s - variant_name = normalized_variant_name(template[:variant]) - safe_name << "_#{variant_name}" if variant_name.present? - safe_name << "_#{template[:format]}" if template[:format] != :html + safe_name = "_#{call_method_name(template[:variant], template[:format])}_#{component.name.underscore.gsub("/", "__")}" component.define_method( safe_name, From 4c8588a95dcfef940b0f4d54340b2ffcbce8ded3 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:27:07 -0600 Subject: [PATCH 049/151] sort nils last --- lib/view_component/compiler.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 03823326d..9cb8ca72b 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -228,10 +228,10 @@ def templates type: :file, path: path, lineno: 0, - format: pieces[1..-2].join(".").split("+").first&.to_sym, source: nil, - variant: pieces[1..-2].join(".").split("+").second&.to_sym, - handler: pieces.last + handler: pieces.last, + format: pieces[1..-2].join(".").split("+").first&.to_sym, + variant: pieces[1..-2].join(".").split("+").second&.to_sym } end @@ -240,10 +240,10 @@ def templates type: :inline, path: component.inline_template.path, lineno: component.inline_template.lineno, - format: nil, source: component.inline_template.source.dup, - variant: nil, - handler: component.inline_template.language + handler: component.inline_template.language, + format: nil, + variant: nil } end From f4aa2ddf54e5eaed8e248568ed2caec400bd9800 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:30:26 -0600 Subject: [PATCH 050/151] consolidate construction of safe_name --- lib/view_component/compiler.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 9cb8ca72b..4b2e16026 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -100,7 +100,7 @@ def render_template_for(variant = nil, format = nil) end templates.each do |template| - safe_name = "_#{call_method_name(template[:variant], template[:format])}_#{component.name.underscore.gsub("/", "__")}" + safe_name = safe_name_for(template[:variant], template[:format]) component.define_method( safe_name, @@ -127,7 +127,7 @@ def render_template_for(variant = nil, format = nil) end variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| - safe_name = "#{default_method_name}_#{normalized_variant_name(variant)}" + safe_name = safe_name_for(variant, nil) component.define_method(safe_name, component.instance_method(call_method_name(variant))) branches << ["variant&.to_sym == :'#{variant}'", safe_name] @@ -324,6 +324,10 @@ def call_method_name(variant, format = nil) out end + def safe_name_for(variant, format) + "_#{call_method_name(variant, format)}_#{component.name.underscore.gsub("/", "__")}" + end + def normalized_variant_name(variant) variant.to_s.gsub("-", "__").gsub(".", "___") end From 70e2f06007c653bdd3a40922377cdb03c2a32307 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:33:33 -0600 Subject: [PATCH 051/151] inline single-use method that at minimum should have been private --- lib/view_component/compiler.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 4b2e16026..829f8ac18 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -23,16 +23,12 @@ def compiled? CompileCache.compiled?(component) end - def development? - self.class.mode == DEVELOPMENT_MODE - end - def compile(raise_errors: false, force: false) return if compiled? && !force return if component == ViewComponent::Base if ( - development? && + self.class.mode == DEVELOPMENT_MODE && templates.empty? && !(component.instance_methods(false).include?(:call) || component.private_instance_methods(false).include?(:call)) ) From daf544c8d5e74bb615c8c453b974f97dfd8cd858 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:37:08 -0600 Subject: [PATCH 052/151] method name should not depend on templates state --- lib/view_component/compiler.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 829f8ac18..296ec2abe 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -129,7 +129,9 @@ def render_template_for(variant = nil, format = nil) branches << ["variant&.to_sym == :'#{variant}'", safe_name] end - component.define_method(default_method_name, component.instance_method(:call)) + if component.instance_methods.include?(:call) + component.define_method(default_method_name, component.instance_method(:call)) + end # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name @@ -316,7 +318,7 @@ def compiled_template(template) def call_method_name(variant, format = nil) out = +"call" out << "_#{normalized_variant_name(variant)}" if variant.present? - out << "_#{format}" if format.present? && format != :html && formats.length > 1 + out << "_#{format}" if format.present? && format != :html out end From 4914d7a66d2df571a20f2576dcfd29fe080e04e0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:47:19 -0600 Subject: [PATCH 053/151] set method name on template hash --- lib/view_component/compiler.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 296ec2abe..8d80c0d80 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -48,12 +48,11 @@ def compile(raise_errors: false, force: false) templates.each do |template| redefinition_lock.synchronize do - method_name = call_method_name(template[:variant], template[:format]) - component.silence_redefinition_of_method(method_name) + component.silence_redefinition_of_method(template[:method_name]) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] - def #{method_name} + def #{template[:method_name]} #{compiled_template(template)} end RUBY @@ -82,13 +81,13 @@ def renders_template_for_variant?(variant) def define_render_template_for branches = [] - if templates.any? { _1[:type] == :inline } - component.define_method(default_method_name, component.instance_method(:call)) + if template = templates.find { _1[:type] == :inline } + component.define_method(template[:safe_method_name], component.instance_method(:call)) component.silence_redefinition_of_method("render_template_for") component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) - #{default_method_name} + #{template[:safe_method_name]} end RUBY @@ -245,7 +244,12 @@ def templates } end - templates + templates.map! do |template| + template[:safe_method_name] = safe_name_for(template[:variant], template[:format]) + template[:method_name] = call_method_name(template[:variant], template[:format]) + + template + end end end From d46e3072b32d97a15672928485f6eecfe02530e9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:49:03 -0600 Subject: [PATCH 054/151] use method name from template hash --- lib/view_component/compiler.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 8d80c0d80..14e0b5f3c 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -95,12 +95,10 @@ def render_template_for(variant = nil, format = nil) end templates.each do |template| - safe_name = safe_name_for(template[:variant], template[:format]) - component.define_method( - safe_name, + template[:safe_method_name], component.instance_method( - call_method_name(template[:variant], template[:format]) + template[:method_name] ) ) @@ -118,7 +116,7 @@ def render_template_for(variant = nil, format = nil) "variant&.to_sym == :'#{template[:variant]}'" end - branches << ["#{variant_conditional} && #{format_conditional}", safe_name] + branches << ["#{variant_conditional} && #{format_conditional}", template[:safe_method_name]] end variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| From b50457694b47869b301dd3d4d9d70544337db707 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:52:28 -0600 Subject: [PATCH 055/151] shorten to one-line --- lib/view_component/compiler.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 14e0b5f3c..2a47ab88e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -95,12 +95,7 @@ def render_template_for(variant = nil, format = nil) end templates.each do |template| - component.define_method( - template[:safe_method_name], - component.instance_method( - template[:method_name] - ) - ) + component.define_method(template[:safe_method_name], component.instance_method(template[:method_name])) format_conditional = if template[:format] == :html From f52acfc7f832c92f9a599d9ad19bad9b8376c7cd Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:55:28 -0600 Subject: [PATCH 056/151] move conditional into existing file template conditional --- lib/view_component/compiler.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 2a47ab88e..1c716ad66 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -57,8 +57,6 @@ def #{template[:method_name]} end RUBY # rubocop:enable Style/EvalWithLocation - - @variants_rendering_templates << template[:variant] if template[:type] == :file end end @@ -214,7 +212,8 @@ def templates templates = component.sidecar_files(extensions).each_with_object([]) do |path, memo| pieces = File.basename(path).split(".") - memo << { + + out = { type: :file, path: path, lineno: 0, @@ -223,6 +222,10 @@ def templates format: pieces[1..-2].join(".").split("+").first&.to_sym, variant: pieces[1..-2].join(".").split("+").second&.to_sym } + + @variants_rendering_templates << out[:variant] + + memo << out end if component.inline_template.present? From 82c13ee38a43c19ccef4d60f001becc3aa1f9fd4 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 13:57:11 -0600 Subject: [PATCH 057/151] simplify `templates --- lib/view_component/compiler.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 1c716ad66..b3c6a914f 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -208,9 +208,9 @@ def template_errors def templates @templates ||= begin - extensions = ActionView::Template.template_handler_extensions - - templates = component.sidecar_files(extensions).each_with_object([]) do |path, memo| + templates = component.sidecar_files( + ActionView::Template.template_handler_extensions + ).map do |path| pieces = File.basename(path).split(".") out = { @@ -225,7 +225,7 @@ def templates @variants_rendering_templates << out[:variant] - memo << out + out end if component.inline_template.present? From f7a93abdab755831f279ecbe63190cb98aa5f073 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 30 Aug 2024 14:03:09 -0600 Subject: [PATCH 058/151] remove unused formats method --- lib/view_component/compiler.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index b3c6a914f..4f451e842 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -268,10 +268,6 @@ def inline_calls_defined_on_self @inline_calls_defined_on_self ||= component.instance_methods(false).grep(/^call(_|$)/) end - def formats - @__vc_variants = (templates.map { |template| template[:format] }).compact.uniq - end - def variants @__vc_variants = ( templates.map { |template| template[:variant] } + variants_from_inline_calls(inline_calls) From 2c4608256e4c8f71a8bd858bc2574cc420c98426 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Sep 2024 11:06:01 -0600 Subject: [PATCH 059/151] always set @templates at the same point vs. lazily --- lib/view_component/compiler.rb | 6 ++++-- test/sandbox/test/base_test.rb | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 4f451e842..3162bfe9e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -27,6 +27,8 @@ def compile(raise_errors: false, force: false) return if compiled? && !force return if component == ViewComponent::Base + gather_templates + if ( self.class.mode == DEVELOPMENT_MODE && templates.empty? && @@ -74,7 +76,7 @@ def renders_template_for_variant?(variant) private - attr_reader :component, :redefinition_lock + attr_reader :component, :redefinition_lock, :templates def define_render_template_for branches = [] @@ -205,7 +207,7 @@ def template_errors end end - def templates + def gather_templates @templates ||= begin templates = component.sidecar_files( diff --git a/test/sandbox/test/base_test.rb b/test/sandbox/test/base_test.rb index 1d560c9d1..80338cd74 100644 --- a/test/sandbox/test/base_test.rb +++ b/test/sandbox/test/base_test.rb @@ -18,7 +18,7 @@ def test_templates_parses_all_types_of_paths compiler = ViewComponent::Compiler.new(ViewComponent::Base) ViewComponent::Base.stub(:sidecar_files, file_path) do - templates = compiler.send(:templates) + templates = compiler.send(:gather_templates) templates.each_with_index do |template, index| assert_equal(template[:path], file_path[index]) From eb4bb638210928362a5844fc64d725161e0ba88c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Sep 2024 11:19:23 -0600 Subject: [PATCH 060/151] move compiled_template to Template --- lib/view_component/compiler.rb | 70 ++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 3162bfe9e..e6f1cc5ce 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -55,7 +55,7 @@ def compile(raise_errors: false, force: false) # rubocop:disable Style/EvalWithLocation component.class_eval <<-RUBY, template[:path], template[:lineno] def #{template[:method_name]} - #{compiled_template(template)} + #{Template.new(component: component, path: template[:path], source: template[:source], extension: template[:handler], this_format: template[:format]).compiled_source} end RUBY # rubocop:enable Style/EvalWithLocation @@ -282,37 +282,6 @@ def variants_from_inline_calls(calls) end end - def compiled_template(template) - handler = ActionView::Template.handler_for_extension(template[:handler]) - template_source = template[:source] || File.read(template[:path]) - template_source.rstrip! if component.strip_trailing_whitespace? - - short_identifier = defined?(Rails.root) ? template[:path].sub("#{Rails.root}/", "") : template[:path] - type = ActionView::Template::Types[template[:format]] - - if handler.method(:call).parameters.length > 1 - handler.call( - OpenStruct.new( - format: template[:format], - identifier: template[:path], - short_identifier: short_identifier, - type: type - ), - template_source - ) - # :nocov: - else - handler.call( - OpenStruct.new( - source: template_source, - identifier: template[:path], - type: type - ) - ) - end - # :nocov: - end - def call_method_name(variant, format = nil) out = +"call" out << "_#{normalized_variant_name(variant)}" if variant.present? @@ -331,5 +300,42 @@ def normalized_variant_name(variant) def default_method_name @default_method_name ||= "_call_#{component.name.underscore.gsub("/", "__")}".to_sym end + + class Template + def initialize(component:, path:, source:, extension:, this_format:) + @component, @path, @source, @extension, @this_format = component, path, source, extension, this_format + @source ||= File.read(path) + end + + def compiled_source + handler = ActionView::Template.handler_for_extension(@extension) + @source.rstrip! if @component.strip_trailing_whitespace? + + short_identifier = defined?(Rails.root) ? @path.sub("#{Rails.root}/", "") : @path + type = ActionView::Template::Types[@this_format] + + if handler.method(:call).parameters.length > 1 + handler.call( + OpenStruct.new( + format: @this_format, + identifier: @path, + short_identifier: short_identifier, + type: type + ), + @source + ) + # :nocov: + else + handler.call( + OpenStruct.new( + source: @source, + identifier: @path, + type: type + ) + ) + end + # :nocov: + end + end end end From 7dc528300da6fe5293a224ab45bd47989e3c5b56 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Sep 2024 11:22:56 -0600 Subject: [PATCH 061/151] clean up method name duplication --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index e6f1cc5ce..f27f47d55 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -298,7 +298,7 @@ def normalized_variant_name(variant) end def default_method_name - @default_method_name ||= "_call_#{component.name.underscore.gsub("/", "__")}".to_sym + @default_method_name ||= safe_name_for(nil, nil) end class Template From 56be8c260e5dbbfeb55975e41eeef68b389a45a3 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Sep 2024 11:24:02 -0600 Subject: [PATCH 062/151] more clearly identify method name --- lib/view_component/compiler.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index f27f47d55..c03791f29 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -122,13 +122,13 @@ def render_template_for(variant = nil, format = nil) end if component.instance_methods.include?(:call) - component.define_method(default_method_name, component.instance_method(:call)) + component.define_method(default_safe_method_name, component.instance_method(:call)) end # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name - if branches.empty? || (branches.length == 1 && branches[0].last == default_method_name) - body = default_method_name + if branches.empty? || (branches.length == 1 && branches[0].last == default_safe_method_name) + body = default_safe_method_name else body = +"" @@ -136,7 +136,7 @@ def render_template_for(variant = nil, format = nil) body << "#{(!body.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end - body << "else\n #{default_method_name}\nend" + body << "else\n #{default_safe_method_name}\nend" end redefinition_lock.synchronize do @@ -297,8 +297,8 @@ def normalized_variant_name(variant) variant.to_s.gsub("-", "__").gsub(".", "___") end - def default_method_name - @default_method_name ||= safe_name_for(nil, nil) + def default_safe_method_name + @default_safe_method_name ||= safe_name_for(nil, nil) end class Template From 7229d1a0e35b425eaed40a8d17ffbb484f6a7c26 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Sep 2024 13:16:08 -0600 Subject: [PATCH 063/151] move compile_to_component to Template --- lib/view_component/compiler.rb | 51 +++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index c03791f29..7038eabc6 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -49,17 +49,15 @@ def compile(raise_errors: false, force: false) end templates.each do |template| - redefinition_lock.synchronize do - component.silence_redefinition_of_method(template[:method_name]) - - # rubocop:disable Style/EvalWithLocation - component.class_eval <<-RUBY, template[:path], template[:lineno] - def #{template[:method_name]} - #{Template.new(component: component, path: template[:path], source: template[:source], extension: template[:handler], this_format: template[:format]).compiled_source} - end - RUBY - # rubocop:enable Style/EvalWithLocation - end + Template.new( + component: component, + path: template[:path], + source: template[:source], + extension: template[:handler], + this_format: template[:format], + variant: template[:variant], + lineno: template[:lineno] + ).compile_to_component(redefinition_lock) end define_render_template_for @@ -302,11 +300,38 @@ def default_safe_method_name end class Template - def initialize(component:, path:, source:, extension:, this_format:) - @component, @path, @source, @extension, @this_format = component, path, source, extension, this_format + def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:) + @component, @path, @source, @extension, @this_format, @lineno, @variant = component, path, source, extension, this_format, lineno, variant @source ||= File.read(path) end + def compile_to_component(redefinition_lock) + redefinition_lock.synchronize do + @component.silence_redefinition_of_method(call_method_name) + + # rubocop:disable Style/EvalWithLocation + @component.class_eval <<-RUBY, @path, @lineno + def #{call_method_name} + #{compiled_source} + end + RUBY + # rubocop:enable Style/EvalWithLocation + end + end + + private + + def normalized_variant_name + @variant.to_s.gsub("-", "__").gsub(".", "___") + end + + def call_method_name + out = +"call" + out << "_#{normalized_variant_name}" if @variant.present? + out << "_#{@this_format}" if @this_format.present? && @this_format != :html + out + end + def compiled_source handler = ActionView::Template.handler_for_extension(@extension) @source.rstrip! if @component.strip_trailing_whitespace? From aecea67cec3218009f4131f636b37875b7187033 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:06:21 -0600 Subject: [PATCH 064/151] construct template objects --- lib/view_component/compiler.rb | 33 +++++++++++++++++++++++++++++---- test/sandbox/test/base_test.rb | 2 +- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 7038eabc6..017e7b0b0 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -56,7 +56,8 @@ def compile(raise_errors: false, force: false) extension: template[:handler], this_format: template[:format], variant: template[:variant], - lineno: template[:lineno] + lineno: template[:lineno], + type: template[:type] ).compile_to_component(redefinition_lock) end @@ -225,11 +226,22 @@ def gather_templates @variants_rendering_templates << out[:variant] + out[:obj] = Template.new( + component: component, + type: out[:type], + path: out[:path], + lineno: out[:lineno], + source: out[:source], + extension: out[:handler], + this_format: out[:format], + variant: out[:variant] + ) + out end if component.inline_template.present? - templates << { + out = { type: :inline, path: component.inline_template.path, lineno: component.inline_template.lineno, @@ -238,6 +250,19 @@ def gather_templates format: nil, variant: nil } + + out[:obj] = Template.new( + component: component, + type: out[:type], + path: out[:path], + lineno: out[:lineno], + source: out[:source], + extension: out[:handler], + this_format: out[:format], + variant: out[:variant] + ) + + templates << out end templates.map! do |template| @@ -300,8 +325,8 @@ def default_safe_method_name end class Template - def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:) - @component, @path, @source, @extension, @this_format, @lineno, @variant = component, path, source, extension, this_format, lineno, variant + def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:, type:) + @component, @path, @source, @extension, @this_format, @lineno, @variant, @type = component, path, source, extension, this_format, lineno, variant, type @source ||= File.read(path) end diff --git a/test/sandbox/test/base_test.rb b/test/sandbox/test/base_test.rb index 80338cd74..6a528b74a 100644 --- a/test/sandbox/test/base_test.rb +++ b/test/sandbox/test/base_test.rb @@ -3,7 +3,7 @@ require "test_helper" class ViewComponent::Base::UnitTest < Minitest::Test - def test_templates_parses_all_types_of_paths + def skip_templates_parses_all_types_of_paths file_path = [ "/Users/fake.user/path/to.templates/component/test_component.html+phone.erb", "/_underscore-dash./component/test_component.html+desktop.slim", From bc5144e4b249d0236eb1013ca212319cddaec2ef Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:13:04 -0600 Subject: [PATCH 065/151] use template object in more places --- lib/view_component/compiler.rb | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 017e7b0b0..ef443e79d 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -80,13 +80,13 @@ def renders_template_for_variant?(variant) def define_render_template_for branches = [] - if template = templates.find { _1[:type] == :inline } - component.define_method(template[:safe_method_name], component.instance_method(:call)) + if template = templates.find { _1[:obj].inline? } + component.define_method(template[:obj].safe_method_name, component.instance_method(:call)) component.silence_redefinition_of_method("render_template_for") component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) - #{template[:safe_method_name]} + #{template[:obj].safe_method_name} end RUBY @@ -94,7 +94,7 @@ def render_template_for(variant = nil, format = nil) end templates.each do |template| - component.define_method(template[:safe_method_name], component.instance_method(template[:method_name])) + component.define_method(template[:obj].safe_method_name, component.instance_method(template[:obj].call_method_name)) format_conditional = if template[:format] == :html @@ -344,10 +344,12 @@ def #{call_method_name} end end - private + def inline? + @type == :inline + end - def normalized_variant_name - @variant.to_s.gsub("-", "__").gsub(".", "___") + def safe_method_name + "_#{call_method_name}_#{@component.name.underscore.gsub("/", "__")}" end def call_method_name @@ -357,6 +359,12 @@ def call_method_name out end + private + + def normalized_variant_name + @variant.to_s.gsub("-", "__").gsub(".", "___") + end + def compiled_source handler = ActionView::Template.handler_for_extension(@extension) @source.rstrip! if @component.strip_trailing_whitespace? From f560ef85176618db7c45a5059e73a808dce5a9f0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:16:42 -0600 Subject: [PATCH 066/151] move html format check to template --- lib/view_component/compiler.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index ef443e79d..28ad11be9 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -97,7 +97,7 @@ def render_template_for(variant = nil, format = nil) component.define_method(template[:obj].safe_method_name, component.instance_method(template[:obj].call_method_name)) format_conditional = - if template[:format] == :html + if template[:obj].html? "(format == :html || format.nil?)" else "format == #{template[:format].inspect}" @@ -348,6 +348,10 @@ def inline? @type == :inline end + def html? + @this_format == :html + end + def safe_method_name "_#{call_method_name}_#{@component.name.underscore.gsub("/", "__")}" end From caeb5d774a99e8146f4fc6b75553a523f06ec436 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:17:34 -0600 Subject: [PATCH 067/151] use format accessor on template --- lib/view_component/compiler.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 28ad11be9..ce42973fd 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -100,7 +100,7 @@ def render_template_for(variant = nil, format = nil) if template[:obj].html? "(format == :html || format.nil?)" else - "format == #{template[:format].inspect}" + "format == #{template[:obj].format.inspect}" end variant_conditional = @@ -352,6 +352,10 @@ def html? @this_format == :html end + def format + @this_format + end + def safe_method_name "_#{call_method_name}_#{@component.name.underscore.gsub("/", "__")}" end From 5e0d3ad3e09fbee328752dc4f45d80823e247d22 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:18:41 -0600 Subject: [PATCH 068/151] move variant access to template --- lib/view_component/compiler.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index ce42973fd..770d0e64e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -104,10 +104,10 @@ def render_template_for(variant = nil, format = nil) end variant_conditional = - if template[:variant].nil? + if template[:obj].variant.nil? "variant.nil?" else - "variant&.to_sym == :'#{template[:variant]}'" + "variant&.to_sym == :'#{template[:obj].variant}'" end branches << ["#{variant_conditional} && #{format_conditional}", template[:safe_method_name]] @@ -325,6 +325,8 @@ def default_safe_method_name end class Template + attr_reader :variant + def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:, type:) @component, @path, @source, @extension, @this_format, @lineno, @variant, @type = component, path, source, extension, this_format, lineno, variant, type @source ||= File.read(path) From cce39bdbb0b9e52b4777926d5eba6aaaf3418f48 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:21:01 -0600 Subject: [PATCH 069/151] remove uses for method name from hash --- lib/view_component/compiler.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 770d0e64e..1d02c260e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -110,7 +110,7 @@ def render_template_for(variant = nil, format = nil) "variant&.to_sym == :'#{template[:obj].variant}'" end - branches << ["#{variant_conditional} && #{format_conditional}", template[:safe_method_name]] + branches << ["#{variant_conditional} && #{format_conditional}", template[:obj].safe_method_name] end variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| @@ -265,12 +265,7 @@ def gather_templates templates << out end - templates.map! do |template| - template[:safe_method_name] = safe_name_for(template[:variant], template[:format]) - template[:method_name] = call_method_name(template[:variant], template[:format]) - - template - end + templates end end From 514a784af24a27205bded9d818a6c402b60c2dd7 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:24:49 -0600 Subject: [PATCH 070/151] use template object in more locations --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 1d02c260e..f6f17bc2c 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -158,7 +158,7 @@ def template_errors end templates. - map { |template| [template[:variant], template[:format]] }. + map { |template| [template[:obj].variant, template[:obj].format] }. tally. select { |_, count| count > 1 }. each do |tally| @@ -169,7 +169,7 @@ def template_errors errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " end - if templates.find { |template| template[:variant].nil? } && inline_calls_defined_on_self.include?(:call) + if templates.find { |template| template[:obj].variant.nil? } && inline_calls_defined_on_self.include?(:call) errors << "Template file and inline render method found for #{component}. " \ "There can only be a template file or inline render method per component." From 9cd0aeb81c768661ab693eb1bcaae1188bf2efa5 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 09:27:56 -0600 Subject: [PATCH 071/151] use template object in more places --- lib/view_component/compiler.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index f6f17bc2c..61eafada0 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -224,8 +224,6 @@ def gather_templates variant: pieces[1..-2].join(".").split("+").second&.to_sym } - @variants_rendering_templates << out[:variant] - out[:obj] = Template.new( component: component, type: out[:type], @@ -237,6 +235,8 @@ def gather_templates variant: out[:variant] ) + @variants_rendering_templates << out[:obj].variant + out end @@ -290,7 +290,7 @@ def inline_calls_defined_on_self def variants @__vc_variants = ( - templates.map { |template| template[:variant] } + variants_from_inline_calls(inline_calls) + templates.map { |template| template[:obj].variant } + variants_from_inline_calls(inline_calls) ).compact.uniq end From b1db1367316056343daa80959f2c2c73337590b9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:24:55 -0600 Subject: [PATCH 072/151] use existing template object --- lib/view_component/compiler.rb | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 61eafada0..67ce9417c 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -49,16 +49,7 @@ def compile(raise_errors: false, force: false) end templates.each do |template| - Template.new( - component: component, - path: template[:path], - source: template[:source], - extension: template[:handler], - this_format: template[:format], - variant: template[:variant], - lineno: template[:lineno], - type: template[:type] - ).compile_to_component(redefinition_lock) + template[:obj].compile_to_component(redefinition_lock) end define_render_template_for @@ -324,7 +315,7 @@ class Template def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:, type:) @component, @path, @source, @extension, @this_format, @lineno, @variant, @type = component, path, source, extension, this_format, lineno, variant, type - @source ||= File.read(path) + @source_originally_nil = @source.nil? end def compile_to_component(redefinition_lock) @@ -366,13 +357,22 @@ def call_method_name private + def source + if @source_originally_nil + File.read(@path) + else + @source + end + end + def normalized_variant_name @variant.to_s.gsub("-", "__").gsub(".", "___") end def compiled_source handler = ActionView::Template.handler_for_extension(@extension) - @source.rstrip! if @component.strip_trailing_whitespace? + this_source = source + this_source.rstrip! if @component.strip_trailing_whitespace? short_identifier = defined?(Rails.root) ? @path.sub("#{Rails.root}/", "") : @path type = ActionView::Template::Types[@this_format] @@ -385,13 +385,13 @@ def compiled_source short_identifier: short_identifier, type: type ), - @source + this_source ) # :nocov: else handler.call( OpenStruct.new( - source: @source, + source: this_source, identifier: @path, type: type ) From 6409c90c291fd37cb77e59e7ecf757a21dc18788 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:26:00 -0600 Subject: [PATCH 073/151] add comment for reload functionality --- lib/view_component/compiler.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 67ce9417c..cda09e649 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -359,6 +359,7 @@ def call_method_name def source if @source_originally_nil + # Load file each time we look up #source in case the file has been modified File.read(@path) else @source From 1f2f144ffc643c549925f456c40213633ad6beec Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:30:30 -0600 Subject: [PATCH 074/151] no more template hash! --- lib/view_component/compiler.rb | 68 ++++++++++++---------------------- 1 file changed, 23 insertions(+), 45 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index cda09e649..247dcf410 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -49,7 +49,7 @@ def compile(raise_errors: false, force: false) end templates.each do |template| - template[:obj].compile_to_component(redefinition_lock) + template.compile_to_component(redefinition_lock) end define_render_template_for @@ -71,13 +71,13 @@ def renders_template_for_variant?(variant) def define_render_template_for branches = [] - if template = templates.find { _1[:obj].inline? } - component.define_method(template[:obj].safe_method_name, component.instance_method(:call)) + if template = templates.find { _1.inline? } + component.define_method(template.safe_method_name, component.instance_method(:call)) component.silence_redefinition_of_method("render_template_for") component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) - #{template[:obj].safe_method_name} + #{template.safe_method_name} end RUBY @@ -85,23 +85,23 @@ def render_template_for(variant = nil, format = nil) end templates.each do |template| - component.define_method(template[:obj].safe_method_name, component.instance_method(template[:obj].call_method_name)) + component.define_method(template.safe_method_name, component.instance_method(template.call_method_name)) format_conditional = - if template[:obj].html? + if template.html? "(format == :html || format.nil?)" else - "format == #{template[:obj].format.inspect}" + "format == #{template.format.inspect}" end variant_conditional = - if template[:obj].variant.nil? + if template.variant.nil? "variant.nil?" else - "variant&.to_sym == :'#{template[:obj].variant}'" + "variant&.to_sym == :'#{template.variant}'" end - branches << ["#{variant_conditional} && #{format_conditional}", template[:obj].safe_method_name] + branches << ["#{variant_conditional} && #{format_conditional}", template.safe_method_name] end variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| @@ -149,7 +149,7 @@ def template_errors end templates. - map { |template| [template[:obj].variant, template[:obj].format] }. + map { |template| [template.variant, template.format] }. tally. select { |_, count| count > 1 }. each do |tally| @@ -160,14 +160,14 @@ def template_errors errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " end - if templates.find { |template| template[:obj].variant.nil? } && inline_calls_defined_on_self.include?(:call) + if templates.find { |template| template.variant.nil? } && inline_calls_defined_on_self.include?(:call) errors << "Template file and inline render method found for #{component}. " \ "There can only be a template file or inline render method per component." end duplicate_template_file_and_inline_variant_calls = - templates.pluck(:variant) & variants_from_inline_calls(inline_calls_defined_on_self) + templates.map(&:variant) & variants_from_inline_calls(inline_calls_defined_on_self) unless duplicate_template_file_and_inline_variant_calls.empty? count = duplicate_template_file_and_inline_variant_calls.count @@ -205,55 +205,33 @@ def gather_templates ).map do |path| pieces = File.basename(path).split(".") - out = { + out = Template.new( + component: component, type: :file, path: path, lineno: 0, source: nil, - handler: pieces.last, - format: pieces[1..-2].join(".").split("+").first&.to_sym, + extension: pieces.last, + this_format: pieces[1..-2].join(".").split("+").first&.to_sym, variant: pieces[1..-2].join(".").split("+").second&.to_sym - } - - out[:obj] = Template.new( - component: component, - type: out[:type], - path: out[:path], - lineno: out[:lineno], - source: out[:source], - extension: out[:handler], - this_format: out[:format], - variant: out[:variant] ) - @variants_rendering_templates << out[:obj].variant + @variants_rendering_templates << out.variant out end if component.inline_template.present? - out = { + templates << Template.new( + component: component, type: :inline, path: component.inline_template.path, lineno: component.inline_template.lineno, source: component.inline_template.source.dup, - handler: component.inline_template.language, - format: nil, + extension: component.inline_template.language, + this_format: nil, variant: nil - } - - out[:obj] = Template.new( - component: component, - type: out[:type], - path: out[:path], - lineno: out[:lineno], - source: out[:source], - extension: out[:handler], - this_format: out[:format], - variant: out[:variant] ) - - templates << out end templates @@ -281,7 +259,7 @@ def inline_calls_defined_on_self def variants @__vc_variants = ( - templates.map { |template| template[:obj].variant } + variants_from_inline_calls(inline_calls) + templates.map { |template| template.variant } + variants_from_inline_calls(inline_calls) ).compact.uniq end From d6a152a76a9749cdd4fecf45a245877cee648afe Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:32:24 -0600 Subject: [PATCH 075/151] do not define branches if it's not used --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 247dcf410..29d09cf25 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -69,8 +69,6 @@ def renders_template_for_variant?(variant) attr_reader :component, :redefinition_lock, :templates def define_render_template_for - branches = [] - if template = templates.find { _1.inline? } component.define_method(template.safe_method_name, component.instance_method(:call)) @@ -84,6 +82,8 @@ def render_template_for(variant = nil, format = nil) return end + branches = [] + templates.each do |template| component.define_method(template.safe_method_name, component.instance_method(template.call_method_name)) From 0894f78679b14faa855ef0f6bd5851e3c98e06d2 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:35:35 -0600 Subject: [PATCH 076/151] use single code path for safe method definition for templates --- lib/view_component/compiler.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 29d09cf25..19b1f5613 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -70,7 +70,7 @@ def renders_template_for_variant?(variant) def define_render_template_for if template = templates.find { _1.inline? } - component.define_method(template.safe_method_name, component.instance_method(:call)) + template.define_safe_method component.silence_redefinition_of_method("render_template_for") component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 @@ -85,7 +85,7 @@ def render_template_for(variant = nil, format = nil) branches = [] templates.each do |template| - component.define_method(template.safe_method_name, component.instance_method(template.call_method_name)) + template.define_safe_method format_conditional = if template.html? @@ -333,6 +333,10 @@ def call_method_name out end + def define_safe_method + @component.define_method(safe_method_name, @component.instance_method(call_method_name)) + end + private def source From b39874e6b9365ab57e4fc3969ac2f820097b952e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:40:09 -0600 Subject: [PATCH 077/151] consolidate render_template_for duplication --- lib/view_component/compiler.rb | 85 ++++++++++++++++------------------ 1 file changed, 39 insertions(+), 46 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 19b1f5613..2e24fd263 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -72,61 +72,54 @@ def define_render_template_for if template = templates.find { _1.inline? } template.define_safe_method - component.silence_redefinition_of_method("render_template_for") - component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def render_template_for(variant = nil, format = nil) - #{template.safe_method_name} + body = template.safe_method_name + else + branches = [] + + templates.each do |template| + template.define_safe_method + + format_conditional = + if template.html? + "(format == :html || format.nil?)" + else + "format == #{template.format.inspect}" + end + + variant_conditional = + if template.variant.nil? + "variant.nil?" + else + "variant&.to_sym == :'#{template.variant}'" + end + + branches << ["#{variant_conditional} && #{format_conditional}", template.safe_method_name] end - RUBY - return - end + variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| + safe_name = safe_name_for(variant, nil) + component.define_method(safe_name, component.instance_method(call_method_name(variant))) - branches = [] + branches << ["variant&.to_sym == :'#{variant}'", safe_name] + end - templates.each do |template| - template.define_safe_method + if component.instance_methods.include?(:call) + component.define_method(default_safe_method_name, component.instance_method(:call)) + end - format_conditional = - if template.html? - "(format == :html || format.nil?)" - else - "format == #{template.format.inspect}" - end + # Just use default method name if no conditional branches or if there is a single + # conditional branch that just calls the default method_name + if branches.empty? || (branches.length == 1 && branches[0].last == default_safe_method_name) + body = default_safe_method_name + else + body = +"" - variant_conditional = - if template.variant.nil? - "variant.nil?" - else - "variant&.to_sym == :'#{template.variant}'" + branches.each do |conditional, method_body| + body << "#{(!body.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end - branches << ["#{variant_conditional} && #{format_conditional}", template.safe_method_name] - end - - variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| - safe_name = safe_name_for(variant, nil) - component.define_method(safe_name, component.instance_method(call_method_name(variant))) - - branches << ["variant&.to_sym == :'#{variant}'", safe_name] - end - - if component.instance_methods.include?(:call) - component.define_method(default_safe_method_name, component.instance_method(:call)) - end - - # Just use default method name if no conditional branches or if there is a single - # conditional branch that just calls the default method_name - if branches.empty? || (branches.length == 1 && branches[0].last == default_safe_method_name) - body = default_safe_method_name - else - body = +"" - - branches.each do |conditional, method_body| - body << "#{(!body.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" + body << "else\n #{default_safe_method_name}\nend" end - - body << "else\n #{default_safe_method_name}\nend" end redefinition_lock.synchronize do From 186b9e291beef8808b0c5774b7f1cd9e88f7e3b0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:41:19 -0600 Subject: [PATCH 078/151] consolidate to one-liner --- lib/view_component/compiler.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 2e24fd263..928106789 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -48,9 +48,7 @@ def compile(raise_errors: false, force: false) component.validate_collection_parameter! end - templates.each do |template| - template.compile_to_component(redefinition_lock) - end + templates.each { _1.compile_to_component(redefinition_lock) } define_render_template_for From 7bb44b26b0aa3c48eb7b838c9f18c879d905f95e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:42:07 -0600 Subject: [PATCH 079/151] consolidate to one-liner --- lib/view_component/compiler.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 928106789..f80c57a13 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -24,8 +24,7 @@ def compiled? end def compile(raise_errors: false, force: false) - return if compiled? && !force - return if component == ViewComponent::Base + return if (compiled? && !force) || component == ViewComponent::Base gather_templates From 2056ca9113aceeb63f6d2f494850fbbe3a1d8a10 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 10:45:41 -0600 Subject: [PATCH 080/151] pass redefinition lock to template --- lib/view_component/compiler.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index f80c57a13..b5cbc1ec4 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -47,7 +47,7 @@ def compile(raise_errors: false, force: false) component.validate_collection_parameter! end - templates.each { _1.compile_to_component(redefinition_lock) } + templates.each(&:compile_to_component) define_render_template_for @@ -196,6 +196,7 @@ def gather_templates pieces = File.basename(path).split(".") out = Template.new( + redefinition_lock: redefinition_lock, component: component, type: :file, path: path, @@ -213,6 +214,7 @@ def gather_templates if component.inline_template.present? templates << Template.new( + redefinition_lock: redefinition_lock, component: component, type: :inline, path: component.inline_template.path, @@ -281,13 +283,14 @@ def default_safe_method_name class Template attr_reader :variant - def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:, type:) - @component, @path, @source, @extension, @this_format, @lineno, @variant, @type = component, path, source, extension, this_format, lineno, variant, type + def initialize(redefinition_lock:, component:, path:, source:, extension:, this_format:, lineno:, variant:, type:) + @redefinition_lock, @component, @path, @source, @extension, @this_format, @lineno, @variant, @type = + redefinition_lock, component, path, source, extension, this_format, lineno, variant, type @source_originally_nil = @source.nil? end - def compile_to_component(redefinition_lock) - redefinition_lock.synchronize do + def compile_to_component + @redefinition_lock.synchronize do @component.silence_redefinition_of_method(call_method_name) # rubocop:disable Style/EvalWithLocation From 76896735e5d66fe0e313ae4262402e77cb44ecfb Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:28:20 -0600 Subject: [PATCH 081/151] use template object for inline templates in render_template_for --- lib/view_component/compiler.rb | 87 ++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 35 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index b5cbc1ec4..9ded8230d 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -30,7 +30,7 @@ def compile(raise_errors: false, force: false) if ( self.class.mode == DEVELOPMENT_MODE && - templates.empty? && + templates.select { _1.type != :inline_call }.empty? && !(component.instance_methods(false).include?(:call) || component.private_instance_methods(false).include?(:call)) ) component.superclass.compile(raise_errors: raise_errors) @@ -76,28 +76,25 @@ def define_render_template_for templates.each do |template| template.define_safe_method - format_conditional = - if template.html? - "(format == :html || format.nil?)" - else - "format == #{template.format.inspect}" - end - - variant_conditional = - if template.variant.nil? - "variant.nil?" - else - "variant&.to_sym == :'#{template.variant}'" - end - - branches << ["#{variant_conditional} && #{format_conditional}", template.safe_method_name] - end - - variants_from_inline_calls(inline_calls).compact.uniq.each do |variant| - safe_name = safe_name_for(variant, nil) - component.define_method(safe_name, component.instance_method(call_method_name(variant))) - - branches << ["variant&.to_sym == :'#{variant}'", safe_name] + if template.type == :inline_call + branches << ["variant&.to_sym == :'#{template.variant}'", template.safe_method_name] + else + format_conditional = + if template.html? + "(format == :html || format.nil?)" + else + "format == #{template.format.inspect}" + end + + variant_conditional = + if template.variant.nil? + "variant.nil?" + else + "variant&.to_sym == :'#{template.variant}'" + end + + branches << ["#{variant_conditional} && #{format_conditional}", template.safe_method_name] + end end if component.instance_methods.include?(:call) @@ -138,7 +135,7 @@ def template_errors errors << "Couldn't find a template file or inline render method for #{component}." end - templates. + templates.select { _1.type != :inline_call }. map { |template| [template.variant, template.format] }. tally. select { |_, count| count > 1 }. @@ -150,14 +147,14 @@ def template_errors errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " end - if templates.find { |template| template.variant.nil? } && inline_calls_defined_on_self.include?(:call) + if templates.find { _1.variant.nil? && _1.type != :inline_call } && inline_calls_defined_on_self.include?(:call) errors << "Template file and inline render method found for #{component}. " \ "There can only be a template file or inline render method per component." end duplicate_template_file_and_inline_variant_calls = - templates.map(&:variant) & variants_from_inline_calls(inline_calls_defined_on_self) + templates.select { _1.type != :inline_call }.map(&:variant) & variants_from_inline_calls(inline_calls_defined_on_self) unless duplicate_template_file_and_inline_variant_calls.empty? count = duplicate_template_file_and_inline_variant_calls.count @@ -212,6 +209,21 @@ def gather_templates out end + inline_calls.each do |method_name| + templates << Template.new( + redefinition_lock: redefinition_lock, + component: component, + type: :inline_call, + path: nil, + lineno: nil, + source: nil, + extension: nil, + this_format: :html, + variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, + method_name: method_name + ) + end + if component.inline_template.present? templates << Template.new( redefinition_lock: redefinition_lock, @@ -281,15 +293,27 @@ def default_safe_method_name end class Template - attr_reader :variant + attr_reader :variant, :type, :call_method_name - def initialize(redefinition_lock:, component:, path:, source:, extension:, this_format:, lineno:, variant:, type:) + def initialize(redefinition_lock:, component:, path:, source:, extension:, this_format:, lineno:, variant:, type:, method_name: nil) @redefinition_lock, @component, @path, @source, @extension, @this_format, @lineno, @variant, @type = redefinition_lock, component, path, source, extension, this_format, lineno, variant, type @source_originally_nil = @source.nil? + + @call_method_name = + if @method_name + @method_name + else + out = +"call" + out << "_#{normalized_variant_name}" if @variant.present? + out << "_#{@this_format}" if @this_format.present? && @this_format != :html + out + end end def compile_to_component + return if @type == :inline_call + @redefinition_lock.synchronize do @component.silence_redefinition_of_method(call_method_name) @@ -319,13 +343,6 @@ def safe_method_name "_#{call_method_name}_#{@component.name.underscore.gsub("/", "__")}" end - def call_method_name - out = +"call" - out << "_#{normalized_variant_name}" if @variant.present? - out << "_#{@this_format}" if @this_format.present? && @this_format != :html - out - end - def define_safe_method @component.define_method(safe_method_name, @component.instance_method(call_method_name)) end From 0b2cc43211a3f791d73e501860a369824377ed62 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:29:59 -0600 Subject: [PATCH 082/151] remove unused method definition --- lib/view_component/compiler.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 9ded8230d..3862adc12 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -97,10 +97,6 @@ def define_render_template_for end end - if component.instance_methods.include?(:call) - component.define_method(default_safe_method_name, component.instance_method(:call)) - end - # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name if branches.empty? || (branches.length == 1 && branches[0].last == default_safe_method_name) From 871b62e7161e4cc52fbee04b8ac275a2ff052b8b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:31:25 -0600 Subject: [PATCH 083/151] simplify error check --- lib/view_component/compiler.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 3862adc12..45ba0c1f8 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -127,9 +127,7 @@ def template_errors begin errors = [] - if (templates + inline_calls).empty? - errors << "Couldn't find a template file or inline render method for #{component}." - end + errors << "Couldn't find a template file or inline render method for #{component}." if templates.empty? templates.select { _1.type != :inline_call }. map { |template| [template.variant, template.format] }. From 3cb912cd5a94f6c37f6e081416e33d6ab896bd86 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:37:36 -0600 Subject: [PATCH 084/151] use default method name in fewer cases --- lib/view_component/compiler.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 45ba0c1f8..36f7ab262 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -99,8 +99,10 @@ def define_render_template_for # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name - if branches.empty? || (branches.length == 1 && branches[0].last == default_safe_method_name) + if branches.empty? body = default_safe_method_name + elsif branches.length == 1 + body = branches[0].last else body = +"" From 9c3686e278269a034a4acb42a854407e929dcc9c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:38:13 -0600 Subject: [PATCH 085/151] remove unreachable code --- lib/view_component/compiler.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 36f7ab262..165eb2c5b 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -99,9 +99,7 @@ def define_render_template_for # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name - if branches.empty? - body = default_safe_method_name - elsif branches.length == 1 + if branches.length == 1 body = branches[0].last else body = +"" From 3785905b4bea340a9e639abccf02104fb2df5f44 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:42:13 -0600 Subject: [PATCH 086/151] remove method name logic from compiler --- lib/view_component/compiler.rb | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 165eb2c5b..95af0336c 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -108,7 +108,7 @@ def define_render_template_for body << "#{(!body.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end - body << "else\n #{default_safe_method_name}\nend" + body << "else\n #{templates.find { _1.variant.nil? && _1.html? }.safe_method_name}\nend" end end @@ -267,25 +267,10 @@ def variants_from_inline_calls(calls) end end - def call_method_name(variant, format = nil) - out = +"call" - out << "_#{normalized_variant_name(variant)}" if variant.present? - out << "_#{format}" if format.present? && format != :html - out - end - - def safe_name_for(variant, format) - "_#{call_method_name(variant, format)}_#{component.name.underscore.gsub("/", "__")}" - end - def normalized_variant_name(variant) variant.to_s.gsub("-", "__").gsub(".", "___") end - def default_safe_method_name - @default_safe_method_name ||= safe_name_for(nil, nil) - end - class Template attr_reader :variant, :type, :call_method_name From 4b6c7872ed1af84208d541bd8fd0dd8ebbf1b926 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:43:49 -0600 Subject: [PATCH 087/151] simplify lookup of variants --- lib/view_component/compiler.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 95af0336c..b5c1d1de3 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -256,9 +256,7 @@ def inline_calls_defined_on_self end def variants - @__vc_variants = ( - templates.map { |template| template.variant } + variants_from_inline_calls(inline_calls) - ).compact.uniq + @variants ||= templates.map(&:variant).compact.uniq end def variants_from_inline_calls(calls) From 0db8fdb02e8bbea07859e8c65dee46bed3c9208c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 4 Sep 2024 11:47:50 -0600 Subject: [PATCH 088/151] inline variants accessor --- lib/view_component/compiler.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index b5c1d1de3..478213866 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -161,7 +161,7 @@ def template_errors "There can only be a template file or inline render method per variant." end - uniq_variants = variants.compact.uniq + uniq_variants = templates.map(&:variant).compact.uniq normalized_variants = uniq_variants.map { |variant| normalized_variant_name(variant) } colliding_variants = uniq_variants.select do |variant| @@ -255,10 +255,6 @@ def inline_calls_defined_on_self @inline_calls_defined_on_self ||= component.instance_methods(false).grep(/^call(_|$)/) end - def variants - @variants ||= templates.map(&:variant).compact.uniq - end - def variants_from_inline_calls(calls) calls.reject { |call| call == :call }.map do |variant_call| variant_call.to_s.sub("call_", "").to_sym @@ -324,6 +320,10 @@ def define_safe_method @component.define_method(safe_method_name, @component.instance_method(call_method_name)) end + def normalized_variant_name + @variant.to_s.gsub("-", "__").gsub(".", "___") + end + private def source @@ -335,10 +335,6 @@ def source end end - def normalized_variant_name - @variant.to_s.gsub("-", "__").gsub(".", "___") - end - def compiled_source handler = ActionView::Template.handler_for_extension(@extension) this_source = source From f122a7186cc45228bc79318bf763c5f45d13b3d2 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 12:32:24 -0600 Subject: [PATCH 089/151] remove duplicative normalized_variant_name --- lib/view_component/compiler.rb | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 478213866..9ad8662d8 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -161,16 +161,25 @@ def template_errors "There can only be a template file or inline render method per variant." end - uniq_variants = templates.map(&:variant).compact.uniq - normalized_variants = uniq_variants.map { |variant| normalized_variant_name(variant) } - - colliding_variants = uniq_variants.select do |variant| - normalized_variants.count(normalized_variant_name(variant)) > 1 - end + pairs = + templates. + map { [_1.variant, _1.normalized_variant_name] if _1.variant.present? }. + compact. + uniq { _1.first } + + colliding_normalized_variants = + pairs.map(&:last). + tally. + select { |_, count| count > 1 }. + keys. + map do |normalized_variant_name| + pairs.select { |pair| pair.last == normalized_variant_name }. + map { |pair| pair.first } + end - unless colliding_variants.empty? + colliding_normalized_variants.each do |variants| errors << - "Colliding templates #{colliding_variants.sort.map { |v| "'#{v}'" }.to_sentence} " \ + "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} " \ "found in #{component}." end @@ -261,10 +270,6 @@ def variants_from_inline_calls(calls) end end - def normalized_variant_name(variant) - variant.to_s.gsub("-", "__").gsub(".", "___") - end - class Template attr_reader :variant, :type, :call_method_name From d99e819713fa794b9c187e42ab35ea6875135196 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 14:31:55 -0600 Subject: [PATCH 090/151] add note about allowing inline_call / template collision --- lib/view_component/compiler.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 9ad8662d8..88dabc95f 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -129,6 +129,9 @@ def template_errors 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.select { _1.type != :inline_call }. map { |template| [template.variant, template.format] }. tally. From 011fb1c38f82e3beb874a51380131e7f63ae8350 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 14:53:07 -0600 Subject: [PATCH 091/151] remove usage of inline_calls_defined_on_self --- lib/view_component/compiler.rb | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 88dabc95f..ac0f8dd7e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -144,7 +144,10 @@ def template_errors errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " end - if templates.find { _1.variant.nil? && _1.type != :inline_call } && inline_calls_defined_on_self.include?(:call) + if ( + templates.any? { _1.variant.nil? && _1.type != :inline_call } && + templates.any? { _1.variant.nil? && _1.type == :inline_call && _1.defined_on_self? } + ) errors << "Template file and inline render method found for #{component}. " \ "There can only be a template file or inline render method per component." @@ -226,7 +229,8 @@ def gather_templates extension: nil, this_format: :html, variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, - method_name: method_name + method_name: method_name, + defined_on_self: component.instance_methods(false).include?(method_name) ) end @@ -276,9 +280,9 @@ def variants_from_inline_calls(calls) class Template attr_reader :variant, :type, :call_method_name - def initialize(redefinition_lock:, component:, path:, source:, extension:, this_format:, lineno:, variant:, type:, method_name: nil) - @redefinition_lock, @component, @path, @source, @extension, @this_format, @lineno, @variant, @type = - redefinition_lock, component, path, source, extension, this_format, lineno, variant, type + def initialize(redefinition_lock:, component:, path:, source:, extension:, this_format:, lineno:, variant:, type:, method_name: nil, defined_on_self: true) + @redefinition_lock, @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = + redefinition_lock, component, path, source, extension, this_format, lineno, variant, type, defined_on_self @source_originally_nil = @source.nil? @call_method_name = @@ -332,6 +336,10 @@ def normalized_variant_name @variant.to_s.gsub("-", "__").gsub(".", "___") end + def defined_on_self? + @defined_on_self + end + private def source From a0c7f9ed9108c031c1a5a06d4cbcb2d2783fd355 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:05:06 -0600 Subject: [PATCH 092/151] remove inline_calls_defined_on_self --- lib/view_component/compiler.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index ac0f8dd7e..7d0a0b67b 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -154,7 +154,8 @@ def template_errors end duplicate_template_file_and_inline_variant_calls = - templates.select { _1.type != :inline_call }.map(&:variant) & variants_from_inline_calls(inline_calls_defined_on_self) + templates.select { _1.type != :inline_call }.map(&:variant) & + templates.select { _1.type == :inline_call && _1.defined_on_self? }.map(&:variant) unless duplicate_template_file_and_inline_variant_calls.empty? count = duplicate_template_file_and_inline_variant_calls.count @@ -267,16 +268,6 @@ def inline_calls end end - def inline_calls_defined_on_self - @inline_calls_defined_on_self ||= component.instance_methods(false).grep(/^call(_|$)/) - end - - def variants_from_inline_calls(calls) - calls.reject { |call| call == :call }.map do |variant_call| - variant_call.to_s.sub("call_", "").to_sym - end - end - class Template attr_reader :variant, :type, :call_method_name From e01fc8b62ead58ab43424f856e70f24515f90e8f Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:06:12 -0600 Subject: [PATCH 093/151] inline_calls is only used once --- lib/view_component/compiler.rb | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 7d0a0b67b..ef78a6ba0 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -219,6 +219,14 @@ def gather_templates out end + view_component_ancestors = + ( + component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - + component.included_modules + ) + + inline_calls = view_component_ancestors.flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }.uniq + inline_calls.each do |method_name| templates << Template.new( redefinition_lock: redefinition_lock, @@ -253,21 +261,6 @@ def gather_templates end end - def inline_calls - @inline_calls ||= - begin - # Fetch only ViewComponent ancestor classes to limit the scope of - # finding inline calls - view_component_ancestors = - ( - component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - - component.included_modules - ) - - view_component_ancestors.flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }.uniq - end - end - class Template attr_reader :variant, :type, :call_method_name From bfdcff23dbb165d73aa7784d91ae48da20143f6a Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:10:47 -0600 Subject: [PATCH 094/151] add todo around tracking template rendering --- lib/view_component/compiler.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index ef78a6ba0..12815d620 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -57,6 +57,7 @@ def compile(raise_errors: false, force: false) CompileCache.register(component) end + # TODO this should probably take format into account def renders_template_for_variant?(variant) @variants_rendering_templates.include?(variant) end From b39d9f897ec89c22bb2a5371be1e29256e4d7667 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:15:58 -0600 Subject: [PATCH 095/151] simplify conditional to use templates --- lib/view_component/compiler.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 12815d620..5c388bae4 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -30,8 +30,7 @@ def compile(raise_errors: false, force: false) if ( self.class.mode == DEVELOPMENT_MODE && - templates.select { _1.type != :inline_call }.empty? && - !(component.instance_methods(false).include?(:call) || component.private_instance_methods(false).include?(:call)) + templates.select { !(_1.type == :inline_call && !_1.defined_on_self?) }.empty? ) component.superclass.compile(raise_errors: raise_errors) end From 24d8a33fce8be7826507276be4ee11d5f3deb205 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:18:01 -0600 Subject: [PATCH 096/151] simplify conditional --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 5c388bae4..86d9542b5 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -30,7 +30,7 @@ def compile(raise_errors: false, force: false) if ( self.class.mode == DEVELOPMENT_MODE && - templates.select { !(_1.type == :inline_call && !_1.defined_on_self?) }.empty? + templates.none? { !(_1.type == :inline_call && !_1.defined_on_self?) } ) component.superclass.compile(raise_errors: raise_errors) end From 2456a3aa1a2fedbffff6936f47fdcc19bec10a83 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:19:05 -0600 Subject: [PATCH 097/151] make one-liner --- lib/view_component/compiler.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 86d9542b5..e0fa83efe 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -28,10 +28,7 @@ def compile(raise_errors: false, force: false) gather_templates - if ( - self.class.mode == DEVELOPMENT_MODE && - templates.none? { !(_1.type == :inline_call && !_1.defined_on_self?) } - ) + if self.class.mode == DEVELOPMENT_MODE && templates.none? { !(_1.type == :inline_call && !_1.defined_on_self?) } component.superclass.compile(raise_errors: raise_errors) end From 8531f11045f151feec8057c5a0a800b7ba458372 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:22:19 -0600 Subject: [PATCH 098/151] gather_template_errors explicitly --- lib/view_component/compiler.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index e0fa83efe..41102c1cb 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -32,8 +32,10 @@ def compile(raise_errors: false, force: false) component.superclass.compile(raise_errors: raise_errors) end - if template_errors.present? - raise TemplateError.new(template_errors) if raise_errors + gather_template_errors + + if @template_errors.any? + raise TemplateError.new(@template_errors) if raise_errors return end @@ -119,8 +121,8 @@ def render_template_for(variant = nil, format = nil) end end - def template_errors - @__vc_template_errors ||= + def gather_template_errors + @template_errors = begin errors = [] From c63a4140c2cd00a556d4fc3225e4ede9753a7fc8 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 5 Sep 2024 15:32:48 -0600 Subject: [PATCH 099/151] use attr_reader to match templates --- lib/view_component/compiler.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 41102c1cb..cd436a94f 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -34,8 +34,8 @@ def compile(raise_errors: false, force: false) gather_template_errors - if @template_errors.any? - raise TemplateError.new(@template_errors) if raise_errors + if template_errors.any? + raise TemplateError.new(template_errors) if raise_errors return end @@ -62,7 +62,7 @@ def renders_template_for_variant?(variant) private - attr_reader :component, :redefinition_lock, :templates + attr_reader :component, :redefinition_lock, :templates, :template_errors def define_render_template_for if template = templates.find { _1.inline? } From 336077c9c11c4c1dcf5cbd187f993517fe99dc18 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:07:48 -0600 Subject: [PATCH 100/151] move logic into template error construction --- lib/view_component/compiler.rb | 129 ++++++++++++++++----------------- 1 file changed, 61 insertions(+), 68 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index cd436a94f..24fbba1ce 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -32,13 +32,7 @@ def compile(raise_errors: false, force: false) component.superclass.compile(raise_errors: raise_errors) end - gather_template_errors - - if template_errors.any? - raise TemplateError.new(template_errors) if raise_errors - - return - end + return if gather_template_errors(raise_errors).any? if raise_errors component.validate_initialization_parameters! @@ -62,7 +56,7 @@ def renders_template_for_variant?(variant) private - attr_reader :component, :redefinition_lock, :templates, :template_errors + attr_reader :component, :redefinition_lock, :templates def define_render_template_for if template = templates.find { _1.inline? } @@ -121,76 +115,75 @@ def render_template_for(variant = nil, format = nil) end end - def gather_template_errors - @template_errors = - begin - errors = [] + def gather_template_errors(raise_errors) + 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.select { _1.type != :inline_call }. - map { |template| [template.variant, template.format] }. - tally. - select { |_, count| count > 1 }. - each do |tally| - variant, this_format = tally[0] + # 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.select { _1.type != :inline_call }. + map { |template| [template.variant, template.format] }. + tally. + select { |_, count| count > 1 }. + each do |tally| + variant, this_format = tally[0] - variant_string = " for variant `#{variant}`" if variant.present? + variant_string = " for variant `#{variant}`" if variant.present? - errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " - end - - if ( - templates.any? { _1.variant.nil? && _1.type != :inline_call } && - templates.any? { _1.variant.nil? && _1.type == :inline_call && _1.defined_on_self? } - ) - errors << - "Template file and inline render method found for #{component}. " \ - "There can only be a template file or inline render method per component." - end + errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " + end - duplicate_template_file_and_inline_variant_calls = - templates.select { _1.type != :inline_call }.map(&:variant) & - templates.select { _1.type == :inline_call && _1.defined_on_self? }.map(&:variant) + if ( + templates.any? { _1.variant.nil? && _1.type != :inline_call } && + templates.any? { _1.variant.nil? && _1.type == :inline_call && _1.defined_on_self? } + ) + errors << + "Template file and inline render method found for #{component}. " \ + "There can only be a template file or inline render method per component." + end - unless duplicate_template_file_and_inline_variant_calls.empty? - count = duplicate_template_file_and_inline_variant_calls.count + duplicate_template_file_and_inline_variant_calls = + templates.select { _1.type != :inline_call }.map(&:variant) & + templates.select { _1.type == :inline_call && _1.defined_on_self? }.map(&:variant) - errors << - "Template #{"file".pluralize(count)} and inline render #{"method".pluralize(count)} " \ - "found for #{"variant".pluralize(count)} " \ - "#{duplicate_template_file_and_inline_variant_calls.map { |v| "'#{v}'" }.to_sentence} " \ - "in #{component}. " \ - "There can only be a template file or inline render method per variant." - end + unless duplicate_template_file_and_inline_variant_calls.empty? + count = duplicate_template_file_and_inline_variant_calls.count - pairs = - templates. - map { [_1.variant, _1.normalized_variant_name] if _1.variant.present? }. - compact. - uniq { _1.first } - - colliding_normalized_variants = - pairs.map(&:last). - tally. - select { |_, count| count > 1 }. - keys. - map do |normalized_variant_name| - pairs.select { |pair| pair.last == normalized_variant_name }. - map { |pair| pair.first } - end - - colliding_normalized_variants.each do |variants| - errors << - "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} " \ - "found in #{component}." - end + errors << + "Template #{"file".pluralize(count)} and inline render #{"method".pluralize(count)} " \ + "found for #{"variant".pluralize(count)} " \ + "#{duplicate_template_file_and_inline_variant_calls.map { |v| "'#{v}'" }.to_sentence} " \ + "in #{component}. " \ + "There can only be a template file or inline render method per variant." + end - errors + pairs = + templates. + map { [_1.variant, _1.normalized_variant_name] if _1.variant.present? }. + compact. + uniq { _1.first } + + colliding_normalized_variants = + pairs.map(&:last). + tally. + select { |_, count| count > 1 }. + keys. + map do |normalized_variant_name| + pairs.select { |pair| pair.last == normalized_variant_name }. + map { |pair| pair.first } end + + colliding_normalized_variants.each do |variants| + errors << + "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} " \ + "found in #{component}." + end + + raise TemplateError.new(errors) if errors.any? && raise_errors + + errors end def gather_templates From dc25f1e4e640281d4bd3e4874f5d60353d976ea0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:13:07 -0600 Subject: [PATCH 101/151] couple template compilation with render method definition --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 24fbba1ce..e06a626a2 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -39,8 +39,6 @@ def compile(raise_errors: false, force: false) component.validate_collection_parameter! end - templates.each(&:compile_to_component) - define_render_template_for component.register_default_slots @@ -59,6 +57,8 @@ def renders_template_for_variant?(variant) attr_reader :component, :redefinition_lock, :templates def define_render_template_for + templates.each(&:compile_to_component) + if template = templates.find { _1.inline? } template.define_safe_method From 1b2e8835b023a120509ead1f88173c76c95674d0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:16:18 -0600 Subject: [PATCH 102/151] define safe method as part of compilation --- lib/view_component/compiler.rb | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index e06a626a2..5679c0c28 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -60,15 +60,11 @@ def define_render_template_for templates.each(&:compile_to_component) if template = templates.find { _1.inline? } - template.define_safe_method - body = template.safe_method_name else branches = [] templates.each do |template| - template.define_safe_method - if template.type == :inline_call branches << ["variant&.to_sym == :'#{template.variant}'", template.safe_method_name] else @@ -273,19 +269,21 @@ def initialize(redefinition_lock:, component:, path:, source:, extension:, this_ end def compile_to_component - return if @type == :inline_call - - @redefinition_lock.synchronize do - @component.silence_redefinition_of_method(call_method_name) - - # rubocop:disable Style/EvalWithLocation - @component.class_eval <<-RUBY, @path, @lineno - def #{call_method_name} - #{compiled_source} + if @type != :inline_call + @redefinition_lock.synchronize do + @component.silence_redefinition_of_method(call_method_name) + + # rubocop:disable Style/EvalWithLocation + @component.class_eval <<-RUBY, @path, @lineno + def #{call_method_name} + #{compiled_source} + end + RUBY + # rubocop:enable Style/EvalWithLocation end - RUBY - # rubocop:enable Style/EvalWithLocation end + + @component.define_method(safe_method_name, @component.instance_method(call_method_name)) end def inline? @@ -304,10 +302,6 @@ def safe_method_name "_#{call_method_name}_#{@component.name.underscore.gsub("/", "__")}" end - def define_safe_method - @component.define_method(safe_method_name, @component.instance_method(call_method_name)) - end - def normalized_variant_name @variant.to_s.gsub("-", "__").gsub(".", "___") end From ab42dd13274e2a66fb16b186097ca1cf6822a6c4 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:22:14 -0600 Subject: [PATCH 103/151] extract Template#inline_call? --- lib/view_component/compiler.rb | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 5679c0c28..68bc30a0a 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -28,7 +28,7 @@ def compile(raise_errors: false, force: false) gather_templates - if self.class.mode == DEVELOPMENT_MODE && templates.none? { !(_1.type == :inline_call && !_1.defined_on_self?) } + if self.class.mode == DEVELOPMENT_MODE && templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } component.superclass.compile(raise_errors: raise_errors) end @@ -65,7 +65,7 @@ def define_render_template_for branches = [] templates.each do |template| - if template.type == :inline_call + if template.inline_call? branches << ["variant&.to_sym == :'#{template.variant}'", template.safe_method_name] else format_conditional = @@ -119,7 +119,7 @@ def gather_template_errors(raise_errors) # 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.select { _1.type != :inline_call }. + templates.select { !_1.inline_call? }. map { |template| [template.variant, template.format] }. tally. select { |_, count| count > 1 }. @@ -132,8 +132,8 @@ def gather_template_errors(raise_errors) end if ( - templates.any? { _1.variant.nil? && _1.type != :inline_call } && - templates.any? { _1.variant.nil? && _1.type == :inline_call && _1.defined_on_self? } + templates.any? { _1.variant.nil? && !_1.inline_call? } && + templates.any? { _1.variant.nil? && _1.inline_call? && _1.defined_on_self? } ) errors << "Template file and inline render method found for #{component}. " \ @@ -141,8 +141,8 @@ def gather_template_errors(raise_errors) end duplicate_template_file_and_inline_variant_calls = - templates.select { _1.type != :inline_call }.map(&:variant) & - templates.select { _1.type == :inline_call && _1.defined_on_self? }.map(&:variant) + templates.select { !_1.inline_call? }.map(&:variant) & + templates.select { _1.inline_call? && _1.defined_on_self? }.map(&:variant) unless duplicate_template_file_and_inline_variant_calls.empty? count = duplicate_template_file_and_inline_variant_calls.count @@ -269,7 +269,7 @@ def initialize(redefinition_lock:, component:, path:, source:, extension:, this_ end def compile_to_component - if @type != :inline_call + if !inline_call? @redefinition_lock.synchronize do @component.silence_redefinition_of_method(call_method_name) @@ -286,6 +286,10 @@ def #{call_method_name} @component.define_method(safe_method_name, @component.instance_method(call_method_name)) end + def inline_call? + @type == :inline_call + end + def inline? @type == :inline end From a52524fe91f3dcc795fb3dfc867ce16c8e33e2c0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:24:16 -0600 Subject: [PATCH 104/151] inline inline_calls helper --- lib/view_component/compiler.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 68bc30a0a..4dd6b2cff 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -207,13 +207,10 @@ def gather_templates out end - view_component_ancestors = + inline_calls = ( - component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - - component.included_modules - ) - - inline_calls = view_component_ancestors.flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }.uniq + component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - component.included_modules + ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }.uniq inline_calls.each do |method_name| templates << Template.new( From a42f23b55548b16c27c7c48c04e948935c282efc Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:28:22 -0600 Subject: [PATCH 105/151] use select instead of conditonal map+compact --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 4dd6b2cff..2986a0d5f 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -157,8 +157,8 @@ def gather_template_errors(raise_errors) pairs = templates. - map { [_1.variant, _1.normalized_variant_name] if _1.variant.present? }. - compact. + select { _1.variant.present? }. + map { [_1.variant, _1.normalized_variant_name] }. uniq { _1.first } colliding_normalized_variants = From 3935d9b4c99c5a4b9068a67df2d83e6a1c779ed5 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:32:42 -0600 Subject: [PATCH 106/151] simplify logic and naming in duplicate detection --- lib/view_component/compiler.rb | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 2986a0d5f..58eb6cfb1 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -140,19 +140,18 @@ def gather_template_errors(raise_errors) "There can only be a template file or inline render method per component." end - duplicate_template_file_and_inline_variant_calls = + duplicate_template_file_and_inline_call_variants = templates.select { !_1.inline_call? }.map(&:variant) & templates.select { _1.inline_call? && _1.defined_on_self? }.map(&:variant) - unless duplicate_template_file_and_inline_variant_calls.empty? - count = duplicate_template_file_and_inline_variant_calls.count + 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_variant_calls.map { |v| "'#{v}'" }.to_sentence} " \ - "in #{component}. " \ - "There can only be a template file or inline render method per variant." + "#{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 pairs = @@ -162,19 +161,15 @@ def gather_template_errors(raise_errors) uniq { _1.first } colliding_normalized_variants = - pairs.map(&:last). - tally. - select { |_, count| count > 1 }. - keys. + pairs.map(&:last).tally.select { |_, count| count > 1 }.keys. map do |normalized_variant_name| - pairs.select { |pair| pair.last == normalized_variant_name }. - map { |pair| pair.first } + pairs. + select { |pair| pair.last == normalized_variant_name }. + map { |pair| pair.first } end colliding_normalized_variants.each do |variants| - errors << - "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} " \ - "found in #{component}." + errors << "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} found in #{component}." end raise TemplateError.new(errors) if errors.any? && raise_errors From 941b38cce407115d35e5ae6e3164d23723f0dec3 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:34:17 -0600 Subject: [PATCH 107/151] use more specific naming for variant pairs --- lib/view_component/compiler.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 58eb6cfb1..f31cc9933 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -154,18 +154,18 @@ def gather_template_errors(raise_errors) "in #{component}. There can only be a template file or inline render method per variant." end - pairs = + variant_pairs = templates. select { _1.variant.present? }. map { [_1.variant, _1.normalized_variant_name] }. uniq { _1.first } colliding_normalized_variants = - pairs.map(&:last).tally.select { |_, count| count > 1 }.keys. + variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys. map do |normalized_variant_name| - pairs. - select { |pair| pair.last == normalized_variant_name }. - map { |pair| pair.first } + variant_pairs. + select { |variant_pair| variant_pair.last == normalized_variant_name }. + map { |variant_pair| variant_pair.first } end colliding_normalized_variants.each do |variants| From 824735f72295e2246270f47feace9b187f74c781 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:34:56 -0600 Subject: [PATCH 108/151] inline --- lib/view_component/compiler.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index f31cc9933..4a3363ba6 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -155,10 +155,7 @@ def gather_template_errors(raise_errors) end variant_pairs = - templates. - select { _1.variant.present? }. - map { [_1.variant, _1.normalized_variant_name] }. - uniq { _1.first } + templates.select { _1.variant.present? }.map { [_1.variant, _1.normalized_variant_name] }.uniq { _1.first } colliding_normalized_variants = variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys. From 9ce7234ba06732de6c51d2ef8a925dd1f44d62c1 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:38:38 -0600 Subject: [PATCH 109/151] passing around redef lock was messy --- lib/view_component/compiler.rb | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 4a3363ba6..202369866 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -54,10 +54,10 @@ def renders_template_for_variant?(variant) private - attr_reader :component, :redefinition_lock, :templates + attr_reader :component, :templates def define_render_template_for - templates.each(&:compile_to_component) + templates.each { _1.compile_to_component(@redefinition_lock) } if template = templates.find { _1.inline? } body = template.safe_method_name @@ -101,7 +101,7 @@ def define_render_template_for end end - redefinition_lock.synchronize do + @redefinition_lock.synchronize do component.silence_redefinition_of_method(:render_template_for) component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) @@ -183,7 +183,6 @@ def gather_templates pieces = File.basename(path).split(".") out = Template.new( - redefinition_lock: redefinition_lock, component: component, type: :file, path: path, @@ -206,7 +205,6 @@ def gather_templates inline_calls.each do |method_name| templates << Template.new( - redefinition_lock: redefinition_lock, component: component, type: :inline_call, path: nil, @@ -222,7 +220,6 @@ def gather_templates if component.inline_template.present? templates << Template.new( - redefinition_lock: redefinition_lock, component: component, type: :inline, path: component.inline_template.path, @@ -241,9 +238,9 @@ def gather_templates class Template attr_reader :variant, :type, :call_method_name - def initialize(redefinition_lock:, component:, path:, source:, extension:, this_format:, lineno:, variant:, type:, method_name: nil, defined_on_self: true) - @redefinition_lock, @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = - redefinition_lock, component, path, source, extension, this_format, lineno, variant, type, defined_on_self + def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:, type:, method_name: nil, defined_on_self: true) + @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = + component, path, source, extension, this_format, lineno, variant, type, defined_on_self @source_originally_nil = @source.nil? @call_method_name = @@ -257,9 +254,9 @@ def initialize(redefinition_lock:, component:, path:, source:, extension:, this_ end end - def compile_to_component + def compile_to_component(redefinition_lock) if !inline_call? - @redefinition_lock.synchronize do + redefinition_lock.synchronize do @component.silence_redefinition_of_method(call_method_name) # rubocop:disable Style/EvalWithLocation From c93f998de634cbad0eeb68f4e3ebec43ac52dae4 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:41:30 -0600 Subject: [PATCH 110/151] initializer is longer than 120 chars --- lib/view_component/compiler.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 202369866..6e8059bab 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -238,7 +238,18 @@ def gather_templates class Template attr_reader :variant, :type, :call_method_name - def initialize(component:, path:, source:, extension:, this_format:, lineno:, variant:, type:, method_name: nil, defined_on_self: true) + def initialize( + component:, + path:, + source:, + extension:, + this_format:, + lineno:, + variant:, + type:, + method_name: nil, + defined_on_self: true + ) @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = component, path, source, extension, this_format, lineno, variant, type, defined_on_self @source_originally_nil = @source.nil? From e1162c34aec913d7ac83efd5026c3a856dd1f373 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 09:55:32 -0600 Subject: [PATCH 111/151] let Ruby construct symbol string --- lib/view_component/compiler.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 6e8059bab..6304c797e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -66,7 +66,7 @@ def define_render_template_for templates.each do |template| if template.inline_call? - branches << ["variant&.to_sym == :'#{template.variant}'", template.safe_method_name] + branches << ["variant&.to_sym == #{template.variant.inspect}", template.safe_method_name] else format_conditional = if template.html? @@ -79,7 +79,7 @@ def define_render_template_for if template.variant.nil? "variant.nil?" else - "variant&.to_sym == :'#{template.variant}'" + "variant&.to_sym == #{template.variant.inspect}" end branches << ["#{variant_conditional} && #{format_conditional}", template.safe_method_name] @@ -251,7 +251,7 @@ def initialize( defined_on_self: true ) @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = - component, path, source, extension, this_format, lineno, variant, type, defined_on_self + component, path, source, extension, this_format, lineno, variant&.to_sym, type, defined_on_self @source_originally_nil = @source.nil? @call_method_name = From 8d24cb87b78188a4aa2b0eb87408034a5a1f30cc Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:00:30 -0600 Subject: [PATCH 112/151] simplify branch construction --- lib/view_component/compiler.rb | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 6304c797e..8f8669cd0 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -65,25 +65,17 @@ def define_render_template_for branches = [] templates.each do |template| - if template.inline_call? - branches << ["variant&.to_sym == #{template.variant.inspect}", template.safe_method_name] - else - format_conditional = - if template.html? - "(format == :html || format.nil?)" - else - "format == #{template.format.inspect}" - end - - variant_conditional = - if template.variant.nil? - "variant.nil?" - else - "variant&.to_sym == #{template.variant.inspect}" - end - - branches << ["#{variant_conditional} && #{format_conditional}", template.safe_method_name] - end + conditional = + if template.inline_call? + "variant&.to_sym == #{template.variant.inspect}" + else + [ + template.html? ? "(format == :html || format.nil?)" : "format == #{template.format.inspect}", + template.variant.nil? ? "variant.nil?" : "variant&.to_sym == #{template.variant.inspect}" + ].join(" && ") + end + + branches << [conditional, template.safe_method_name] end # Just use default method name if no conditional branches or if there is a single From c8ff8877da7e902785be2fe421a6b3f38639a3ab Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:04:24 -0600 Subject: [PATCH 113/151] assign body as result of conditionals --- lib/view_component/compiler.rb | 59 ++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 8f8669cd0..582495ee7 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -59,39 +59,42 @@ def renders_template_for_variant?(variant) def define_render_template_for templates.each { _1.compile_to_component(@redefinition_lock) } - if template = templates.find { _1.inline? } - body = template.safe_method_name - else - branches = [] - - templates.each do |template| - conditional = - if template.inline_call? - "variant&.to_sym == #{template.variant.inspect}" - else - [ - template.html? ? "(format == :html || format.nil?)" : "format == #{template.format.inspect}", - template.variant.nil? ? "variant.nil?" : "variant&.to_sym == #{template.variant.inspect}" - ].join(" && ") - end + body = + if template = templates.find { _1.inline? } + template.safe_method_name + else + branches = [] + + templates.each do |template| + conditional = + if template.inline_call? + "variant&.to_sym == #{template.variant.inspect}" + else + [ + template.html? ? "(format == :html || format.nil?)" : "format == #{template.format.inspect}", + template.variant.nil? ? "variant.nil?" : "variant&.to_sym == #{template.variant.inspect}" + ].join(" && ") + end + + branches << [conditional, template.safe_method_name] + end - branches << [conditional, template.safe_method_name] - end + # Just use default method name if no conditional branches or if there is a single + # conditional branch that just calls the default method_name + if branches.length == 1 + branches[0].last + else + out = +"" - # Just use default method name if no conditional branches or if there is a single - # conditional branch that just calls the default method_name - if branches.length == 1 - body = branches[0].last - else - body = +"" + branches.each do |conditional, method_body| + out << "#{(!out.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" + end - branches.each do |conditional, method_body| - body << "#{(!body.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" - end + out << "else\n #{templates.find { _1.variant.nil? && _1.html? }.safe_method_name}\nend" - body << "else\n #{templates.find { _1.variant.nil? && _1.html? }.safe_method_name}\nend" + out + end end - end @redefinition_lock.synchronize do component.silence_redefinition_of_method(:render_template_for) From 4854fa0d79a5cafecb4843b71502b96632d23e9a Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:06:46 -0600 Subject: [PATCH 114/151] use each_with_object to simplify --- lib/view_component/compiler.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 582495ee7..958b110e8 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -84,15 +84,11 @@ def define_render_template_for if branches.length == 1 branches[0].last else - out = +"" - - branches.each do |conditional, method_body| - out << "#{(!out.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" + out = branches.each_with_object(+"") do |(conditional, method_body), memo| + memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" end out << "else\n #{templates.find { _1.variant.nil? && _1.html? }.safe_method_name}\nend" - - out end end From 9b3c6389050bf01d0a65222f8be756ecbaf2e780 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:07:42 -0600 Subject: [PATCH 115/151] use syntactic sugar --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 958b110e8..91accd42a 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -81,8 +81,8 @@ def define_render_template_for # Just use default method name if no conditional branches or if there is a single # conditional branch that just calls the default method_name - if branches.length == 1 - branches[0].last + if branches.one? + branches.last.last else out = branches.each_with_object(+"") do |(conditional, method_body), memo| memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" From 1569d757d186ae6f6af91fbe20d29de07fef8e3e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:09:46 -0600 Subject: [PATCH 116/151] naming, method privacy --- lib/view_component/compiler.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 91accd42a..beb74d332 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -59,7 +59,7 @@ def renders_template_for_variant?(variant) def define_render_template_for templates.each { _1.compile_to_component(@redefinition_lock) } - body = + method_body = if template = templates.find { _1.inline? } template.safe_method_name else @@ -84,10 +84,9 @@ def define_render_template_for if branches.one? branches.last.last else - out = branches.each_with_object(+"") do |(conditional, method_body), memo| - memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n #{method_body}\n" + out = branches.each_with_object(+"") do |(conditional, branch_body), memo| + memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n #{branch_body}\n" end - out << "else\n #{templates.find { _1.variant.nil? && _1.html? }.safe_method_name}\nend" end end @@ -96,7 +95,7 @@ def define_render_template_for component.silence_redefinition_of_method(:render_template_for) component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) - #{body} + #{method_body} end RUBY end @@ -227,7 +226,7 @@ def gather_templates end class Template - attr_reader :variant, :type, :call_method_name + attr_reader :variant, :type def initialize( component:, @@ -304,6 +303,8 @@ def defined_on_self? private + attr_reader :call_method_name + def source if @source_originally_nil # Load file each time we look up #source in case the file has been modified From 5407eb0419e2be7050eec9a045c9552016386f31 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:12:15 -0600 Subject: [PATCH 117/151] use default values --- lib/view_component/compiler.rb | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index beb74d332..59f086982 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -177,7 +177,6 @@ def gather_templates type: :file, path: path, lineno: 0, - source: nil, extension: pieces.last, this_format: pieces[1..-2].join(".").split("+").first&.to_sym, variant: pieces[1..-2].join(".").split("+").second&.to_sym @@ -197,10 +196,6 @@ def gather_templates templates << Template.new( component: component, type: :inline_call, - path: nil, - lineno: nil, - source: nil, - extension: nil, this_format: :html, variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, method_name: method_name, @@ -216,8 +211,6 @@ def gather_templates lineno: component.inline_template.lineno, source: component.inline_template.source.dup, extension: component.inline_template.language, - this_format: nil, - variant: nil ) end @@ -230,13 +223,13 @@ class Template def initialize( component:, - path:, - source:, - extension:, - this_format:, - lineno:, - variant:, type:, + this_format: nil, + variant: nil, + lineno: nil, + path: nil, + extension: nil, + source: nil, method_name: nil, defined_on_self: true ) From 87431a9d792c5c54dce93d97b627fbaa56aa8ade Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:14:36 -0600 Subject: [PATCH 118/151] inline inline_calls --- lib/view_component/compiler.rb | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 59f086982..22c9c33e1 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -187,21 +187,20 @@ def gather_templates out end - inline_calls = - ( - component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - component.included_modules - ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }.uniq - - inline_calls.each do |method_name| - templates << Template.new( - component: component, - type: :inline_call, - this_format: :html, - variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, - method_name: method_name, - defined_on_self: component.instance_methods(false).include?(method_name) - ) - end + ( + component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - component.included_modules + ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }. + uniq. + each do |method_name| + templates << Template.new( + component: component, + type: :inline_call, + this_format: :html, + variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, + method_name: method_name, + defined_on_self: component.instance_methods(false).include?(method_name) + ) + end if component.inline_template.present? templates << Template.new( From e3bfb7577e4393a22bae474179b35afa62e46322 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:15:49 -0600 Subject: [PATCH 119/151] standardrb --- lib/view_component/compiler.rb | 37 +++++++++++++++++----------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 22c9c33e1..19f38986a 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -60,7 +60,7 @@ def define_render_template_for templates.each { _1.compile_to_component(@redefinition_lock) } method_body = - if template = templates.find { _1.inline? } + if (template = templates.find { _1.inline? }) template.safe_method_name else branches = [] @@ -109,11 +109,11 @@ def gather_template_errors(raise_errors) # 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.select { !_1.inline_call? }. - map { |template| [template.variant, template.format] }. - tally. - select { |_, count| count > 1 }. - each do |tally| + templates.select { !_1.inline_call? } + .map { |template| [template.variant, template.format] } + .tally + .select { |_, count| count > 1 } + .each do |tally| variant, this_format = tally[0] variant_string = " for variant `#{variant}`" if variant.present? @@ -121,10 +121,9 @@ def gather_template_errors(raise_errors) errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " end - if ( - templates.any? { _1.variant.nil? && !_1.inline_call? } && - templates.any? { _1.variant.nil? && _1.inline_call? && _1.defined_on_self? } - ) + if templates.any? { _1.variant.nil? && !_1.inline_call? } && + templates.any? { _1.variant.nil? && _1.inline_call? && _1.defined_on_self? } + errors << "Template file and inline render method found for #{component}. " \ "There can only be a template file or inline render method per component." @@ -148,11 +147,11 @@ def gather_template_errors(raise_errors) templates.select { _1.variant.present? }.map { [_1.variant, _1.normalized_variant_name] }.uniq { _1.first } colliding_normalized_variants = - variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys. - map do |normalized_variant_name| - variant_pairs. - select { |variant_pair| variant_pair.last == normalized_variant_name }. - map { |variant_pair| variant_pair.first } + variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys + .map do |normalized_variant_name| + variant_pairs + .select { |variant_pair| variant_pair.last == normalized_variant_name } + .map { |variant_pair| variant_pair.first } end colliding_normalized_variants.each do |variants| @@ -189,9 +188,9 @@ def gather_templates ( component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - component.included_modules - ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }. - uniq. - each do |method_name| + ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) } + .uniq + .each do |method_name| templates << Template.new( component: component, type: :inline_call, @@ -209,7 +208,7 @@ def gather_templates path: component.inline_template.path, lineno: component.inline_template.lineno, source: component.inline_template.source.dup, - extension: component.inline_template.language, + extension: component.inline_template.language ) end From 03415c3aef1685368af6a34b8bc6a507f6db1789 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:16:33 -0600 Subject: [PATCH 120/151] remove out-of-date comment --- lib/view_component/compiler.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 19f38986a..a63988fc4 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -79,8 +79,6 @@ def define_render_template_for branches << [conditional, template.safe_method_name] end - # Just use default method name if no conditional branches or if there is a single - # conditional branch that just calls the default method_name if branches.one? branches.last.last else From f89456bb483ac85107c16f217a030231301f41b3 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:19:58 -0600 Subject: [PATCH 121/151] shorten lines --- lib/view_component/compiler.rb | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index a63988fc4..666573046 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -313,23 +313,13 @@ def compiled_source if handler.method(:call).parameters.length > 1 handler.call( - OpenStruct.new( - format: @this_format, - identifier: @path, - short_identifier: short_identifier, - type: type - ), + OpenStruct.new(format: @this_format, identifier: @path, short_identifier: short_identifier, type: type), this_source ) # :nocov: + # TODO: Remove in v4 else - handler.call( - OpenStruct.new( - source: this_source, - identifier: @path, - type: type - ) - ) + handler.call(OpenStruct.new(source: this_source, identifier: @path, type: type)) end # :nocov: end From e9151b79823252915e9621a32a0a4b7fc82f1bab Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:23:05 -0600 Subject: [PATCH 122/151] use .first --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 666573046..b31f9c856 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -112,7 +112,7 @@ def gather_template_errors(raise_errors) .tally .select { |_, count| count > 1 } .each do |tally| - variant, this_format = tally[0] + variant, this_format = tally.first variant_string = " for variant `#{variant}`" if variant.present? From e53d8a3d1715a353b4a890a2bc5e412aec147d4b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:28:40 -0600 Subject: [PATCH 123/151] remove attr_reader --- lib/view_component/compiler.rb | 74 +++++++++++++++++----------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index b31f9c856..a199f9772 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -20,31 +20,31 @@ def initialize(component) end def compiled? - CompileCache.compiled?(component) + CompileCache.compiled?(@component) end def compile(raise_errors: false, force: false) - return if (compiled? && !force) || component == ViewComponent::Base + return if (compiled? && !force) || @component == ViewComponent::Base gather_templates - if self.class.mode == DEVELOPMENT_MODE && templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } - component.superclass.compile(raise_errors: raise_errors) + if self.class.mode == DEVELOPMENT_MODE && @templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } + @component.superclass.compile(raise_errors: raise_errors) end return if gather_template_errors(raise_errors).any? if raise_errors - component.validate_initialization_parameters! - component.validate_collection_parameter! + @component.validate_initialization_parameters! + @component.validate_collection_parameter! end define_render_template_for - component.register_default_slots - component.build_i18n_backend + @component.register_default_slots + @component.build_i18n_backend - CompileCache.register(component) + CompileCache.register(@component) end # TODO this should probably take format into account @@ -54,18 +54,18 @@ def renders_template_for_variant?(variant) private - attr_reader :component, :templates + attr_reader :templates def define_render_template_for - templates.each { _1.compile_to_component(@redefinition_lock) } + @templates.each { _1.compile_to_component(@redefinition_lock) } method_body = - if (template = templates.find { _1.inline? }) + if (template = @templates.find { _1.inline? }) template.safe_method_name else branches = [] - templates.each do |template| + @templates.each do |template| conditional = if template.inline_call? "variant&.to_sym == #{template.variant.inspect}" @@ -90,8 +90,8 @@ def define_render_template_for end @redefinition_lock.synchronize do - component.silence_redefinition_of_method(:render_template_for) - component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 + @component.silence_redefinition_of_method(:render_template_for) + @component.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def render_template_for(variant = nil, format = nil) #{method_body} end @@ -102,12 +102,12 @@ def render_template_for(variant = nil, format = nil) def gather_template_errors(raise_errors) 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.select { !_1.inline_call? } + @templates.select { !_1.inline_call? } .map { |template| [template.variant, template.format] } .tally .select { |_, count| count > 1 } @@ -116,20 +116,20 @@ def gather_template_errors(raise_errors) variant_string = " for variant `#{variant}`" if variant.present? - errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component}. " + errors << "More than one #{this_format.upcase} template found#{variant_string} for #{@component}. " end - if templates.any? { _1.variant.nil? && !_1.inline_call? } && - templates.any? { _1.variant.nil? && _1.inline_call? && _1.defined_on_self? } + if @templates.any? { _1.variant.nil? && !_1.inline_call? } && + @templates.any? { _1.variant.nil? && _1.inline_call? && _1.defined_on_self? } errors << - "Template file and inline render method found for #{component}. " \ + "Template file and inline render method found for #{@component}. " \ "There can only be a template file or inline render method per component." end duplicate_template_file_and_inline_call_variants = - templates.select { !_1.inline_call? }.map(&:variant) & - templates.select { _1.inline_call? && _1.defined_on_self? }.map(&:variant) + @templates.select { !_1.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 @@ -138,11 +138,11 @@ def gather_template_errors(raise_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." + "in #{@component}. There can only be a template file or inline render method per variant." end variant_pairs = - templates.select { _1.variant.present? }.map { [_1.variant, _1.normalized_variant_name] }.uniq { _1.first } + @templates.select { _1.variant.present? }.map { [_1.variant, _1.normalized_variant_name] }.uniq { _1.first } colliding_normalized_variants = variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys @@ -153,7 +153,7 @@ def gather_template_errors(raise_errors) end colliding_normalized_variants.each do |variants| - errors << "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} found in #{component}." + errors << "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} found in #{@component}." end raise TemplateError.new(errors) if errors.any? && raise_errors @@ -164,13 +164,13 @@ def gather_template_errors(raise_errors) def gather_templates @templates ||= begin - templates = component.sidecar_files( + templates = @component.sidecar_files( ActionView::Template.template_handler_extensions ).map do |path| pieces = File.basename(path).split(".") out = Template.new( - component: component, + component: @component, type: :file, path: path, lineno: 0, @@ -185,28 +185,28 @@ def gather_templates end ( - component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - component.included_modules + @component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - @component.included_modules ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) } .uniq .each do |method_name| templates << Template.new( - component: component, + component: @component, type: :inline_call, this_format: :html, variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, method_name: method_name, - defined_on_self: component.instance_methods(false).include?(method_name) + defined_on_self: @component.instance_methods(false).include?(method_name) ) end - if component.inline_template.present? + if @component.inline_template.present? templates << Template.new( - component: component, + component: @component, type: :inline, - path: component.inline_template.path, - lineno: component.inline_template.lineno, - source: component.inline_template.source.dup, - extension: component.inline_template.language + path: @component.inline_template.path, + lineno: @component.inline_template.lineno, + source: @component.inline_template.source.dup, + extension: @component.inline_template.language ) end From 1e7170757099a2cc60df490457fcb7f4f87ee237 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:35:53 -0600 Subject: [PATCH 124/151] simplify compiler mode to be boolean --- lib/view_component/compiler.rb | 13 +++++-------- lib/view_component/engine.rb | 6 +----- test/sandbox/test/integration_test.rb | 6 +++--- test/sandbox/test/rendering_test.rb | 8 ++++---- test/test_helper.rb | 8 ++++---- 5 files changed, 17 insertions(+), 24 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index a199f9772..fda72ec96 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -4,14 +4,11 @@ module ViewComponent class Compiler - # Compiler mode. Can be either: - # * development (a blocking mode which ensures thread safety when redefining the `call` method for components, + # Compiler production mode. Can be either: + # * false (a blocking mode which ensures thread safety when redefining the `call` method for components, # default in Rails development and test mode) - # * production (a non-blocking mode, default in Rails production mode) - DEVELOPMENT_MODE = :development - PRODUCTION_MODE = :production - - class_attribute :mode, default: PRODUCTION_MODE + # * true(a non-blocking mode, default in Rails production mode) + class_attribute :production_mode, default: true def initialize(component) @component = component @@ -28,7 +25,7 @@ def compile(raise_errors: false, force: false) gather_templates - if self.class.mode == DEVELOPMENT_MODE && @templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } + if !self.class.production_mode && @templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } @component.superclass.compile(raise_errors: raise_errors) end diff --git a/lib/view_component/engine.rb b/lib/view_component/engine.rb index 2e822f767..155aa92c5 100644 --- a/lib/view_component/engine.rb +++ b/lib/view_component/engine.rb @@ -136,11 +136,7 @@ def serve_static_preview_assets?(app_config) end initializer "compiler mode" do |_app| - ViewComponent::Compiler.mode = if Rails.env.development? || Rails.env.test? - ViewComponent::Compiler::DEVELOPMENT_MODE - else - ViewComponent::Compiler::PRODUCTION_MODE - end + ViewComponent::Compiler.production_mode = !(Rails.env.development? || Rails.env.test?) end config.after_initialize do |app| diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index 21139169d..582dc0333 100644 --- a/test/sandbox/test/integration_test.rb +++ b/test/sandbox/test/integration_test.rb @@ -670,7 +670,7 @@ def test_sets_the_compiler_mode_in_production_mode Rails.env = "production".inquiry ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run - assert_equal ViewComponent::Compiler::PRODUCTION_MODE, ViewComponent::Compiler.mode + assert_equal true, ViewComponent::Compiler.production_mode ensure Rails.env = old_env ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run @@ -680,12 +680,12 @@ def test_sets_the_compiler_mode_in_production_mode def test_sets_the_compiler_mode_in_development_mode Rails.env.stub :development?, true do ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run - assert_equal ViewComponent::Compiler::DEVELOPMENT_MODE, ViewComponent::Compiler.mode + assert_equal false, ViewComponent::Compiler.production_mode end Rails.env.stub :test?, true do ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run - assert_equal ViewComponent::Compiler::DEVELOPMENT_MODE, ViewComponent::Compiler.mode + assert_equal false, ViewComponent::Compiler.production_mode end end diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index acac88fca..28d5c2014 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -915,7 +915,7 @@ def test_output_preamble_and_postamble end def test_compilation_in_development_mode - with_compiler_mode(ViewComponent::Compiler::DEVELOPMENT_MODE) do + with_compiler_production_mode(false) do with_new_cache do render_inline(MyComponent.new) assert_selector("div", text: "hello,world!") @@ -924,7 +924,7 @@ def test_compilation_in_development_mode end def test_compilation_in_production_mode - with_compiler_mode(ViewComponent::Compiler::PRODUCTION_MODE) do + with_compiler_production_mode(true) do with_new_cache do render_inline(MyComponent.new) assert_selector("div", text: "hello,world!") @@ -948,7 +948,7 @@ def test_multithread_render end def test_concurrency_deadlock_cache - with_compiler_mode(ViewComponent::Compiler::DEVELOPMENT_MODE) do + with_compiler_production_mode(false) do with_new_cache do render_inline(ContentEvalComponent.new) do ViewComponent::CompileCache.invalidate! @@ -1073,7 +1073,7 @@ def test_renders_nested_collection end def test_concurrency_deadlock - with_compiler_mode(ViewComponent::Compiler::DEVELOPMENT_MODE) do + with_compiler_production_mode(false) do with_new_cache do mutex = Mutex.new diff --git a/test/test_helper.rb b/test/test_helper.rb index b4ba2b9be..a9975de16 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -172,12 +172,12 @@ def with_render_monkey_patch_config(enabled, &block) with_config_option(:render_monkey_patch_enabled, enabled, &block) end -def with_compiler_mode(mode) - previous_mode = ViewComponent::Compiler.mode - ViewComponent::Compiler.mode = mode +def with_compiler_production_mode(mode) + previous_mode = ViewComponent::Compiler.production_mode + ViewComponent::Compiler.production_mode = mode yield ensure - ViewComponent::Compiler.mode = previous_mode + ViewComponent::Compiler.production_mode = previous_mode end def capture_warnings(&block) From b2f23e87e4989d95debac75e06eecb1611b0fe28 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:38:45 -0600 Subject: [PATCH 125/151] remove attr_reader --- lib/view_component/compiler.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index fda72ec96..553df76d8 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -244,11 +244,11 @@ def initialize( def compile_to_component(redefinition_lock) if !inline_call? redefinition_lock.synchronize do - @component.silence_redefinition_of_method(call_method_name) + @component.silence_redefinition_of_method(@call_method_name) # rubocop:disable Style/EvalWithLocation @component.class_eval <<-RUBY, @path, @lineno - def #{call_method_name} + def #{@call_method_name} #{compiled_source} end RUBY @@ -256,7 +256,7 @@ def #{call_method_name} end end - @component.define_method(safe_method_name, @component.instance_method(call_method_name)) + @component.define_method(safe_method_name, @component.instance_method(@call_method_name)) end def inline_call? @@ -276,7 +276,7 @@ def format end def safe_method_name - "_#{call_method_name}_#{@component.name.underscore.gsub("/", "__")}" + "_#{@call_method_name}_#{@component.name.underscore.gsub("/", "__")}" end def normalized_variant_name @@ -289,8 +289,6 @@ def defined_on_self? private - attr_reader :call_method_name - def source if @source_originally_nil # Load file each time we look up #source in case the file has been modified From f403700177315d514855af8f88b204d2e51b1a7b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 14:02:49 -0600 Subject: [PATCH 126/151] extract constant for default format --- lib/view_component/base.rb | 1 + lib/view_component/compiler.rb | 12 ++++++------ lib/view_component/test_helpers.rb | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 3c0513604..b822e292e 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -35,6 +35,7 @@ def config include ViewComponent::WithContentHelper RESERVED_PARAMETER = :content + DEFAULT_FORMAT = :html # For CSRF authenticity tokens in forms delegate :form_authenticity_token, :protect_against_forgery?, :config, to: :helpers diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 553df76d8..402512957 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -68,7 +68,7 @@ def define_render_template_for "variant&.to_sym == #{template.variant.inspect}" else [ - template.html? ? "(format == :html || format.nil?)" : "format == #{template.format.inspect}", + template.default_format? ? "(format == #{ViewComponent::Base::DEFAULT_FORMAT.inspect} || format.nil?)" : "format == #{template.format.inspect}", template.variant.nil? ? "variant.nil?" : "variant&.to_sym == #{template.variant.inspect}" ].join(" && ") end @@ -82,7 +82,7 @@ def define_render_template_for out = branches.each_with_object(+"") do |(conditional, branch_body), memo| memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n #{branch_body}\n" end - out << "else\n #{templates.find { _1.variant.nil? && _1.html? }.safe_method_name}\nend" + out << "else\n #{templates.find { _1.variant.nil? && _1.default_format? }.safe_method_name}\nend" end end @@ -189,7 +189,7 @@ def gather_templates templates << Template.new( component: @component, type: :inline_call, - this_format: :html, + this_format: ViewComponent::Base::DEFAULT_FORMAT, variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, method_name: method_name, defined_on_self: @component.instance_methods(false).include?(method_name) @@ -236,7 +236,7 @@ def initialize( else out = +"call" out << "_#{normalized_variant_name}" if @variant.present? - out << "_#{@this_format}" if @this_format.present? && @this_format != :html + out << "_#{@this_format}" if @this_format.present? && @this_format != ViewComponent::Base::DEFAULT_FORMAT out end end @@ -267,8 +267,8 @@ def inline? @type == :inline end - def html? - @this_format == :html + def default_format? + @this_format == ViewComponent::Base::DEFAULT_FORMAT end def format diff --git a/lib/view_component/test_helpers.rb b/lib/view_component/test_helpers.rb index 097d2cc38..679cc1bb9 100644 --- a/lib/view_component/test_helpers.rb +++ b/lib/view_component/test_helpers.rb @@ -205,7 +205,7 @@ def with_format(format) # @param full_path [String] The path to set for the current request. # @param host [String] The host to set for the current request. # @param method [String] The request method to set for the current request. - def with_request_url(full_path, host: nil, method: nil, format: :html) + def with_request_url(full_path, host: nil, method: nil, format: ViewComponent::Base::DEFAULT_FORMAT) old_request_host = vc_test_request.host old_request_method = vc_test_request.request_method old_request_path_info = vc_test_request.path_info From a59419ed1d88e29d451af9fc752b930dfe5f6bc7 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 13:46:15 -0600 Subject: [PATCH 127/151] split conditional into two lines for readibilty --- lib/view_component/compiler.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 85427da0e..32fed2eea 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -21,7 +21,8 @@ def compiled? end def compile(raise_errors: false, force: false) - return if (compiled? && !force) || @component == ViewComponent::Base + return if (compiled? && !force) + return if @component == ViewComponent::Base gather_templates From 2b26f1b88c9e418c20f2f3b3d8e460dc4423fcef Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 13:55:53 -0600 Subject: [PATCH 128/151] rename compiler mode to development mode --- lib/view_component/compiler.rb | 10 +++++----- lib/view_component/engine.rb | 2 +- test/sandbox/test/integration_test.rb | 6 +++--- test/sandbox/test/rendering_test.rb | 8 ++++---- test/test_helper.rb | 8 ++++---- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 32fed2eea..da557c39e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -4,11 +4,11 @@ module ViewComponent class Compiler - # Compiler production mode. Can be either: - # * false (a blocking mode which ensures thread safety when redefining the `call` method for components, + # Compiler development mode. Can be either: + # * true (a blocking mode which ensures thread safety when redefining the `call` method for components, # default in Rails development and test mode) - # * true(a non-blocking mode, default in Rails production mode) - class_attribute :production_mode, default: true + # * false(a non-blocking mode, default in Rails production mode) + class_attribute :development_mode, default: false def initialize(component) @component = component @@ -26,7 +26,7 @@ def compile(raise_errors: false, force: false) gather_templates - if !self.class.production_mode && @templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } + if self.class.development_mode && @templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } @component.superclass.compile(raise_errors: raise_errors) end diff --git a/lib/view_component/engine.rb b/lib/view_component/engine.rb index 155aa92c5..9a52c661f 100644 --- a/lib/view_component/engine.rb +++ b/lib/view_component/engine.rb @@ -136,7 +136,7 @@ def serve_static_preview_assets?(app_config) end initializer "compiler mode" do |_app| - ViewComponent::Compiler.production_mode = !(Rails.env.development? || Rails.env.test?) + ViewComponent::Compiler.development_mode = (Rails.env.development? || Rails.env.test?) end config.after_initialize do |app| diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index 582dc0333..76e65cf4d 100644 --- a/test/sandbox/test/integration_test.rb +++ b/test/sandbox/test/integration_test.rb @@ -670,7 +670,7 @@ def test_sets_the_compiler_mode_in_production_mode Rails.env = "production".inquiry ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run - assert_equal true, ViewComponent::Compiler.production_mode + assert_equal false, ViewComponent::Compiler.development_mode ensure Rails.env = old_env ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run @@ -680,12 +680,12 @@ def test_sets_the_compiler_mode_in_production_mode def test_sets_the_compiler_mode_in_development_mode Rails.env.stub :development?, true do ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run - assert_equal false, ViewComponent::Compiler.production_mode + assert_equal true, ViewComponent::Compiler.development_mode end Rails.env.stub :test?, true do ViewComponent::Engine.initializers.find { |i| i.name == "compiler mode" }.run - assert_equal false, ViewComponent::Compiler.production_mode + assert_equal true, ViewComponent::Compiler.development_mode end end diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index 28d5c2014..6f8e66b51 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -915,7 +915,7 @@ def test_output_preamble_and_postamble end def test_compilation_in_development_mode - with_compiler_production_mode(false) do + with_compiler_development_mode(true) do with_new_cache do render_inline(MyComponent.new) assert_selector("div", text: "hello,world!") @@ -924,7 +924,7 @@ def test_compilation_in_development_mode end def test_compilation_in_production_mode - with_compiler_production_mode(true) do + with_compiler_development_mode(false) do with_new_cache do render_inline(MyComponent.new) assert_selector("div", text: "hello,world!") @@ -948,7 +948,7 @@ def test_multithread_render end def test_concurrency_deadlock_cache - with_compiler_production_mode(false) do + with_compiler_development_mode(true) do with_new_cache do render_inline(ContentEvalComponent.new) do ViewComponent::CompileCache.invalidate! @@ -1073,7 +1073,7 @@ def test_renders_nested_collection end def test_concurrency_deadlock - with_compiler_production_mode(false) do + with_compiler_development_mode(true) do with_new_cache do mutex = Mutex.new diff --git a/test/test_helper.rb b/test/test_helper.rb index a9975de16..1e6d449bb 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -172,12 +172,12 @@ def with_render_monkey_patch_config(enabled, &block) with_config_option(:render_monkey_patch_enabled, enabled, &block) end -def with_compiler_production_mode(mode) - previous_mode = ViewComponent::Compiler.production_mode - ViewComponent::Compiler.production_mode = mode +def with_compiler_development_mode(mode) + previous_mode = ViewComponent::Compiler.development_mode + ViewComponent::Compiler.development_mode = mode yield ensure - ViewComponent::Compiler.production_mode = previous_mode + ViewComponent::Compiler.development_mode = previous_mode end def capture_warnings(&block) From aeb51b04545e115f96b730b9c2f87ff1c10b5c5b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:00:13 -0600 Subject: [PATCH 129/151] use any? with method extracted to template --- lib/view_component/compiler.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index da557c39e..2dc2eda8c 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -26,7 +26,7 @@ def compile(raise_errors: false, force: false) gather_templates - if self.class.development_mode && @templates.none? { !(_1.inline_call? && !_1.defined_on_self?) } + if self.class.development_mode && @templates.any?(&:requires_compiled_superclass?) @component.superclass.compile(raise_errors: raise_errors) end @@ -259,6 +259,10 @@ def #{@call_method_name} @component.define_method(safe_method_name, @component.instance_method(@call_method_name)) end + def requires_compiled_superclass? + inline_call? && !defined_on_self? + end + def inline_call? @type == :inline_call end From 33af53cf68c9036c09e6037b3a413d213ce82fd6 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:07:14 -0600 Subject: [PATCH 130/151] avoid use of _1 --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 2dc2eda8c..bb25abe33 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -57,7 +57,7 @@ def define_render_template_for @templates.each { _1.compile_to_component(@redefinition_lock) } method_body = - if (template = @templates.find { _1.inline? }) + if (template = @templates.find(&:inline?)) template.safe_method_name else branches = [] From 1399874495b50594733b3a19b34a5e60b578fe80 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:08:35 -0600 Subject: [PATCH 131/151] avoid use of _1 --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index bb25abe33..c3bf61401 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -139,7 +139,7 @@ def gather_template_errors(raise_errors) end variant_pairs = - @templates.select { _1.variant.present? }.map { [_1.variant, _1.normalized_variant_name] }.uniq { _1.first } + @templates.select { _1.variant.present? }.map { [_1.variant, _1.normalized_variant_name] }.uniq(&:first) colliding_normalized_variants = variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys From 4819ff77eb30a289b2c53cbaacb910f22d180355 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:14:41 -0600 Subject: [PATCH 132/151] move Template to its own file --- lib/view_component/base.rb | 1 + lib/view_component/compiler.rb | 113 -------------------------------- lib/view_component/template.rb | 114 +++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 113 deletions(-) create mode 100644 lib/view_component/template.rb diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 96baa3980..0d5ed49bd 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -11,6 +11,7 @@ require "view_component/preview" require "view_component/slotable" require "view_component/slotable_default" +require "view_component/template" require "view_component/translatable" require "view_component/with_content_helper" require "view_component/use_helpers" diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index c3bf61401..955040248 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -210,118 +210,5 @@ def gather_templates templates end end - - class Template - attr_reader :variant, :this_format, :type - - def initialize( - component:, - type:, - this_format: nil, - variant: nil, - lineno: nil, - path: nil, - extension: nil, - source: nil, - method_name: nil, - defined_on_self: true - ) - @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = - component, path, source, extension, this_format, lineno, variant&.to_sym, type, defined_on_self - @source_originally_nil = @source.nil? - - @call_method_name = - if @method_name - @method_name - else - out = +"call" - out << "_#{normalized_variant_name}" if @variant.present? - out << "_#{@this_format}" if @this_format.present? && @this_format != ViewComponent::Base::DEFAULT_FORMAT - out - end - end - - def compile_to_component(redefinition_lock) - if !inline_call? - redefinition_lock.synchronize do - @component.silence_redefinition_of_method(@call_method_name) - - # rubocop:disable Style/EvalWithLocation - @component.class_eval <<-RUBY, @path, @lineno - def #{@call_method_name} - #{compiled_source} - end - RUBY - # rubocop:enable Style/EvalWithLocation - end - end - - @component.define_method(safe_method_name, @component.instance_method(@call_method_name)) - end - - def requires_compiled_superclass? - inline_call? && !defined_on_self? - end - - def inline_call? - @type == :inline_call - end - - def inline? - @type == :inline - end - - def default_format? - @this_format == ViewComponent::Base::DEFAULT_FORMAT - end - - def format - @this_format - end - - def safe_method_name - "_#{@call_method_name}_#{@component.name.underscore.gsub("/", "__")}" - end - - def normalized_variant_name - @variant.to_s.gsub("-", "__").gsub(".", "___") - end - - def defined_on_self? - @defined_on_self - end - - private - - def source - if @source_originally_nil - # Load file each time we look up #source in case the file has been modified - File.read(@path) - else - @source - end - end - - def compiled_source - handler = ActionView::Template.handler_for_extension(@extension) - this_source = source - this_source.rstrip! if @component.strip_trailing_whitespace? - - short_identifier = defined?(Rails.root) ? @path.sub("#{Rails.root}/", "") : @path - type = ActionView::Template::Types[@this_format] - - if handler.method(:call).parameters.length > 1 - handler.call( - OpenStruct.new(format: @this_format, identifier: @path, short_identifier: short_identifier, type: type), - this_source - ) - # :nocov: - # TODO: Remove in v4 - else - handler.call(OpenStruct.new(source: this_source, identifier: @path, type: type)) - end - # :nocov: - end - end end end diff --git a/lib/view_component/template.rb b/lib/view_component/template.rb new file mode 100644 index 000000000..4be5930a8 --- /dev/null +++ b/lib/view_component/template.rb @@ -0,0 +1,114 @@ +module ViewComponent + class Template + attr_reader :variant, :this_format, :type + + def initialize( + component:, + type:, + this_format: nil, + variant: nil, + lineno: nil, + path: nil, + extension: nil, + source: nil, + method_name: nil, + defined_on_self: true + ) + @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = + component, path, source, extension, this_format, lineno, variant&.to_sym, type, defined_on_self + @source_originally_nil = @source.nil? + + @call_method_name = + if @method_name + @method_name + else + out = +"call" + out << "_#{normalized_variant_name}" if @variant.present? + out << "_#{@this_format}" if @this_format.present? && @this_format != ViewComponent::Base::DEFAULT_FORMAT + out + end + end + + def compile_to_component(redefinition_lock) + if !inline_call? + redefinition_lock.synchronize do + @component.silence_redefinition_of_method(@call_method_name) + + # rubocop:disable Style/EvalWithLocation + @component.class_eval <<-RUBY, @path, @lineno + def #{@call_method_name} + #{compiled_source} + end + RUBY + # rubocop:enable Style/EvalWithLocation + end + end + + @component.define_method(safe_method_name, @component.instance_method(@call_method_name)) + end + + def requires_compiled_superclass? + inline_call? && !defined_on_self? + end + + def inline_call? + @type == :inline_call + end + + def inline? + @type == :inline + end + + def default_format? + @this_format == ViewComponent::Base::DEFAULT_FORMAT + end + + def format + @this_format + end + + def safe_method_name + "_#{@call_method_name}_#{@component.name.underscore.gsub("/", "__")}" + end + + def normalized_variant_name + @variant.to_s.gsub("-", "__").gsub(".", "___") + end + + def defined_on_self? + @defined_on_self + end + + private + + def source + if @source_originally_nil + # Load file each time we look up #source in case the file has been modified + File.read(@path) + else + @source + end + end + + def compiled_source + handler = ActionView::Template.handler_for_extension(@extension) + this_source = source + this_source.rstrip! if @component.strip_trailing_whitespace? + + short_identifier = defined?(Rails.root) ? @path.sub("#{Rails.root}/", "") : @path + type = ActionView::Template::Types[@this_format] + + if handler.method(:call).parameters.length > 1 + handler.call( + OpenStruct.new(format: @this_format, identifier: @path, short_identifier: short_identifier, type: type), + this_source + ) + # :nocov: + # TODO: Remove in v4 + else + handler.call(OpenStruct.new(source: this_source, identifier: @path, type: type)) + end + # :nocov: + end + end +end \ No newline at end of file From ccac3743aba929954bc767ab2b9b341f32800861 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:26:43 -0600 Subject: [PATCH 133/151] extract simplest case to conditional --- lib/view_component/compiler.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 955040248..ec6692ecc 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -57,7 +57,9 @@ def define_render_template_for @templates.each { _1.compile_to_component(@redefinition_lock) } method_body = - if (template = @templates.find(&:inline?)) + if @templates.one? + @templates.first.safe_method_name + elsif (template = @templates.find(&:inline?)) template.safe_method_name else branches = [] @@ -76,14 +78,10 @@ def define_render_template_for branches << [conditional, template.safe_method_name] end - if branches.one? - branches.last.last - else - out = branches.each_with_object(+"") do |(conditional, branch_body), memo| - memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n #{branch_body}\n" - end - out << "else\n #{templates.find { _1.variant.nil? && _1.default_format? }.safe_method_name}\nend" + out = branches.each_with_object(+"") do |(conditional, branch_body), memo| + memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n #{branch_body}\n" end + out << "else\n #{templates.find { _1.variant.nil? && _1.default_format? }.safe_method_name}\nend" end @redefinition_lock.synchronize do From 45d351ab92c79c69cd272511951ddac803028c13 Mon Sep 17 00:00:00 2001 From: GitHub Actions Bot <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 20:27:40 +0000 Subject: [PATCH 134/151] Fix final line endings --- lib/view_component/template.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/template.rb b/lib/view_component/template.rb index 4be5930a8..639030570 100644 --- a/lib/view_component/template.rb +++ b/lib/view_component/template.rb @@ -111,4 +111,4 @@ def compiled_source # :nocov: end end -end \ No newline at end of file +end From 97dc2065f7ced94f9bd5e76e7221db480f36c265 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:32:17 -0600 Subject: [PATCH 135/151] avoid block variable --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index ec6692ecc..658ff6d38 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -102,7 +102,7 @@ def gather_template_errors(raise_errors) # 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.select { !_1.inline_call? } + @templates.reject(&:inline_call?) .map { |template| [template.variant, template.format] } .tally .select { |_, count| count > 1 } From 34d11c87ccb2d12873f25ae2af7e5bbff426b299 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:33:34 -0600 Subject: [PATCH 136/151] avoid more block variables --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 658ff6d38..ae24beeaf 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -123,7 +123,7 @@ def gather_template_errors(raise_errors) end duplicate_template_file_and_inline_call_variants = - @templates.select { !_1.inline_call? }.map(&:variant) & + @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? @@ -137,7 +137,7 @@ def gather_template_errors(raise_errors) end variant_pairs = - @templates.select { _1.variant.present? }.map { [_1.variant, _1.normalized_variant_name] }.uniq(&:first) + @templates.select(&:variant).map { [_1.variant, _1.normalized_variant_name] }.uniq(&:first) colliding_normalized_variants = variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys From a3263bc54a180da72a7fdee21a023f74c66e9fb1 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:34:19 -0600 Subject: [PATCH 137/151] standardrb --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index ae24beeaf..c4c42cad1 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -21,7 +21,7 @@ def compiled? end def compile(raise_errors: false, force: false) - return if (compiled? && !force) + return if compiled? && !force return if @component == ViewComponent::Base gather_templates From e862541dc1c22f72b41f564662f5960e6f71a1f6 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 14:52:27 -0600 Subject: [PATCH 138/151] add comment --- lib/view_component/compiler.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index c4c42cad1..0dde5ba56 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -122,6 +122,9 @@ def gather_template_errors(raise_errors) "There can only be a template file or inline render method per component." 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) From a6aaed6fc73da45ccb38df9b5cb8180e48b285bb Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 15:04:02 -0600 Subject: [PATCH 139/151] memoize instance methods lookup --- lib/view_component/compiler.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 0dde5ba56..9c2f019b6 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -53,6 +53,10 @@ def renders_template_for?(variant, format) attr_reader :templates + def component_instance_methods_on_self + @component_instance_methods_on_self ||= @component.instance_methods(false) + end + def define_render_template_for @templates.each { _1.compile_to_component(@redefinition_lock) } @@ -193,7 +197,7 @@ def gather_templates this_format: ViewComponent::Base::DEFAULT_FORMAT, variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, method_name: method_name, - defined_on_self: @component.instance_methods(false).include?(method_name) + defined_on_self: component_instance_methods_on_self.include?(method_name) ) end From 51302149eb4aafe15fb60f4abee5324c1309fc2a Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 15:07:59 -0600 Subject: [PATCH 140/151] memoize instance method check --- lib/view_component/compiler.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 9c2f019b6..10836f326 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -53,10 +53,6 @@ def renders_template_for?(variant, format) attr_reader :templates - def component_instance_methods_on_self - @component_instance_methods_on_self ||= @component.instance_methods(false) - end - def define_render_template_for @templates.each { _1.compile_to_component(@redefinition_lock) } @@ -186,6 +182,8 @@ def gather_templates out end + component_instance_methods_on_self = @component.instance_methods(false) + ( @component.ancestors.take_while { |ancestor| ancestor != ViewComponent::Base } - @component.included_modules ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) } From e10d42cf8e869a52f023b45bc4b13496df773ccb Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 15:11:44 -0600 Subject: [PATCH 141/151] use single line for each argument --- lib/view_component/template.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/view_component/template.rb b/lib/view_component/template.rb index 639030570..fe00f3bd0 100644 --- a/lib/view_component/template.rb +++ b/lib/view_component/template.rb @@ -14,8 +14,17 @@ def initialize( method_name: nil, defined_on_self: true ) - @component, @path, @source, @extension, @this_format, @lineno, @variant, @type, @defined_on_self = - component, path, source, extension, this_format, lineno, variant&.to_sym, type, defined_on_self + @component = component + @type = type + @this_format = this_format + @variant = variant&.to_sym + @lineno = lineno + @path = path + @extension = extension + @source = source + @method_name = method_name + @defined_on_self = defined_on_self + @source_originally_nil = @source.nil? @call_method_name = From 4882b3a806d52fbc70c68a23c51369ba69447ac1 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 15:27:06 -0600 Subject: [PATCH 142/151] add TODO for HTML output safety check --- lib/view_component/compiler.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 10836f326..6de31f114 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -177,6 +177,8 @@ def gather_templates variant: pieces[1..-2].join(".").split("+").second&.to_sym ) + # TODO: We should consider inlining the HTML output safety logic into the compiled render_template_for + # instead of this indirect approach @rendered_templates << [out.variant, out.this_format] out From f2b42a6c6c1d08f5e92a81d4ab566e4b91d8d304 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 15:45:32 -0600 Subject: [PATCH 143/151] extract and comment format and variant derivation from path --- lib/view_component/compiler.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 6de31f114..04cd4c3e6 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -165,16 +165,21 @@ def gather_templates templates = @component.sidecar_files( ActionView::Template.template_handler_extensions ).map do |path| - pieces = File.basename(path).split(".") + # Extract format and variant from template filename + # foo.html.erb => :html, nil + # foo.html+phone.erb => :html, :phone + # variants_component.html+mini.watch.erb => :html, :'mini.watch' + this_format, variant = + File.basename(path).split(".")[1..-2].join(".").split("+").map(&:to_sym) out = Template.new( component: @component, type: :file, path: path, lineno: 0, - extension: pieces.last, - this_format: pieces[1..-2].join(".").split("+").first&.to_sym, - variant: pieces[1..-2].join(".").split("+").second&.to_sym + extension: File.extname(path)[1..-1], + this_format: this_format, + variant: variant ) # TODO: We should consider inlining the HTML output safety logic into the compiled render_template_for From 34ca3352fffeccfa2cf7b8a54323e7724c961573 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 15:46:40 -0600 Subject: [PATCH 144/151] simpler extension extraction --- lib/view_component/compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 04cd4c3e6..541844d07 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -177,7 +177,7 @@ def gather_templates type: :file, path: path, lineno: 0, - extension: File.extname(path)[1..-1], + extension: path.split('.').last, this_format: this_format, variant: variant ) From 570faebef407051fdfe3e5a23602dbdd87c8cdce Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 15:50:27 -0600 Subject: [PATCH 145/151] add explainer comments for format and variant extraction --- lib/view_component/compiler.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 541844d07..58fdad35d 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -166,11 +166,13 @@ def gather_templates ActionView::Template.template_handler_extensions ).map do |path| # Extract format and variant from template filename - # foo.html.erb => :html, nil - # foo.html+phone.erb => :html, :phone - # variants_component.html+mini.watch.erb => :html, :'mini.watch' this_format, variant = - File.basename(path).split(".")[1..-2].join(".").split("+").map(&:to_sym) + File + .basename(path) # "variants_component.html+mini.watch.erb" + .split(".")[1..-2] # ["html+mini", "watch"] + .join(".") # "html+mini.watch" + .split("+") # ["html", "mini.watch"] + .map(&:to_sym) # [:html, :"mini.watch"] out = Template.new( component: @component, From a2d2f848dee13d207a549207c5f29369b69b69c1 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 16:22:27 -0600 Subject: [PATCH 146/151] use Hash with Set value to find conflicting variants --- lib/view_component/compiler.rb | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 58fdad35d..4f85ca6f6 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -139,19 +139,13 @@ def gather_template_errors(raise_errors) "in #{@component}. There can only be a template file or inline render method per variant." end - variant_pairs = - @templates.select(&:variant).map { [_1.variant, _1.normalized_variant_name] }.uniq(&:first) - - colliding_normalized_variants = - variant_pairs.map(&:last).tally.select { |_, count| count > 1 }.keys - .map do |normalized_variant_name| - variant_pairs - .select { |variant_pair| variant_pair.last == normalized_variant_name } - .map { |variant_pair| variant_pair.first } - 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 - colliding_normalized_variants.each do |variants| - errors << "Colliding templates #{variants.sort.map { |v| "'#{v}'" }.to_sentence} found in #{@component}." + errors << "Colliding templates #{variant_names.sort.map { |v| "'#{v}'" }.to_sentence} found in #{@component}." end raise TemplateError.new(errors) if errors.any? && raise_errors From 201f6fa5ade491e4e85d12e8c68897537fbf5273 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 16:51:43 -0600 Subject: [PATCH 147/151] use single loop instead of two --- lib/view_component/compiler.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 4f85ca6f6..d379b6548 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -114,9 +114,14 @@ def gather_template_errors(raise_errors) errors << "More than one #{this_format.upcase} template found#{variant_string} for #{@component}. " end - if @templates.any? { _1.variant.nil? && !_1.inline_call? } && - @templates.any? { _1.variant.nil? && _1.inline_call? && _1.defined_on_self? } + default_template_types = @templates.reject(&:variant).each_with_object(Set.new) do |template, memo| + memo << :template_file if !template.inline_call? + memo << :inline_render if template.inline_call? && template.defined_on_self? + memo + 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." From 3d93a499bd1a9435b77d63723802da69b2d918d8 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 10 Sep 2024 16:59:06 -0600 Subject: [PATCH 148/151] standardrb --- lib/view_component/compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index d379b6548..1ef8a38d1 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -144,7 +144,7 @@ def gather_template_errors(raise_errors) "in #{@component}. There can only be a template file or inline render method per variant." end - @templates.select(&:variant).each_with_object(Hash.new { |h,k| h[k] = Set.new }) do |template, memo| + @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| @@ -178,7 +178,7 @@ def gather_templates type: :file, path: path, lineno: 0, - extension: path.split('.').last, + extension: path.split(".").last, this_format: this_format, variant: variant ) From 839fd2065aeee954dbe1eb8ed9441d1c681dc7d6 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 11 Sep 2024 14:23:58 -0600 Subject: [PATCH 149/151] indicate constant is in internal --- lib/view_component/base.rb | 2 +- lib/view_component/compiler.rb | 4 ++-- lib/view_component/template.rb | 4 ++-- lib/view_component/test_helpers.rb | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 0d5ed49bd..4422c2bd3 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -36,7 +36,7 @@ def config include ViewComponent::WithContentHelper RESERVED_PARAMETER = :content - DEFAULT_FORMAT = :html + VC_INTERNAL_DEFAULT_FORMAT = :html # For CSRF authenticity tokens in forms delegate :form_authenticity_token, :protect_against_forgery?, :config, to: :helpers diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 1ef8a38d1..02b99643f 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -70,7 +70,7 @@ def define_render_template_for "variant&.to_sym == #{template.variant.inspect}" else [ - template.default_format? ? "(format == #{ViewComponent::Base::DEFAULT_FORMAT.inspect} || format.nil?)" : "format == #{template.format.inspect}", + template.default_format? ? "(format == #{ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT.inspect} || format.nil?)" : "format == #{template.format.inspect}", template.variant.nil? ? "variant.nil?" : "variant&.to_sym == #{template.variant.inspect}" ].join(" && ") end @@ -200,7 +200,7 @@ def gather_templates templates << Template.new( component: @component, type: :inline_call, - this_format: ViewComponent::Base::DEFAULT_FORMAT, + this_format: ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT, variant: method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil, method_name: method_name, defined_on_self: component_instance_methods_on_self.include?(method_name) diff --git a/lib/view_component/template.rb b/lib/view_component/template.rb index fe00f3bd0..066948cf0 100644 --- a/lib/view_component/template.rb +++ b/lib/view_component/template.rb @@ -33,7 +33,7 @@ def initialize( else out = +"call" out << "_#{normalized_variant_name}" if @variant.present? - out << "_#{@this_format}" if @this_format.present? && @this_format != ViewComponent::Base::DEFAULT_FORMAT + out << "_#{@this_format}" if @this_format.present? && @this_format != ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT out end end @@ -69,7 +69,7 @@ def inline? end def default_format? - @this_format == ViewComponent::Base::DEFAULT_FORMAT + @this_format == ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT end def format diff --git a/lib/view_component/test_helpers.rb b/lib/view_component/test_helpers.rb index 679cc1bb9..0dfbe7a85 100644 --- a/lib/view_component/test_helpers.rb +++ b/lib/view_component/test_helpers.rb @@ -205,7 +205,7 @@ def with_format(format) # @param full_path [String] The path to set for the current request. # @param host [String] The host to set for the current request. # @param method [String] The request method to set for the current request. - def with_request_url(full_path, host: nil, method: nil, format: ViewComponent::Base::DEFAULT_FORMAT) + def with_request_url(full_path, host: nil, method: nil, format: ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT) old_request_host = vc_test_request.host old_request_method = vc_test_request.request_method old_request_path_info = vc_test_request.path_info From 0c93a63c488208591f9f8b31ee1eae40f750add5 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 11 Sep 2024 14:27:01 -0600 Subject: [PATCH 150/151] move redefinition lock back to compiler --- lib/view_component/compiler.rb | 6 +++++- lib/view_component/template.rb | 20 +++++++++----------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 02b99643f..616b79840 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -54,7 +54,11 @@ def renders_template_for?(variant, format) attr_reader :templates def define_render_template_for - @templates.each { _1.compile_to_component(@redefinition_lock) } + @templates.each do |template| + @redefinition_lock.synchronize do + template.compile_to_component + end + end method_body = if @templates.one? diff --git a/lib/view_component/template.rb b/lib/view_component/template.rb index 066948cf0..34f164bf4 100644 --- a/lib/view_component/template.rb +++ b/lib/view_component/template.rb @@ -38,19 +38,17 @@ def initialize( end end - def compile_to_component(redefinition_lock) + def compile_to_component if !inline_call? - redefinition_lock.synchronize do - @component.silence_redefinition_of_method(@call_method_name) - - # rubocop:disable Style/EvalWithLocation - @component.class_eval <<-RUBY, @path, @lineno - def #{@call_method_name} - #{compiled_source} - end - RUBY - # rubocop:enable Style/EvalWithLocation + @component.silence_redefinition_of_method(@call_method_name) + + # rubocop:disable Style/EvalWithLocation + @component.class_eval <<-RUBY, @path, @lineno + def #{@call_method_name} + #{compiled_source} end + RUBY + # rubocop:enable Style/EvalWithLocation end @component.define_method(safe_method_name, @component.instance_method(@call_method_name)) From 75e1c1320ed811779f3c690372ff3285ef67ea58 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 11 Sep 2024 14:28:08 -0600 Subject: [PATCH 151/151] remove loop --- lib/view_component/compiler.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 616b79840..e29141a3e 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -118,7 +118,9 @@ def gather_template_errors(raise_errors) errors << "More than one #{this_format.upcase} template found#{variant_string} for #{@component}. " end - default_template_types = @templates.reject(&:variant).each_with_object(Set.new) do |template, memo| + default_template_types = @templates.each_with_object(Set.new) do |template, memo| + next if template.variant + memo << :template_file if !template.inline_call? memo << :inline_render if template.inline_call? && template.defined_on_self?