DEV - versions of JS files written to a JS file to be included by loa… (#10649)

* DEV - versions of JS files written to a JS file to be included by load-script and appended as params to URLs

* Formatting

* Incorporate feedback from PR

* Update filename of public-js-versions

app/assets/javascripts/admin/components/ace-editor.js

@@ -81,7 +81,7 @@ export default Component.extend({
 
   didInsertElement() {
     this._super(...arguments);
-    loadScript("/javascripts/ace/ace.js?v=1.4.12").then(() => {
+    loadScript("/javascripts/ace/ace.js").then(() => {
       window.ace.require(["ace/ace"], (loadedAce) => {
         loadedAce.config.set("loadWorkerFromBlob", false);
         loadedAce.config.set("workerPath", getURL("/javascripts/ace")); // Do not use CDN for workers

app/assets/javascripts/discourse/app/lib/load-script.js

@@ -1,6 +1,7 @@
 import { default as getURL, getURLWithCDN } from "discourse-common/lib/get-url";
 import { run } from "@ember/runloop";
 import { ajax } from "discourse/lib/ajax";
+import { PUBLIC_JS_VERSIONS } from "discourse/lib/public-js-versions";
 import { Promise } from "rsvp";
 
 const _loaded = {};
@@ -50,6 +51,10 @@ export default function loadScript(url, opts) {
     return Promise.resolve();
   }
 
+  if (PUBLIC_JS_VERSIONS && !opts.css) {
+    url = cacheBuster(url);
+  }
+
   // Scripts should always load from CDN
   // CSS is type text, to accept it from a CDN we would need to handle CORS
   const fullUrl = opts.css ? getURL(url) : getURLWithCDN(url);
@@ -102,3 +107,17 @@ export default function loadScript(url, opts) {
     }
   });
 }
+
+export function cacheBuster(url) {
+  if (PUBLIC_JS_VERSIONS) {
+    const pathParts = url.split("/");
+    if (pathParts[1] === "javascripts") {
+      const version = PUBLIC_JS_VERSIONS[pathParts[2]];
+      if (typeof version !== "undefined") {
+        return `${url}?v=${version}`;
+      }
+    }
+  }
+
+  return url;
+}

app/assets/javascripts/discourse/app/lib/public-js-versions.js

@@ -0,0 +1,12 @@
+// DO NOT EDIT THIS FILE!!!
+// Update it by running `rake javascript:update`
+
+export const PUBLIC_JS_VERSIONS = {
+  ace: "1.4.12",
+  "Chart.min.js": "2.9.3",
+  "chartjs-plugin-datalabels.min.js": "0.7.0",
+  "jquery.magnific-popup.min.js": "1.1.0",
+  "pikaday.js": "1.8.0",
+  "spectrum.js": "1.8.0",
+  workbox: "4.3.1",
+};

lib/tasks/javascript.rake

@@ -16,10 +16,10 @@ def library_src
   "#{Rails.root}/node_modules"
 end
 
-def write_template(path, template)
+def write_template(path, task_name, template)
   header = <<~HEADER
     // DO NOT EDIT THIS FILE!!!
-    // Update it by running `rake javascript:update_constants`
+    // Update it by running `rake javascript:#{task_name}`
   HEADER
 
   basename = File.basename(path)
@@ -32,13 +32,15 @@ def write_template(path, template)
 end
 
 task 'javascript:update_constants' => :environment do
-  write_template("discourse/app/lib/constants.js", <<~JS)
+  task_name = 'update_constants'
+
+  write_template("discourse/app/lib/constants.js", task_name, <<~JS)
     export const SEARCH_PRIORITIES = #{Searchable::PRIORITIES.to_json};
 
     export const SEARCH_PHRASE_REGEXP = '#{Search::PHRASE_MATCH_REGEXP_PATTERN}';
   JS
 
-  write_template("pretty-text/addon/emoji/data.js", <<~JS)
+  write_template("pretty-text/addon/emoji/data.js", task_name, <<~JS)
     export const emojis = #{Emoji.standard.map(&:name).flatten.inspect};
     export const tonableEmojis = #{Emoji.tonable_emojis.flatten.inspect};
     export const aliases = #{Emoji.aliases.inspect.gsub("=>", ":")};
@@ -47,7 +49,7 @@ task 'javascript:update_constants' => :environment do
     export const replacements = #{Emoji.unicode_replacements_json};
   JS
 
-  write_template("pretty-text/addon/emoji/version.js", <<~JS)
+  write_template("pretty-text/addon/emoji/version.js", task_name, <<~JS)
     export const IMAGE_VERSION = "#{Emoji::EMOJI_VERSION}";
   JS
 end
@@ -84,7 +86,8 @@ task 'javascript:update' do
       public: true
     }, {
       source: 'spectrum-colorpicker/spectrum.css',
-      public: true
+      public: true,
+      skip_versioning: true
     }, {
       source: 'favcount/favcount.js'
     }, {
@@ -171,6 +174,7 @@ task 'javascript:update' do
 
   ]
 
+  versions = {}
   start = Time.now
 
   dependencies.each do |f|
@@ -202,6 +206,12 @@ task 'javascript:update' do
     if f[:public_root]
       dest = "#{public_root}/#{filename}"
     elsif f[:public]
+      unless f[:skip_versioning]
+        package_name = f[:source].split('/').first
+        package_version = JSON.parse(File.read("#{library_src}/#{package_name}/package.json"))["version"]
+        versions[filename] = package_version
+      end
+
       dest = "#{public_js}/#{filename}"
     else
       dest = "#{vendor_js}/#{filename}"
@@ -210,6 +220,7 @@ task 'javascript:update' do
     if src.include? "ace.js"
       ace_root = "#{library_src}/ace-builds/src-min-noconflict/"
       addtl_files = [ "ext-searchbox", "mode-html", "mode-scss", "mode-sql", "theme-chrome", "worker-html"]
+      FileUtils.mkdir(dest) unless File.directory?(dest)
       addtl_files.each do |file|
         FileUtils.cp_r("#{ace_root}#{file}.js", dest)
       end
@@ -232,5 +243,9 @@ task 'javascript:update' do
     end
   end
 
+  write_template("discourse/app/lib/public-js-versions.js", "update", <<~JS)
+    export const PUBLIC_JS_VERSIONS = #{versions.to_json};
+  JS
+
   STDERR.puts "Completed copying dependencies: #{(Time.now - start).round(2)} secs"
 end

test/javascripts/lib/load-script-test.js

@@ -1,4 +1,5 @@
-import loadScript from "discourse/lib/load-script";
+import { loadScript, cacheBuster } from "discourse/lib/load-script";
+import { PUBLIC_JS_VERSIONS as jsVersions } from "discourse/lib/public-js-versions";
 
 QUnit.module("lib:load-script");
 
@@ -19,3 +20,28 @@ QUnit.skip(
     );
   }
 );
+
+QUnit.test("works when a value is not present", (assert) => {
+  assert.equal(
+    cacheBuster("/javascripts/my-script.js"),
+    "/javascripts/my-script.js"
+  );
+  assert.equal(
+    cacheBuster("/javascripts/my-project/script.js"),
+    "/javascripts/my-project/script.js"
+  );
+});
+
+QUnit.test(
+  "generates URLs with version number in the query params",
+  (assert) => {
+    assert.equal(
+      cacheBuster("/javascripts/pikaday.js"),
+      `/javascripts/pikaday.js?v=${jsVersions["pikaday.js"]}`
+    );
+    assert.equal(
+      cacheBuster("/javascripts/ace/ace.js"),
+      `/javascripts/ace/ace.js?v=${jsVersions["ace"]}`
+    );
+  }
+);