From 804b8fd9f94d0d67152d6177241b14722aba6f61 Mon Sep 17 00:00:00 2001
From: Joe <33972521+hnb-ku@users.noreply.github.com>
Date: Mon, 20 Jun 2022 09:47:37 +0800
Subject: [PATCH] DEV: Defer loading core/plugin/theme JS files (#17063)

This is pre-request work to introduce a splash screen while site assets load.

The only change this commit introduces is that it ensures we add the defer attribute to core/plugin/theme .JS files. This will allow us to insert markup before the browser starts evaluating those scripts later on. It has no visual or functional impact on core.

This will not have any impact on how themes and plugins work. The only exception is themes loading external scripts in the </head> theme field directly via script tags. Everything will work the same but those would need to add the defer attribute if they want to keep the benefits introduced in this PR.
---
 app/assets/javascripts/discourse/app/index.html       |  8 +++++---
 app/assets/javascripts/discourse/ember-cli-build.js   |  1 +
 .../javascripts/discourse/lib/bootstrap-json/index.js |  8 ++++----
 app/assets/javascripts/discourse/tests/index.html     |  2 ++
 .../javascripts/discourse/tests/test_starter.js       | 11 ++++++-----
 app/helpers/application_helper.rb                     |  2 +-
 app/models/theme.rb                                   |  4 ++--
 app/models/theme_field.rb                             |  4 ++--
 app/views/layouts/application.html.erb                |  4 ++--
 spec/helpers/application_helper_spec.rb               |  2 +-
 spec/models/theme_field_spec.rb                       |  8 ++++----
 11 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/index.html b/app/assets/javascripts/discourse/app/index.html
index ba049ba54ce..0dde7cb0ab4 100644
--- a/app/assets/javascripts/discourse/app/index.html
+++ b/app/assets/javascripts/discourse/app/index.html
@@ -11,8 +11,10 @@
     <bootstrap-content key="before-script-load">
     {{content-for "before-script-load"}}
 
-    <script src="{{rootURL}}assets/vendor.js"></script>
-    <script src="{{rootURL}}assets/discourse.js"></script>
+    <script defer src="{{rootURL}}assets/vendor.js"></script>
+    <script defer src="{{rootURL}}assets/discourse.js"></script>
+
+    <ember-auto-import-scripts defer entrypoint="app"></ember-auto-import-scripts>
 
     <bootstrap-content key="head">
     {{content-for "head"}}
@@ -27,7 +29,7 @@
     <bootstrap-content key="hidden-login-form">
     <bootstrap-content key="preloaded">
 
-    <script src="{{rootURL}}assets/start-discourse.js"></script>
+    <script defer src="{{rootURL}}assets/start-discourse.js"></script>
 
     <bootstrap-content key="body-footer">
     {{content-for "body-footer"}}
diff --git a/app/assets/javascripts/discourse/ember-cli-build.js b/app/assets/javascripts/discourse/ember-cli-build.js
index d9232652ab8..7065afc5573 100644
--- a/app/assets/javascripts/discourse/ember-cli-build.js
+++ b/app/assets/javascripts/discourse/ember-cli-build.js
@@ -29,6 +29,7 @@ module.exports = function (defaults) {
     },
     autoImport: {
       forbidEval: true,
+      insertScriptsAt: "ember-auto-import-scripts",
     },
     fingerprint: {
       // Handled by Rails asset pipeline
diff --git a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js
index a1fcc954366..46c891ecc6a 100644
--- a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js
+++ b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js
@@ -89,7 +89,7 @@ function head(buffer, bootstrap, headers, baseURL) {
     let { admin, staff } = user;
 
     if (staff) {
-      buffer.push(`<script src="${baseURL}assets/admin.js"></script>`);
+      buffer.push(`<script defer src="${baseURL}assets/admin.js"></script>`);
     }
 
     if (admin) {
@@ -98,7 +98,7 @@ function head(buffer, bootstrap, headers, baseURL) {
   }
 
   bootstrap.plugin_js.forEach((src) =>
-    buffer.push(`<script src="${src}"></script>`)
+    buffer.push(`<script defer src="${src}"></script>`)
   );
 
   buffer.push(bootstrap.theme_html.translations);
@@ -108,14 +108,14 @@ function head(buffer, bootstrap, headers, baseURL) {
 }
 
 function localeScript(buffer, bootstrap) {
-  buffer.push(`<script src="${bootstrap.locale_script}"></script>`);
+  buffer.push(`<script defer src="${bootstrap.locale_script}"></script>`);
 }
 
 function beforeScriptLoad(buffer, bootstrap) {
   buffer.push(bootstrap.html.before_script_load);
   localeScript(buffer, bootstrap);
   (bootstrap.extra_locales || []).forEach((l) =>
-    buffer.push(`<script src="${l}"></script>`)
+    buffer.push(`<script defer src="${l}"></script>`)
   );
 }
 
diff --git a/app/assets/javascripts/discourse/tests/index.html b/app/assets/javascripts/discourse/tests/index.html
index cc64b777149..8917097f05c 100644
--- a/app/assets/javascripts/discourse/tests/index.html
+++ b/app/assets/javascripts/discourse/tests/index.html
@@ -46,7 +46,9 @@
 
     <script src="/testem.js" integrity=""></script>
     <script src="{{rootURL}}assets/vendor.js"></script>
+    <ember-auto-import-scripts entrypoint="app"></ember-auto-import-scripts>
     <script src="{{rootURL}}assets/test-support.js"></script>
+    <ember-auto-import-scripts entrypoint="tests"></ember-auto-import-scripts>
     <script src="{{rootURL}}assets/discourse.js"></script>
     <script src="{{rootURL}}assets/discourse-markdown.js"></script>
     <script src="{{rootURL}}assets/admin.js"></script>
diff --git a/app/assets/javascripts/discourse/tests/test_starter.js b/app/assets/javascripts/discourse/tests/test_starter.js
index 0ab61639ba7..067e890fc16 100644
--- a/app/assets/javascripts/discourse/tests/test_starter.js
+++ b/app/assets/javascripts/discourse/tests/test_starter.js
@@ -1,10 +1,11 @@
 // discourse-skip-module
 
-document.write(
-  '<div id="ember-testing-container"><div id="ember-testing"></div></div>'
-);
-document.write(
-  "<style>#ember-testing-container { position: fixed; background: white; bottom: 0; right: 0; width: 640px; height: 384px; overflow: auto; z-index: 9999; border: 1px solid #ccc; transform: translateZ(0)} #ember-testing { width: 200%; height: 200%; transform: scale(0.5); transform-origin: top left; }</style>"
+document.body.insertAdjacentHTML(
+  "afterbegin",
+  `
+    <div id="ember-testing-container"><div id="ember-testing"></div></div>
+    <style>#ember-testing-container { position: fixed; background: white; bottom: 0; right: 0; width: 640px; height: 384px; overflow: auto; z-index: 9999; border: 1px solid #ccc; transform: translateZ(0)} #ember-testing { width: 200%; height: 200%; transform: scale(0.5); transform-origin: top left; }</style>
+  `
 );
 
 let setupTestsLegacy = require("discourse/tests/setup-tests").setupTestsLegacy;
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 5773efd851f..f069f5560c1 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -153,7 +153,7 @@ module ApplicationHelper
   def preload_script_url(url)
     <<~HTML.html_safe
       <link rel="preload" href="#{url}" as="script">
-      <script src="#{url}"></script>
+      <script defer src="#{url}"></script>
     HTML
   end
 
diff --git a/app/models/theme.rb b/app/models/theme.rb
index 5c510086f7f..6488b36e52d 100644
--- a/app/models/theme.rb
+++ b/app/models/theme.rb
@@ -154,7 +154,7 @@ class Theme < ActiveRecord::Base
     SvgSprite.expire_cache
   end
 
-  BASE_COMPILER_VERSION = 55
+  BASE_COMPILER_VERSION = 56
   def self.compiler_version
     get_set_cache "compiler_version" do
       dependencies = [
@@ -391,7 +391,7 @@ class Theme < ActiveRecord::Base
       end
       caches = JavascriptCache.where(theme_id: theme_ids)
       caches = caches.sort_by { |cache| theme_ids.index(cache.theme_id) }
-      return caches.map { |c| "<script src='#{c.url}' data-theme-id='#{c.theme_id}'></script>" }.join("\n")
+      return caches.map { |c| "<script defer src='#{c.url}' data-theme-id='#{c.theme_id}'></script>" }.join("\n")
     end
     list_baked_fields(theme_ids, target, name).map { |f| f.value_baked || f.value }.join("\n")
   end
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index bdc017c6617..a48b5f92186 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -149,7 +149,7 @@ class ThemeField < ActiveRecord::Base
     javascript_cache.content = js_compiler.content
     javascript_cache.save!
 
-    doc.add_child("<script src='#{javascript_cache.url}' data-theme-id='#{theme_id}'></script>") if javascript_cache.content.present?
+    doc.add_child("<script defer src='#{javascript_cache.url}' data-theme-id='#{theme_id}'></script>") if javascript_cache.content.present?
     [doc.to_s, errors&.join("\n")]
   end
 
@@ -265,7 +265,7 @@ class ThemeField < ActiveRecord::Base
     javascript_cache.content = js_compiler.content
     javascript_cache.save!
     doc = ""
-    doc = "<script src='#{javascript_cache.url}' data-theme-id='#{theme_id}'></script>" if javascript_cache.content.present?
+    doc = "<script defer src='#{javascript_cache.url}' data-theme-id='#{theme_id}'></script>" if javascript_cache.content.present?
     [doc, errors&.join("\n")]
   end
 
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index 83e56dff16e..0ab67a126c0 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -112,11 +112,11 @@
 
     <div class="hidden" id="data-preloaded" data-preloaded="<%= preloaded_json %>"></div>
 
-    <script src="<%= script_asset_path "start-discourse" %>"></script>
+    <script defer src="<%= script_asset_path "start-discourse" %>"></script>
 
     <%= yield :data %>
 
-    <script src="<%= script_asset_path "browser-update" %>"></script>
+    <script defer src="<%= script_asset_path "browser-update" %>"></script>
 
     <%- unless customization_disabled? %>
       <%= theme_lookup("body_tag") %>
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index e7af0c68118..fdf4ce1faa2 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -7,7 +7,7 @@ describe ApplicationHelper do
     def preload_link(url)
       <<~HTML
           <link rel="preload" href="#{url}" as="script">
-          <script src="#{url}"></script>
+          <script defer src="#{url}"></script>
       HTML
     end
 
diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb
index ad43c0d1e09..c4eca24304e 100644
--- a/spec/models/theme_field_spec.rb
+++ b/spec/models/theme_field_spec.rb
@@ -75,7 +75,7 @@ describe ThemeField do
 
     theme_field = ThemeField.create!(theme_id: 1, target_id: 0, name: "header", value: html)
     theme_field.ensure_baked!
-    expect(theme_field.value_baked).to include("<script src=\"#{theme_field.javascript_cache.url}\" data-theme-id=\"1\"></script>")
+    expect(theme_field.value_baked).to include("<script defer=\"\" src=\"#{theme_field.javascript_cache.url}\" data-theme-id=\"1\"></script>")
     expect(theme_field.value_baked).to include("external-script.js")
     expect(theme_field.value_baked).to include('<script type="text/template"')
     expect(theme_field.javascript_cache.content).to include('a = "inline discourse plugin"')
@@ -110,7 +110,7 @@ HTML
     field = ThemeField.create!(theme_id: 1, target_id: 0, name: "header", value: html)
     field.ensure_baked!
     expect(field.error).not_to eq(nil)
-    expect(field.value_baked).to include("<script src=\"#{field.javascript_cache.url}\" data-theme-id=\"1\"></script>")
+    expect(field.value_baked).to include("<script defer=\"\" src=\"#{field.javascript_cache.url}\" data-theme-id=\"1\"></script>")
     expect(field.javascript_cache.content).to include("Theme Transpilation Error:")
 
     field.update!(value: '')
@@ -130,7 +130,7 @@ HTML
     theme_field.ensure_baked!
     javascript_cache = theme_field.javascript_cache
 
-    expect(theme_field.value_baked).to include("<script src=\"#{javascript_cache.url}\" data-theme-id=\"1\"></script>")
+    expect(theme_field.value_baked).to include("<script defer=\"\" src=\"#{javascript_cache.url}\" data-theme-id=\"1\"></script>")
     expect(javascript_cache.content).to include("testing-div")
     expect(javascript_cache.content).to include("string_setting")
     expect(javascript_cache.content).to include("test text \\\" 123!")
@@ -378,7 +378,7 @@ HTML
     describe "javascript cache" do
       it "is generated correctly" do
         fr1.ensure_baked!
-        expect(fr1.value_baked).to include("<script src='#{fr1.javascript_cache.url}' data-theme-id='#{fr1.theme_id}'></script>")
+        expect(fr1.value_baked).to include("<script defer src='#{fr1.javascript_cache.url}' data-theme-id='#{fr1.theme_id}'></script>")
         expect(fr1.javascript_cache.content).to include("bonjourworld")
         expect(fr1.javascript_cache.content).to include("helloworld")
         expect(fr1.javascript_cache.content).to include("enval1")