Use ActionView::TemplateDetails for handling format and variant (#2156)

* add .DS_Store to gitignore

* Add Template subclasses to improve compiler polymorphism

* Move template type-specific logic to constructors

* Inline source into templates that require it

* Flatten inline_call conditional in compile_to_component

* Remove defined_on_self param from non-inline-call templates

* Use ActionView logic for parsing template names

Removes support for variant names containing `.`.

* Delegate template format and variant to TemplateDetails

---------

Co-authored-by: Joel Hawksley <[email protected]>

Author: sfnelson

.gitignore

@@ -1,6 +1,7 @@
 *.gem
 *.rbc
 .ruby-version
+.DS_Store
 /.config
 /coverage/assets
 /coverage/index.html

docs/CHANGELOG.md

@@ -24,6 +24,10 @@ nav_order: 5
 
     *Joel Hawksley*
 
+* BREAKING: Remove support for variant names containing `.` to be consistent with Rails.
+
+    *Stephen Nelson*
+
 * Ensure HTML output safety wrapper is used for all inline templates.
 
     *Joel Hawksley*

docs/index.md

@@ -193,6 +193,7 @@ ViewComponent is built by over a hundred members of the community, including:
 <img src="https://avatars.githubusercontent.com/sammyhenningsson?s=64" alt="sammyhenningsson" width="32" />
 <img src="https://avatars.githubusercontent.com/sampart?s=64" alt="sampart" width="32" />
 <img src="https://avatars.githubusercontent.com/seanpdoyle?s=64" alt="seanpdoyle" width="32" />
+<img src="https://avatars.githubusercontent.com/sfnelson?s=64" alt="sfnelson" width="32" />
 <img src="https://avatars.githubusercontent.com/simonrand?s=64" alt="simonrand" width="32" />
 <img src="https://avatars.githubusercontent.com/skryukov?s=64" alt="skryukov" width="32" />
 <img src="https://avatars.githubusercontent.com/smashwilson?s=64" alt="smashwilson" width="32" />

lib/view_component/compiler.rb

@@ -169,27 +169,12 @@ def template_errors
     def gather_templates
       @templates ||=
         begin
+          path_parser = ActionView::Resolver::PathParser.new
           templates = @component.sidecar_files(
             ActionView::Template.template_handler_extensions
           ).map do |path|
-            # Extract format and variant from template filename
-            this_format, variant =
-              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,
-              type: :file,
-              path: path,
-              lineno: 0,
-              extension: path.split(".").last,
-              this_format: this_format.to_s.split(".").last&.to_sym, # strip locale from this_format, see #2113
-              variant: variant
-            )
+            details = path_parser.parse(path).details
+            out = Template::File.new(component: @component, path: path, details: details)
 
             out
           end
@@ -201,24 +186,17 @@ def gather_templates
           ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }
             .uniq
             .each do |method_name|
-              templates << Template.new(
+              templates << Template::InlineCall.new(
                 component: @component,
-                type: :inline_call,
-                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)
               )
             end
 
           if @component.inline_template.present?
-            templates << Template.new(
+            templates << Template::Inline.new(
               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
+              inline_template: @component.inline_template
             )
           end
 

lib/view_component/template.rb

@@ -5,56 +5,112 @@ class Template
     DataWithSource = Struct.new(:format, :identifier, :short_identifier, :type, keyword_init: true)
     DataNoSource = Struct.new(:source, :identifier, :type, keyword_init: true)
 
-    attr_reader :variant, :this_format, :type
+    attr_reader :details
+
+    delegate :format, :variant, to: :details
 
     def initialize(
       component:,
-      type:,
-      this_format: nil,
-      variant: nil,
+      details:,
       lineno: nil,
       path: nil,
-      extension: nil,
-      source: nil,
-      method_name: nil,
-      defined_on_self: true
+      method_name: nil
     )
       @component = component
-      @type = type
-      @this_format = this_format
-      @variant = variant&.to_sym
+      @details = details
       @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 =
         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::VC_INTERNAL_DEFAULT_FORMAT
+          out << "_#{normalized_variant_name}" if variant.present?
+          out << "_#{format}" if format.present? && format != ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT
           out
         end
     end
 
+    class File < Template
+      def initialize(component:, details:, path:)
+        super(
+          component: component,
+          details: details,
+          path: path,
+          lineno: 0
+        )
+      end
+
+      def type
+        :file
+      end
+
+      # Load file each time we look up #source in case the file has been modified
+      def source
+        ::File.read(@path)
+      end
+    end
+
+    class Inline < Template
+      attr_reader :source
+
+      def initialize(component:, inline_template:)
+        details = ActionView::TemplateDetails.new(nil, inline_template.language.to_sym, nil, nil)
+
+        super(
+          component: component,
+          details: details,
+          path: inline_template.path,
+          lineno: inline_template.lineno,
+        )
+
+        @source = inline_template.source.dup
+      end
+
+      def type
+        :inline
+      end
+    end
+
+    class InlineCall < Template
+      def initialize(component:, method_name:, defined_on_self:)
+        variant = method_name.to_s.include?("call_") ? method_name.to_s.sub("call_", "").to_sym : nil
+        details = ActionView::TemplateDetails.new(nil, nil, nil, variant)
+
+        super(
+          component: component,
+          details: details,
+          method_name: method_name
+        )
+
+        @defined_on_self = defined_on_self
+      end
+
+      def type
+        :inline_call
+      end
+
+      def compile_to_component
+        @component.define_method(safe_method_name, @component.instance_method(@call_method_name))
+      end
+
+      def defined_on_self?
+        @defined_on_self
+      end
+    end
+
     def compile_to_component
-      if !inline_call?
-        @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}
-          #{compiled_source}
-        end
-        RUBY
-        # rubocop:enable Style/EvalWithLocation
+      # rubocop:disable Style/EvalWithLocation
+      @component.class_eval <<-RUBY, @path, @lineno
+      def #{@call_method_name}
+        #{compiled_source}
       end
+      RUBY
+      # rubocop:enable Style/EvalWithLocation
 
       @component.define_method(safe_method_name, @component.instance_method(@call_method_name))
     end
@@ -72,55 +128,39 @@ def requires_compiled_superclass?
     end
 
     def inline_call?
-      @type == :inline_call
+      type == :inline_call
     end
 
     def inline?
-      @type == :inline
+      type == :inline
     end
 
     def default_format?
-      @this_format == ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT
-    end
-
-    def format
-      @this_format
+      format.nil? || format == ViewComponent::Base::VC_INTERNAL_DEFAULT_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
+      variant.to_s.gsub("-", "__")
     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)
+      handler = details.handler_class
       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]
+      format = self.format || ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT
+      type = ActionView::Template::Types[format]
 
       if handler.method(:call).parameters.length > 1
         handler.call(
-          DataWithSource.new(format: @this_format, identifier: @path, short_identifier: short_identifier, type: type),
+          DataWithSource.new(format: format, identifier: @path, short_identifier: short_identifier, type: type),
           this_source
         )
       # :nocov:

test/sandbox/app/components/variants_component.html+mini.watch.erb

@@ -198,14 +198,6 @@ def test_renders_component_with_variant_containing_a_dash
     end
   end
 
-  def test_renders_component_with_variant_containing_a_dot
-    with_variant :"mini.watch" do
-      render_inline(VariantsComponent.new)
-
-      assert_text("Mini Watch with dot")
-    end
-  end
-
   def test_renders_default_template_when_variant_template_is_not_present
     with_variant :variant_without_template do
       render_inline(VariantsComponent.new)