Add uncapped preload headers for style, script assets (#11504)

* Add unlimited preload headers for style, script assets

changelog: Internal, Performance, Add preload headers for all style, script assets

* Update spec assertions for dropped whitespace

* Add specs

* Append with string mutation

See: https://github.com/18F/identity-idp/pull/11504/files#r1841273216

Co-authored-by: Zach Margolis <[email protected]>

* Use plain tag helper for generating script tag

We don't need any of this anymore:

- Asset pipeline lookup
- Preload headers handling
- Server push
- Multiple source concatenation
- Nonce handling

crossorigin as a boolean attribute is same as crossorigin=anonymous

* Add spec for preload_links_header attribute handling

* Evaluate crossorigin once

Previously evaluated per script, even though the result would always be the same

* Micro-optimize append

* Lowercase header key

* Use presence to toggle between true/nil

* Sync specs to use lowercase link key

---------

Co-authored-by: Zach Margolis <[email protected]>

app/helpers/script_helper.rb

@@ -14,14 +14,26 @@ def render_javascript_pack_once_tags(...)
       javascript_packs_tag_once(...)
       return if @scripts.blank?
       concat javascript_assets_tag
+      crossorigin = local_crossorigin_sources?.presence
       @scripts.each do |name, (url_params, attributes)|
         asset_sources.get_sources(name).each do |source|
-          concat javascript_include_tag(
-            UriService.add_params(source, url_params),
+          integrity = asset_sources.get_integrity(source)
+
+          if attributes[:preload_links_header] != false
+            AssetPreloadLinker.append(
+              headers: response.headers,
+              as: :script,
+              url: source,
+              crossorigin:,
+              integrity:,
+            )
+          end
+
+          concat tag.script(
+            src: UriService.add_params(source, url_params),
             **attributes,
-            crossorigin: local_crossorigin_sources? ? true : nil,
-            integrity: asset_sources.get_integrity(source),
-            nopush: false,
+            crossorigin:,
+            integrity:,
           )
         end
       end

app/helpers/stylesheet_helper.rb

@@ -13,7 +13,13 @@ def stylesheet_tag_once(*names)
   def render_stylesheet_once_tags(*names)
     stylesheet_tag_once(*names) if names.present?
     return if @stylesheets.blank?
-    safe_join(@stylesheets.map { |stylesheet| stylesheet_link_tag(stylesheet, nopush: false) })
+    safe_join(
+      @stylesheets.map do |stylesheet|
+        url = stylesheet_path(stylesheet)
+        AssetPreloadLinker.append(headers: response.headers, as: :style, url:)
+        tag.link(rel: :stylesheet, href: url)
+      end,
+    )
   end
 end
 # rubocop:enable Rails/HelperInstanceVariable

app/services/asset_preload_linker.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class AssetPreloadLinker
+  def self.append(headers:, as:, url:, crossorigin: false, integrity: nil)
+    header = +headers['link'].to_s
+    header << ',' if header != ''
+    header << "<#{url}>;rel=preload;as=#{as}"
+    header << ';crossorigin' if crossorigin
+    header << ";integrity=#{integrity}" if integrity
+    headers['link'] = header
+  end
+end

spec/helpers/script_helper_spec.rb

@@ -82,8 +82,8 @@
         render_javascript_pack_once_tags
 
         expect(response.headers['link']).to eq(
-          '</application.js>; rel=preload; as=script,' \
-            '</document-capture.js>; rel=preload; as=script',
+          '</application.js>;rel=preload;as=script,' \
+            '</document-capture.js>;rel=preload;as=script',
         )
         expect(response.headers['link']).to_not include('nopush')
       end
@@ -107,6 +107,18 @@
         end
       end
 
+      context 'with preload links header disabled' do
+        before do
+          javascript_packs_tag_once('application', preload_links_header: false)
+        end
+
+        it 'does not append preload header' do
+          render_javascript_pack_once_tags
+
+          expect(response.headers['link']).to eq('</document-capture.js>;rel=preload;as=script')
+        end
+      end
+
       context 'with attributes' do
         before do
           javascript_packs_tag_once('track-errors', defer: true)

spec/helpers/stylesheet_helper_spec.rb

@@ -34,7 +34,7 @@
       it 'adds preload header without nopush attribute' do
         render_stylesheet_once_tags
 
-        expect(response.headers['link']).to eq('</stylesheets/styles.css>; rel=preload; as=style')
+        expect(response.headers['link']).to eq('</stylesheets/styles.css>;rel=preload;as=style')
         expect(response.headers['link']).to_not include('nopush')
       end
     end

spec/services/asset_preload_linker_spec.rb

@@ -0,0 +1,63 @@
+require 'rails_helper'
+
+RSpec.describe AssetPreloadLinker do
+  describe '.append' do
+    let(:link) { nil }
+    let(:as) { 'script' }
+    let(:url) { '/script.js' }
+    let(:crossorigin) { nil }
+    let(:integrity) { nil }
+    let(:headers) { { 'link' => link } }
+    subject(:result) do
+      AssetPreloadLinker.append(**{ headers:, as:, url:, crossorigin:, integrity: }.compact)
+    end
+
+    context 'with absent link value' do
+      let(:link) { nil }
+
+      it 'returns a string with only the appended link' do
+        expect(result).to eq('</script.js>;rel=preload;as=script')
+      end
+    end
+
+    context 'with empty link value' do
+      let(:link) { '' }
+
+      it 'returns a string with only the appended link' do
+        expect(result).to eq('</script.js>;rel=preload;as=script')
+      end
+    end
+
+    context 'with non-empty link value' do
+      let(:link) { '</a.js>;rel=preload;as=script' }
+
+      it 'returns a comma-separated link value of the new and existing link' do
+        expect(result).to eq('</a.js>;rel=preload;as=script,</script.js>;rel=preload;as=script')
+      end
+
+      context 'with existing link value as frozen string' do
+        let(:link) { '</a.js>;rel=preload;as=script'.freeze }
+
+        it 'returns a comma-separated link value of the new and existing link' do
+          expect(result).to eq('</a.js>;rel=preload;as=script,</script.js>;rel=preload;as=script')
+        end
+      end
+    end
+
+    context 'with crossorigin option' do
+      let(:crossorigin) { true }
+
+      it 'includes crossorigin link param' do
+        expect(result).to eq('</script.js>;rel=preload;as=script;crossorigin')
+      end
+    end
+
+    context 'with integrity option' do
+      let(:integrity) { 'abc123' }
+
+      it 'includes integrity link param' do
+        expect(result).to eq('</script.js>;rel=preload;as=script;integrity=abc123')
+      end
+    end
+  end
+end