Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "FEATURE: Preload resources via link header (#18475)" (#18511) 

This reverts commit 2d1dbc6f96.

We need to increase nginx proxy buffer to land this.
This commit is contained in:
Rafael dos Santos Silva 2022-10-07 15:08:40 -03:00 committed by GitHub
parent 2d1dbc6f96
commit 95a57f7e0c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 17 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
app/controllers/application_controller.rb
View File

@ -49,8 +49,7 @@ 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: :spa_boot_request?
around_action :link_preload, if: :spa_boot_request?
after_action :add_noindex_header_to_non_canonical, if: -> { request.get? && !(request.format && request.format.json?) && !request.xhr? }
HONEYPOT_KEY ||= 'HONEYPOT_KEY'
CHALLENGE_KEY ||= 'CHALLENGE_KEY'
@ -1009,14 +1008,4 @@ 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

12
app/helpers/application_helper.rb
View File

@ -142,16 +142,12 @@ module ApplicationHelper
end
def preload_script_url(url)
add_resource_preload_list(url, 'script')
<<~HTML.html_safe
<link rel="preload" href="#{url}" as="script">
<script defer src="#{url}"></script>
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
@ -593,7 +589,7 @@ module ApplicationHelper
stylesheet_manager
end
manager.stylesheet_link_tag(name, 'all', self.method(:add_resource_preload_list))
manager.stylesheet_link_tag(name, 'all')
end
def discourse_preload_color_scheme_stylesheets
@ -609,10 +605,10 @@ module ApplicationHelper
def discourse_color_scheme_stylesheets
result = +""
result << stylesheet_manager.color_scheme_stylesheet_link_tag(scheme_id, 'all', self.method(:add_resource_preload_list))
result << stylesheet_manager.color_scheme_stylesheet_link_tag(scheme_id, 'all')
if dark_scheme_id != -1
result << stylesheet_manager.color_scheme_stylesheet_link_tag(dark_scheme_id, '(prefers-color-scheme: dark)', self.method(:add_resource_preload_list))
result << stylesheet_manager.color_scheme_stylesheet_link_tag(dark_scheme_id, '(prefers-color-scheme: dark)')
end
result.html_safe

4
app/views/layouts/application.html.erb
View File

@ -21,8 +21,8 @@
<%= build_plugin_html 'server:before-script-load' %>
<% add_resource_preload_list(script_asset_path("start-discourse"), "script") %>
<% add_resource_preload_list(script_asset_path("browser-update"), "script") %>
<link rel="preload" href="<%= script_asset_path "start-discourse" %>" as="script">
<link rel="preload" href="<%= script_asset_path "browser-update" %>" as="script">
<%= preload_script 'browser-detect' %>
<%= preload_script "locales/#{I18n.locale}" %>

6
lib/stylesheet/manager.rb
View File

@ -196,11 +196,10 @@ class Stylesheet::Manager
end.join("\n").html_safe
end
def stylesheet_link_tag(target = :desktop, media = 'all', preload_callback = nil)
def stylesheet_link_tag(target = :desktop, media = 'all')
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]
@ -312,13 +311,12 @@ class Stylesheet::Manager
%[<link href="#{href}" rel="preload" as="style"/>].html_safe
end
def color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all', preload_callback = nil)
def color_scheme_stylesheet_link_tag(color_scheme_id = nil, media = 'all')
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"

56
spec/helpers/application_helper_spec.rb
View File

@ -3,8 +3,9 @@
RSpec.describe ApplicationHelper do
describe "preload_script" do
def script_tag(url)
def preload_link(url)
<<~HTML
<link rel="preload" href="#{url}" as="script">
<script defer src="#{url}"></script>
HTML
end
@ -31,7 +32,7 @@ RSpec.describe ApplicationHelper do
helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br'
link = helper.preload_script('discourse')
expect(link).to eq(script_tag("https://awesome.com/brotli_asset/discourse.js"))
expect(link).to eq(preload_link("https://awesome.com/brotli_asset/discourse.js"))
end
context "with s3 CDN" do
@ -60,77 +61,36 @@ RSpec.describe ApplicationHelper do
helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br'
link = helper.preload_script('discourse')
expect(link).to eq(script_tag("https://s3cdn.com/assets/discourse.br.js"))
expect(link).to eq(preload_link("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(script_tag("https://s3cdn.com/assets/discourse.js"))
expect(link).to eq(preload_link("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(script_tag("https://s3cdn.com/assets/discourse.gz.js"))
expect(link).to eq(preload_link("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(script_tag("https://s3cdn.com/assets/discourse.js"))
expect(link).to eq(preload_link("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(script_tag("https://s3cdn.com/assets/discourse/tests/theme_qunit_ember_jquery.js"))
expect(link).to eq(preload_link("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("<tag>")).to eq("\u003ctag>")

21
spec/lib/stylesheet/manager_spec.rb
View File

@ -148,16 +148,6 @@ 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|
@ -648,17 +638,6 @@ 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;}')

12
spec/requests/application_controller_spec.rb
View File

@ -1124,16 +1124,4 @@ 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