diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d7bd7429229..60e23361be3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -49,7 +49,8 @@ class ApplicationController < ActionController::Base after_action :conditionally_allow_site_embedding after_action :ensure_vary_header after_action :add_noindex_header, if: -> { is_feed_request? || !SiteSetting.allow_index_in_robots_txt } - after_action :add_noindex_header_to_non_canonical, if: -> { request.get? && !(request.format && request.format.json?) && !request.xhr? } + after_action :add_noindex_header_to_non_canonical, if: :spa_boot_request? + around_action :link_preload, if: :spa_boot_request? HONEYPOT_KEY ||= 'HONEYPOT_KEY' CHALLENGE_KEY ||= 'CHALLENGE_KEY' @@ -1008,4 +1009,14 @@ class ApplicationController < ActionController::Base result end + + def link_preload + @links_to_preload = [] + yield + response.headers['Link'] = @links_to_preload.join(', ') if !@links_to_preload.empty? + end + + def spa_boot_request? + request.get? && !(request.format && request.format.json?) && !request.xhr? + end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 0ee00f928ca..d667a5f2c4c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -142,12 +142,16 @@ module ApplicationHelper end def preload_script_url(url) + add_resource_preload_list(url, 'script') <<~HTML.html_safe - HTML end + def add_resource_preload_list(resource_url, type) + @links_to_preload << %Q(<#{resource_url}>; rel="preload"; as="#{type}") if !@links_to_preload.nil? + end + def discourse_csrf_tags # anon can not have a CSRF token cause these are all pages # that may be cached, causing a mismatch between session CSRF @@ -589,7 +593,7 @@ module ApplicationHelper stylesheet_manager end - manager.stylesheet_link_tag(name, 'all') + manager.stylesheet_link_tag(name, 'all', self.method(:add_resource_preload_list)) end def discourse_preload_color_scheme_stylesheets @@ -605,10 +609,10 @@ module ApplicationHelper def discourse_color_scheme_stylesheets result = +"" - result << stylesheet_manager.color_scheme_stylesheet_link_tag(scheme_id, 'all') + result << stylesheet_manager.color_scheme_stylesheet_link_tag(scheme_id, 'all', self.method(:add_resource_preload_list)) if dark_scheme_id != -1 - result << stylesheet_manager.color_scheme_stylesheet_link_tag(dark_scheme_id, '(prefers-color-scheme: dark)') + result << stylesheet_manager.color_scheme_stylesheet_link_tag(dark_scheme_id, '(prefers-color-scheme: dark)', self.method(:add_resource_preload_list)) end result.html_safe diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 7db635e80d1..7fa63d6b27e 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -21,8 +21,8 @@ <%= build_plugin_html 'server:before-script-load' %> - " as="script"> - " as="script"> + <% add_resource_preload_list(script_asset_path("start-discourse"), "script") %> + <% add_resource_preload_list(script_asset_path("browser-update"), "script") %> <%= preload_script 'browser-detect' %> <%= preload_script "locales/#{I18n.locale}" %> diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index 72d4424d0c9..1ee4e8e5579 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -196,10 +196,11 @@ class Stylesheet::Manager end.join("\n").html_safe end - def stylesheet_link_tag(target = :desktop, media = 'all') + def stylesheet_link_tag(target = :desktop, media = 'all', preload_callback = nil) stylesheets = stylesheet_details(target, media) stylesheets.map do |stylesheet| href = stylesheet[:new_href] + preload_callback.call(href, 'style') if preload_callback theme_id = stylesheet[:theme_id] data_theme_id = theme_id ? "data-theme-id=\"#{theme_id}\"" : "" theme_name = stylesheet[:theme_name] @@ -311,12 +312,13 @@ class Stylesheet::Manager %[].html_safe end - def color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all') + def color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all', preload_callback = nil) stylesheet = color_scheme_stylesheet_details(color_scheme_id, media) return '' if !stylesheet href = stylesheet[:new_href] + preload_callback.call(href, 'style') if preload_callback css_class = media == 'all' ? "light-scheme" : "dark-scheme" diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index ee2f1cdc97f..102f8d7c323 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -3,9 +3,8 @@ RSpec.describe ApplicationHelper do describe "preload_script" do - def preload_link(url) + def script_tag(url) <<~HTML - HTML end @@ -32,7 +31,7 @@ RSpec.describe ApplicationHelper do helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br' link = helper.preload_script('discourse') - expect(link).to eq(preload_link("https://awesome.com/brotli_asset/discourse.js")) + expect(link).to eq(script_tag("https://awesome.com/brotli_asset/discourse.js")) end context "with s3 CDN" do @@ -61,36 +60,77 @@ RSpec.describe ApplicationHelper do helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br' link = helper.preload_script('discourse') - expect(link).to eq(preload_link("https://s3cdn.com/assets/discourse.br.js")) + expect(link).to eq(script_tag("https://s3cdn.com/assets/discourse.br.js")) end it "gives s3 cdn if asset host is not set" do link = helper.preload_script('discourse') - expect(link).to eq(preload_link("https://s3cdn.com/assets/discourse.js")) + expect(link).to eq(script_tag("https://s3cdn.com/assets/discourse.js")) end it "can fall back to gzip compression" do helper.request.env["HTTP_ACCEPT_ENCODING"] = 'gzip' link = helper.preload_script('discourse') - expect(link).to eq(preload_link("https://s3cdn.com/assets/discourse.gz.js")) + expect(link).to eq(script_tag("https://s3cdn.com/assets/discourse.gz.js")) end it "gives s3 cdn even if asset host is set" do set_cdn_url "https://awesome.com" link = helper.preload_script('discourse') - expect(link).to eq(preload_link("https://s3cdn.com/assets/discourse.js")) + expect(link).to eq(script_tag("https://s3cdn.com/assets/discourse.js")) end it "gives s3 cdn but without brotli/gzip extensions for theme tests assets" do helper.request.env["HTTP_ACCEPT_ENCODING"] = 'gzip, br' link = helper.preload_script('discourse/tests/theme_qunit_ember_jquery') - expect(link).to eq(preload_link("https://s3cdn.com/assets/discourse/tests/theme_qunit_ember_jquery.js")) + expect(link).to eq(script_tag("https://s3cdn.com/assets/discourse/tests/theme_qunit_ember_jquery.js")) end end end + describe "add_resource_preload_list" do + it "adds resources to the preload list when it's available" do + @links_to_preload = [] + add_resource_preload_list('/assets/discourse.js', 'script') + add_resource_preload_list('/assets/discourse.css', 'style') + + expect(@links_to_preload.size).to eq(2) + end + + it "doesn't add resources to the preload list when it's not available" do + @links_to_preload = nil + add_resource_preload_list('/assets/discourse.js', 'script') + add_resource_preload_list('/assets/discourse.css', 'style') + + expect(@links_to_preload).to eq(nil) + end + + it "adds resources to the preload list when preload_script is called" do + @links_to_preload = [] + helper.preload_script('discourse') + + expect(@links_to_preload.size).to eq(1) + end + + it "adds resources to the preload list when discourse_stylesheet_link_tag is called" do + @links_to_preload = [] + helper.discourse_stylesheet_link_tag(:desktop) + + expect(@links_to_preload.size).to eq(1) + end + + it "adds resources as the correct type" do + @links_to_preload = [] + helper.discourse_stylesheet_link_tag(:desktop) + helper.preload_script('discourse') + + expect(@links_to_preload[0]).to match(/as="style"/) + expect(@links_to_preload[1]).to match(/as="script"/) + end + end + describe "escape_unicode" do it "encodes tags" do expect(helper.escape_unicode("")).to eq("\u003ctag>") diff --git a/spec/lib/stylesheet/manager_spec.rb b/spec/lib/stylesheet/manager_spec.rb index 218f2c49381..caae0e9a830 100644 --- a/spec/lib/stylesheet/manager_spec.rb +++ b/spec/lib/stylesheet/manager_spec.rb @@ -148,6 +148,16 @@ RSpec.describe Stylesheet::Manager do }) end + it "stylesheet_link_tag calls the preload callback when set" do + preload_list = [] + preload_callback = ->(href, type) { preload_list << [href, type] } + + manager = manager(theme.id) + expect { + manager.stylesheet_link_tag(:desktop_theme, 'all', preload_callback) + }.to change(preload_list, :size) + end + context "with stylesheet order" do let(:z_child_theme) do Fabricate(:theme, component: true, name: "ze component").tap do |z| @@ -638,6 +648,17 @@ RSpec.describe Stylesheet::Manager do expect(details1[:new_href]).not_to eq(details2[:new_href]) end + it "calls the preload callback when set" do + preload_list = [] + cs = Fabricate(:color_scheme, name: 'Funky') + theme = Fabricate(:theme, color_scheme_id: cs.id) + preload_callback = ->(href, type) { preload_list << [href, type] } + + expect { + manager.color_scheme_stylesheet_link_tag(theme.id, 'all', preload_callback) + }.to change(preload_list, :size).by(1) + end + context "with theme colors" do let(:theme) { Fabricate(:theme).tap { |t| t.set_field(target: :common, name: "color_definitions", value: ':root {--special: rebeccapurple;}') diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index d25d1a983d6..1828b1898c8 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -1124,4 +1124,16 @@ RSpec.describe ApplicationController do end end end + + describe 'preload Link header' do + it "should have the Link header with assets on full page requests" do + get("/latest") + expect(response.headers).to include('Link') + end + + it "shouldn't have the Link header on xhr api requests" do + get("/latest.json") + expect(response.headers).not_to include('Link') + end + end end