Improve template path detection for forms

[camertron]

What are you trying to accomplish?

This PR adjusts the logic for detecting template paths for Rails forms. Like ViewComponents, Rails forms allow templates that follow specific naming conventions to be located adjacent to the form's .rb file. These templates enable two pieces of functionality:

1. Caption templates allow rendering more complicated field captions, i.e. captions that require more than simple plain text. Caption templates follow the naming convention /_caption.html.erb. For example, a form at foo_form.rb containing a field named bar would render a caption template at foo_form/bar_caption.html.erb.
2. After content allows rendering a template after the form, usually for complicated description text that explains how the form should be used, how it's validated, general warnings, etc. These templates must be named after_content.html.erb. For example, a form at foo_form.rb would render an after content template at foo_form/after_content.html.erb.

When I originally added support for separate template files, I tried to use the new-ish Module.const_source_location method, which returns the absolute path to the file a particular constant was defined in. Doing so allows us to construct the template path for each form .rb file. Unfortunately, due to a bug in Ruby itself, this method returned false if the file was autoloaded, eg. by Zeitwerk, the Rails autoload system. The bug has since been fixed in newer versions of Ruby, but at the time was a blocker. To work around the problem, I introduced the Primer::Forms::Utils.const_source_location method that exploits the Zeitwerk requirement that files are named after the constants they define. In other words, FooForm is required to live in foo_form.rb somewhere on the Rails load path. The workaround therefore was able to loop over each path in the Rails load path looking for a file with the same name as the given constant. This approach seemed to work fine.

Before the advent of Zeitwerk, Rails used to manage autoloads via the ActiveSupport::Dependencies module, which still exists to this day. When adding support for template files, I noticed that ActiveSupport::Dependencies.autoload_paths seemed to contain all the right autoload paths, including the very important app/forms directory. Unfortunately, as I have now learned, it does not contain all the autoload paths that may have been added eg. in application.rb. Dotcom does in fact add autoload paths in application.rb, but these paths are added to a Zeitwerk-specific data structure, i.e. not ActiveSupport::Dependencies.autoload_paths. Instead, Primer forms should iterate over Rails.autoloaders.main.dirs. I modified the code to do so, falling back to ActiveSupport::Dependencies.autoload_paths if Zeitwerk is not enabled (after all, PVC is used by apps outside of the GitHub monolith).

I can hear you asking, "Why do this at all? Can't we just use Module.const_source_location now that Ruby bug has been fixed and released?" Wow, I can see you were paying attention. You're absolutely right! That's where the second change in this PR comes from - using Module.const_source_location and falling back to Primer::Forms::Utils.const_source_location when necessary. Doing so should work for a sufficiently large matrix of Ruby and Rails versions.

Finally, this PR also contains a bit of a refactoring to prevent calling Module.const_source_location before the constant has been defined and the old autoload replaced. Calling Module.const_source_location too early (i.e. in included callbacks) will return the path to an internal Zeitwerk file (cref.rb), which I assume is the file that defines the autoload. The autoload has not been cleaned up by the time included is called, so we have to do it later. This turns out to be a cleaner approach anyway, as it does not require us to set template_root_path on form classes. Nice.

Integration

No changes necessary in production, everything in this PR should be backwards-compatible.

List the issues that this change affects.

Fixes: https://github.com/github/primer/issues/4500

Risk Assessment

• Low risk the change is small, highly observable, and easily rolled back.
• Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
• High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

See above.

Accessibility

• No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Lookbook)
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

🦋 Changeset detected

Latest commit: b878ba2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[jonrohan]

I'll take your word for it 👍🏻