From 033cebf9786de14975235fc3440c65aa4e83d778 Mon Sep 17 00:00:00 2001 From: jbrw Date: Fri, 11 Sep 2020 13:53:56 -0400 Subject: [PATCH] =?UTF-8?q?DEV=20-=20versions=20of=20JS=20files=20written?= =?UTF-8?q?=20to=20a=20JS=20file=20to=20be=20included=20by=20loa=E2=80=A6?= =?UTF-8?q?=20(#10649)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../admin/components/ace-editor.js | 2 +- .../discourse/app/lib/load-script.js | 19 +++++++++++++ .../discourse/app/lib/public-js-versions.js | 12 ++++++++ lib/tasks/javascript.rake | 27 ++++++++++++++---- test/javascripts/lib/load-script-test.js | 28 ++++++++++++++++++- 5 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/public-js-versions.js diff --git a/app/assets/javascripts/admin/components/ace-editor.js b/app/assets/javascripts/admin/components/ace-editor.js index fe1db1645cb..41c22eff56e 100644 --- a/app/assets/javascripts/admin/components/ace-editor.js +++ b/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 diff --git a/app/assets/javascripts/discourse/app/lib/load-script.js b/app/assets/javascripts/discourse/app/lib/load-script.js index c6ff00216bf..a7665810204 100644 --- a/app/assets/javascripts/discourse/app/lib/load-script.js +++ b/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; +} diff --git a/app/assets/javascripts/discourse/app/lib/public-js-versions.js b/app/assets/javascripts/discourse/app/lib/public-js-versions.js new file mode 100644 index 00000000000..453e0406935 --- /dev/null +++ b/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", +}; diff --git a/lib/tasks/javascript.rake b/lib/tasks/javascript.rake index ad5362c5bce..80d725b05d1 100644 --- a/lib/tasks/javascript.rake +++ b/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 diff --git a/test/javascripts/lib/load-script-test.js b/test/javascripts/lib/load-script-test.js index d3f433108ab..b89618ce7fc 100644 --- a/test/javascripts/lib/load-script-test.js +++ b/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"]}` + ); + } +);