Improve template path detection for forms

.changeset/khaki-seals-battle.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Improve template path detection for forms

app/lib/primer/forms/acts_as_component.rb

@@ -50,8 +50,6 @@ def self.extended(base)
       TemplateGlob = Struct.new(:glob_pattern, :method_name, :on_compile_callback)
       TemplateParams = Struct.new(:source, :identifier, :type, :format, keyword_init: true)
 
-      attr_accessor :template_root_path
-
       def renders_templates(glob_pattern, method_name = nil, &block)
         template_globs << TemplateGlob.new(glob_pattern, method_name, block)
       end
@@ -70,6 +68,15 @@ def compile!
 
       private
 
+      def template_root_path
+        return @template_root_path if defined?(@template_root_path)
+
+        form_path = Utils.const_source_location(self.name)
+        @template_root_path = if form_path
+          File.join(File.dirname(form_path), self.name.demodulize.underscore)
+        end
+      end
+
       def template_globs
         @template_globs ||= []
       end

app/lib/primer/forms/base.rb

@@ -29,11 +29,6 @@ def new(builder, **options)
         end
 
         def inherited(base)
-          form_path = Utils.const_source_location(base.name)
-          return unless form_path
-
-          base.template_root_path = File.join(File.dirname(form_path), base.name.demodulize.underscore)
-
           base.renders_template "after_content.html.erb" do
             base.instance_variable_set(:@has_after_content, true)
           end

app/lib/primer/forms/base_component.rb

@@ -7,16 +7,18 @@ class BaseComponent
       include Primer::ClassNameHelper
       extend ActsAsComponent
 
-      def self.inherited(base)
-        base_path = Utils.const_source_location(base.name)
+      def self.compile!
+        base_path = Utils.const_source_location(self.name)
 
         unless base_path
           warn "Could not identify the template for #{base}"
           return
         end
 
         dir = File.dirname(base_path)
-        base.renders_template File.join(dir, "#{base.name.demodulize.underscore}.html.erb"), :render_template
+        renders_template File.join(dir, "#{self.name.demodulize.underscore}.html.erb"), :render_template
+
+        super
       end
 
       delegate :required?, :disabled?, :hidden?, to: :@input

app/lib/primer/forms/utils.rb

@@ -14,13 +14,28 @@ module Utils
       # for the file in the configured autoload paths. Doing so relies on Rails' autoloading
       # conventions, so it should work ok. Zeitwerk also has this information but lacks a
       # public API to map constants to source files.
+      #
+      # Now that the Ruby bug above has been fixed and released, this method should be used only
+      # as a fallback for older Rubies.
       def const_source_location(class_name)
         return nil unless class_name
 
+        if (location = Object.const_source_location(class_name)&.[](0))
+          return location
+        end
+
         # NOTE: underscore respects namespacing, i.e. will convert Foo::Bar to foo/bar.
         class_path = "#{class_name.underscore}.rb"
 
-        ActiveSupport::Dependencies.autoload_paths.each do |autoload_path|
+        # Prefer Zeitwerk-managed paths, falling back to ActiveSupport::Dependencies if Zeitwerk
+        # is disabled or not in use (i.e. the case for older Rails versions).
+        autoload_paths = if Rails.respond_to?(:autoloaders) && Rails.autoloaders.zeitwerk_enabled?
+          Rails.autoloaders.main.dirs
+        else
+          ActiveSupport::Dependencies.autoload_paths
+        end
+
+        autoload_paths.each do |autoload_path|
           absolute_path = File.join(autoload_path, class_path)
           return absolute_path if File.exist?(absolute_path)
         end

demo/config/application.rb

@@ -33,7 +33,6 @@ class Application < Rails::Application
     config.view_component.preview_controller = "PreviewController"
     config.view_component.preview_paths << Rails.root.join("..", "previews")
 
-    config.autoload_paths << Rails.root.join("..", "test", "forms")
     config.autoload_paths << Rails.root.join("..", "test", "test_helpers", "components")
 
     config.action_dispatch.default_headers.clear

test/lib/primer/forms/const_src.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+# Used in utils_test.rb
+module Primer
+  module Forms
+    class ConstSrc
+    end
+  end
+end

test/lib/primer/forms/utils_test.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require "lib/test_helper"
+
+class Primer::Forms::UtilsTest < Minitest::Test
+  # This should pass under both Rubies that have the bug fix described in
+  # https://github.com/ruby/ruby/pull/5646 and Rubies that do not. Ruby versions 2.7 to 3.1
+  # (inclusive) are affected. See also https://bugs.ruby-lang.org/issues/18624.
+  def test_const_source_location
+    ensure_autoload!
+
+    actual_loc = Primer::Forms::Utils.const_source_location("Primer::Forms::ConstSrc")
+    expected_loc = File.join(File.dirname(__FILE__), "const_src.rb")
+
+    assert actual_loc == expected_loc
+  end
+
+  def test_const_source_location_falls_back_to_enumerating_load_path
+    # Normally this would be dangerous since test/lib isn't Zeitwerk-friendly, but because
+    # we're doing it after Zeitwerk setup, it will essentially have no effect.
+    Rails.autoloaders.main.push_dir(Rails.root.join("..", "test", "lib"))
+    ensure_autoload!
+
+    called = false
+
+    Object.stub(:const_source_location, [false, 0]) do
+      actual_loc = Primer::Forms::Utils.const_source_location("Primer::Forms::ConstSrc")
+      expected_loc = File.join(File.dirname(__FILE__), "const_src.rb")
+      assert actual_loc == expected_loc
+      called = true
+    end
+
+    assert called, "Object.const_source_location was not called"
+  end
+
+  private
+
+  def ensure_autoload!
+    # Load the constant so const_source_location will return the correct path instead of
+    # the path to the file that defines the autoload (which is this file, see below).
+    Primer::Forms.const_get(:ConstSrc)
+  rescue NameError
+    # We unfortunately can't add test/lib to the load path because it's not Zeitwerk-friendly,
+    # so we fake it by setting up the autoload to our test file manually.
+    Primer::Forms.autoload :ConstSrc, File.join(File.dirname(__FILE__), "const_src")
+    Primer::Forms.const_get(:ConstSrc)
+  end
+end