Add uncapped preload headers for style, script assets

[aduth]

🛠 Summary of changes

Updates asset helpers to bypass internal stylesheet_link_tag and javascript_include_tag logic for handling preload response headers, appending preload headers without caps. As of rails/rails#48405 (Rails 7.1), preload headers added by tag helpers are capped at 1kb, which is very limiting, especially given our approach of loading many, smaller assets targeted as sidecar assets for UI components.

While the caps have created some incentives to be more intentional with preloading (e.g. #10612), we've already addressed the lowest hanging fruit, and a number of critical scripts are currently not being preloaded (e.g. reCAPTCHA behaviors on the sign-in screen).

References:

• https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link

📜 Testing Plan

Verify that preload headers are included for all scripts and assets, including integrity or crossorigin directives as appropriate. Compare against https://secure.login.gov

curl -I http://localhost:3000

[zachmargolis]

maybe a micro-optimization that isn't worth it, but += will create and throw away all these intermediate strings? What if we appended as we went? And add .dup just incase the incoming Link string is frozen

Suggested change

	header = response.headers['Link'] || ''
	header += ',' if header.present?
	header += "<#{url}>;rel=preload;as=#{as}"
	header += ';crossorigin' if crossorigin
	header += ";integrity=#{integrity}" if integrity
	header = response.headers['Link']&.dup || String.new
	header << ',' if header.present?
	header << "<#{url}>;rel=preload;as=#{as}"
	header << ';crossorigin' if crossorigin
	header << ";integrity=#{integrity}" if integrity

[aduth]

Micro-optimizations welcome! One of the advantages I was hoping for here is that there might be a small performance win, even just by skipping some of the options we have no need for (and those which are evaluated in the default implementation). One of the other things I was going to explore is if it's possible to switch the JavaScript rendering from javascript_include_tag to tag.script similar to the edits made in stylesheet_helper.rb.

This idea seems good. I might even go a step further and say we don't need to duplicate the header value and could write directly to it? Since we're assigning it anyways at the end of this method.

[aduth]

I might even go a step further and say we don't need to duplicate the header value and could write directly to it? Since we're assigning it anyways at the end of this method.

Something like...

(maybe gains offset by cost of hash lookup?)

def self.append(headers:, as:, url:, crossorigin: false, integrity: nil)
  if headers['Link']
    headers['Link'] << ','
  else
    headers['Link'] = ''
  end
  headers['Link'] << "<#{url}>;rel=preload;as=#{as}"
  headers['Link'] << ';crossorigin' if crossorigin
  headers['Link'] << ";integrity=#{integrity}" if integrity
end

[zachmargolis]

The dup is just in case something else sets a frozen string

And with frozen_string_literal: true at the top of the file, I think we need String.new instead of ''

[zachmargolis]

Oh or we can use +'' instead of String.new for a mutable string literal (via StackOverflow)

[zachmargolis]

we should be able to directly assign this right? I'd go a step further an inline the entire thing, but that would make us have to wrap line 23 below

Suggested change

	crossorigin = true if local_crossorigin_sources?
	crossorigin = local_crossorigin_sources?

[aduth]

This branch was actually a few months old, and that change was one of the first things I tried editing when I resurrected it. But there's a significant difference between nil and false in how it's used with javascript_include_tag, which is the reason for the true if. I can't recall off the top of my head, but I'll make sure to add some test coverage for it when I write those.

[zachmargolis]

ah looks like the source code checks for "not nil" and "true"

maybe it would be easier if we updated local_crossorigin_sources? to match that expected format?

# Fake boolean to match expected values for `:crossorigin` option in `javascript_include_tag`
# @see ActionView::Helpers::AssetTagHelper#javascript_include_tag
# @return [true, nil]
def crossorigin
  true if Rails.env.development? && ENV['WEBPACK_PORT'].present?
end

[zachmargolis]

this means that preload_links_header: nil will add the preload header, that seems counterintuitive to me?

[aduth]

this means that preload_links_header: nil will add the preload header, that seems counterintuitive to me?

That would be the same behavior as the default javascript_include_tag behavior [1] [2], though technically it's configurable and respects the configured default.