From 698e9917a2ab3ba9fb5372c8179060c22e5ebd69 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 23 Aug 2024 13:19:31 -0600 Subject: [PATCH 01/22] 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 02/22] 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 03/22] 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 04/22] 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 05/22] 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 06/22] 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 07/22] 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 08/22] 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 09/22] 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 10/22] 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 11/22] 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 12/22] 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 13/22] 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 14/22] 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 15/22] 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 16/22] 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 17/22] 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 18/22] 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 19/22] 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 20/22] 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 69a377fe81f8b87d3ad938be720352df676397ae Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 10:48:17 -0600 Subject: [PATCH 21/22] compiler should check for variant/format render combinations --- lib/view_component/base.rb | 2 +- lib/view_component/compiler.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 9e7ad54bc..9996807e2 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -108,7 +108,7 @@ def render_in(view_context, &block) if render? # Avoid allocating new string when output_preamble and output_postamble are blank rendered_template = - if compiler.renders_template_for_variant?(@__vc_variant) + if compiler.renders_template_for?(@__vc_variant, request&.format&.to_sym) render_template_for(@__vc_variant, request&.format&.to_sym) else maybe_escape_html(render_template_for(@__vc_variant, request&.format&.to_sym)) do diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index fbc9b22a3..ceb0cea76 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -16,7 +16,7 @@ class Compiler def initialize(component_class) @component_class = component_class @redefinition_lock = Mutex.new - @variants_rendering_templates = Set.new + @rendered_templates = Set.new end def compiled? @@ -69,7 +69,7 @@ def render_template_for(variant = nil, format = nil) else templates.each do |template| method_name = call_method_name(template[:variant], template[:format]) - @variants_rendering_templates << template[:variant] + @rendered_templates << [template[:variant], template[:format]] redefinition_lock.synchronize do component_class.silence_redefinition_of_method(method_name) @@ -97,8 +97,8 @@ def #{method_name} CompileCache.register(component_class) end - def renders_template_for_variant?(variant) - @variants_rendering_templates.include?(variant) + def renders_template_for?(variant, format) + @rendered_templates.include?([variant, format]) end private From e0f057f66df809b3dbf29db6c9eb8a964dd3a1b3 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 6 Sep 2024 14:37:13 -0600 Subject: [PATCH 22/22] standardrb --- 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 ceb0cea76..e73dee54b 100644 --- a/lib/view_component/compiler.rb +++ b/lib/view_component/compiler.rb @@ -190,11 +190,11 @@ def template_errors errors << "Couldn't find a template file or inline render method for #{component_class}." end - templates. - map { |template| [template[:variant], template[:format]] }. - tally. - select { |_, count| count > 1 }. - each do |tally| + templates + .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?