From e06b9d4a52f3df5bd7cdef229fc5c21423fdbf94 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 21 Sep 2022 12:38:02 +0100 Subject: [PATCH] DEV: Remove support for legacy plugin JS compilation pipeline (#18293) This became the default in b1755137 --- .github/workflows/tests.yml | 9 +- app/assets/javascripts/admin-plugins.js.erb | 11 --- .../javascripts/discourse/ember-cli-build.js | 12 +-- .../discourse/lib/bootstrap-json/index.js | 86 ++++++++----------- app/assets/javascripts/discourse/testem.js | 9 -- .../discourse/tests/active-plugins.js.erb | 11 --- .../javascripts/discourse/tests/core-tests.js | 3 - .../discourse/tests/plugin-tests.js.erb | 10 --- app/views/qunit/theme.html.erb | 1 - config/initializers/assets.rb | 2 - lib/ember_cli.rb | 12 +-- lib/plugin/instance.rb | 31 ++----- lib/tasks/qunit.rake | 2 +- 13 files changed, 52 insertions(+), 147 deletions(-) delete mode 100644 app/assets/javascripts/admin-plugins.js.erb delete mode 100644 app/assets/javascripts/discourse/tests/active-plugins.js.erb delete mode 100644 app/assets/javascripts/discourse/tests/core-tests.js delete mode 100644 app/assets/javascripts/discourse/tests/plugin-tests.js.erb diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index eee600f74da..bf9a71716a5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,7 +17,7 @@ permissions: jobs: build: - name: ${{ matrix.target }} ${{ matrix.build_type }} ${{ ((matrix.ember_cli_plugin_assets == '1') && '(ember-cli-compiled plugin js)') || ''}} + name: ${{ matrix.target }} ${{ matrix.build_type }} runs-on: ubuntu-latest container: discourse/discourse_test:slim${{ startsWith(matrix.build_type, 'frontend') && '-browsers' || '' }} timeout-minutes: 60 @@ -29,7 +29,6 @@ jobs: PGUSER: discourse PGPASSWORD: discourse USES_PARALLEL_DATABASES: ${{ matrix.build_type == 'backend' }} - EMBER_CLI_PLUGIN_ASSETS: ${{ matrix.ember_cli_plugin_assets }} strategy: fail-fast: false @@ -37,17 +36,11 @@ jobs: matrix: build_type: [backend, frontend, annotations] target: [core, plugins] - ember_cli_plugin_assets: ['1', '0'] exclude: - build_type: annotations target: plugins - build_type: frontend target: core # Handled by core_frontend_tests job (below) - - target: core - ember_cli_plugin_assets: '1' - - target: plugins - build_type: backend - ember_cli_plugin_assets: '1' steps: - uses: actions/checkout@v3 diff --git a/app/assets/javascripts/admin-plugins.js.erb b/app/assets/javascripts/admin-plugins.js.erb deleted file mode 100644 index 70010c46443..00000000000 --- a/app/assets/javascripts/admin-plugins.js.erb +++ /dev/null @@ -1,11 +0,0 @@ -<% -DiscoursePluginRegistry.admin_javascripts.each { |js| require_asset(js) } - -DiscoursePluginRegistry.each_globbed_asset(admin: true) do |f| - if File.directory?(f) - depend_on(f) - else - require_asset(f) - end -end -%> diff --git a/app/assets/javascripts/discourse/ember-cli-build.js b/app/assets/javascripts/discourse/ember-cli-build.js index e8784e95516..8fa10046517 100644 --- a/app/assets/javascripts/discourse/ember-cli-build.js +++ b/app/assets/javascripts/discourse/ember-cli-build.js @@ -145,15 +145,9 @@ module.exports = function (defaults) { "/app/assets/javascripts/discourse/public/assets/scripts/module-shims.js" ); - let discoursePluginsTree; - if (process.env.EMBER_CLI_PLUGIN_ASSETS !== "0") { - discoursePluginsTree = app.project - .findAddonByName("discourse-plugins") - .generatePluginsTree(); - } else { - // Empty tree - no-op - discoursePluginsTree = mergeTrees([]); - } + const discoursePluginsTree = app.project + .findAddonByName("discourse-plugins") + .generatePluginsTree(); const terserPlugin = app.project.findAddonByName("ember-cli-terser"); const applyTerser = (tree) => terserPlugin.postprocessTree("all", tree); diff --git a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js index 82ee8509c1c..b2b243fdc56 100644 --- a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js +++ b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js @@ -381,44 +381,36 @@ module.exports = { if (shouldLoadPluginTestJs() && type === "test-plugin-js") { const scripts = []; - if (process.env.EMBER_CLI_PLUGIN_ASSETS !== "0") { - const pluginInfos = this.app.project - .findAddonByName("discourse-plugins") - .pluginInfos(); + const pluginInfos = this.app.project + .findAddonByName("discourse-plugins") + .pluginInfos(); - for (const { - pluginName, - directoryName, - hasJs, - hasAdminJs, - } of pluginInfos) { - if (hasJs) { - scripts.push({ - src: `plugins/${directoryName}.js`, - name: pluginName, - }); - } - - if (fs.existsSync(`../plugins/${directoryName}_extras.js.erb`)) { - scripts.push({ - src: `plugins/${directoryName}_extras.js`, - name: pluginName, - }); - } - - if (hasAdminJs) { - scripts.push({ - src: `plugins/${directoryName}_admin.js`, - name: pluginName, - }); - } + for (const { + pluginName, + directoryName, + hasJs, + hasAdminJs, + } of pluginInfos) { + if (hasJs) { + scripts.push({ + src: `plugins/${directoryName}.js`, + name: pluginName, + }); + } + + if (fs.existsSync(`../plugins/${directoryName}_extras.js.erb`)) { + scripts.push({ + src: `plugins/${directoryName}_extras.js`, + name: pluginName, + }); + } + + if (hasAdminJs) { + scripts.push({ + src: `plugins/${directoryName}_admin.js`, + name: pluginName, + }); } - } else { - scripts.push({ - src: "discourse/tests/active-plugins.js", - name: "_all", - }); - scripts.push({ src: "admin-plugins.js", name: "_admin" }); } return scripts @@ -428,19 +420,15 @@ module.exports = { ) .join("\n"); } else if (shouldLoadPluginTestJs() && type === "test-plugin-tests-js") { - if (process.env.EMBER_CLI_PLUGIN_ASSETS !== "0") { - return this.app.project - .findAddonByName("discourse-plugins") - .pluginInfos() - .filter(({ hasTests }) => hasTests) - .map( - ({ directoryName, pluginName }) => - `` - ) - .join("\n"); - } else { - return ``; - } + return this.app.project + .findAddonByName("discourse-plugins") + .pluginInfos() + .filter(({ hasTests }) => hasTests) + .map( + ({ directoryName, pluginName }) => + `` + ) + .join("\n"); } }, diff --git a/app/assets/javascripts/discourse/testem.js b/app/assets/javascripts/discourse/testem.js index 94c77831ba1..61483738691 100644 --- a/app/assets/javascripts/discourse/testem.js +++ b/app/assets/javascripts/discourse/testem.js @@ -98,15 +98,6 @@ if (process.argv.includes("-t")) { "/assets/plugins/*_extra.js": { target, }, - "/assets/discourse/tests/active-plugins.js": { - target, - }, - "/assets/admin-plugins.js": { - target, - }, - "/assets/discourse/tests/plugin-tests.js": { - target, - }, "/plugins/": { target, }, diff --git a/app/assets/javascripts/discourse/tests/active-plugins.js.erb b/app/assets/javascripts/discourse/tests/active-plugins.js.erb deleted file mode 100644 index 6b2958541e9..00000000000 --- a/app/assets/javascripts/discourse/tests/active-plugins.js.erb +++ /dev/null @@ -1,11 +0,0 @@ -<% - DiscoursePluginRegistry.javascripts.each { |js| require_asset(js) } - DiscoursePluginRegistry.handlebars.each { |hb| require_asset(hb) } - DiscoursePluginRegistry.each_globbed_asset do |f| - if File.directory?(f) - depend_on(f) - else - require_asset(f) - end - end -%> diff --git a/app/assets/javascripts/discourse/tests/core-tests.js b/app/assets/javascripts/discourse/tests/core-tests.js deleted file mode 100644 index 1890b65888e..00000000000 --- a/app/assets/javascripts/discourse/tests/core-tests.js +++ /dev/null @@ -1,3 +0,0 @@ -//= require_tree ./acceptance -//= require_tree ./integration -//= require_tree ./unit diff --git a/app/assets/javascripts/discourse/tests/plugin-tests.js.erb b/app/assets/javascripts/discourse/tests/plugin-tests.js.erb deleted file mode 100644 index 6e5d6bae588..00000000000 --- a/app/assets/javascripts/discourse/tests/plugin-tests.js.erb +++ /dev/null @@ -1,10 +0,0 @@ -<% - Discourse.plugins.each do |p| - root_path = "#{File.dirname(p.path)}/test/javascripts" - - to_glob = [root_path + '/**/**.es6'] - to_glob << (root_path + '/**/**.js') - - Dir.glob(to_glob) { |f| require_asset(f) } - end -%> diff --git a/app/views/qunit/theme.html.erb b/app/views/qunit/theme.html.erb index c6d026d9edc..f2db3572fb1 100644 --- a/app/views/qunit/theme.html.erb +++ b/app/views/qunit/theme.html.erb @@ -11,7 +11,6 @@ <%- Discourse.find_plugin_js_assets(include_disabled: true).each do |file| %> <%= preload_script file %> <%- end %> - <%= preload_script "admin-plugins" %> <%= preload_script "test-support" %> <%= preload_script "test-helpers" %> <%= theme_translations_lookup %> diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index 0fce6d980b2..84bc719d323 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -46,8 +46,6 @@ Rails.application.config.assets.precompile += %w{ confirm-new-email/bootstrap.js onpopstate-handler.js embed-application.js - discourse/tests/active-plugins.js - admin-plugins.js scripts/discourse-test-listen-boot scripts/discourse-boot } diff --git a/lib/ember_cli.rb b/lib/ember_cli.rb index 181dd72c791..fd3892918b5 100644 --- a/lib/ember_cli.rb +++ b/lib/ember_cli.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true module EmberCli - def self.plugin_assets? - ENV["EMBER_CLI_PLUGIN_ASSETS"] != "0" - end - def self.assets @assets ||= begin assets = %w( @@ -18,11 +14,9 @@ module EmberCli ) assets += Dir.glob("app/assets/javascripts/discourse/scripts/*.js").map { |f| File.basename(f) } - if plugin_assets? - Discourse.find_plugin_js_assets(include_disabled: true).each do |file| - next if file.ends_with?("_extra") # these are still handled by sprockets - assets << "#{file}.js" - end + Discourse.find_plugin_js_assets(include_disabled: true).each do |file| + next if file.ends_with?("_extra") # these are still handled by sprockets + assets << "#{file}.js" end assets diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 011a7423f02..84306edbce9 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -715,12 +715,6 @@ class Plugin::Instance handlebars_includes.each { |hb| contents << "require_asset('#{hb}')" } javascript_includes.each { |js| contents << "require_asset('#{js}')" } - if !EmberCli.plugin_assets? - each_globbed_asset do |f, is_dir| - contents << (is_dir ? "depend_on('#{f}')" : "require_asset('#{f}')") - end - end - if !contents.present? [js_file_path, extra_js_file_path].each do |f| File.delete(f) @@ -732,13 +726,10 @@ class Plugin::Instance contents.insert(0, "<%") contents << "%>" - write_path = EmberCli.plugin_assets? ? extra_js_file_path : js_file_path - delete_path = EmberCli.plugin_assets? ? js_file_path : extra_js_file_path - - Discourse::Utils.atomic_write_file(write_path, contents.join("\n")) + Discourse::Utils.atomic_write_file(extra_js_file_path, contents.join("\n")) begin - File.delete(delete_path) + File.delete(js_file_path) rescue Errno::ENOENT end end @@ -850,25 +841,17 @@ class Plugin::Instance end def js_asset_exists? - if EmberCli.plugin_assets? - # If assets/javascripts exists, ember-cli will output a .js file - File.exist?("#{File.dirname(@path)}/assets/javascripts") - else - File.exist?(js_file_path) - end + # If assets/javascripts exists, ember-cli will output a .js file + File.exist?("#{File.dirname(@path)}/assets/javascripts") end def extra_js_asset_exists? - EmberCli.plugin_assets? && File.exist?(extra_js_file_path) + File.exist?(extra_js_file_path) end def admin_js_asset_exists? - if EmberCli.plugin_assets? - # If this directory exists, ember-cli will output a .js file - File.exist?("#{File.dirname(@path)}/admin/assets/javascripts") - else - false - end + # If this directory exists, ember-cli will output a .js file + File.exist?("#{File.dirname(@path)}/admin/assets/javascripts") end # Receives an array with two elements: diff --git a/lib/tasks/qunit.rake b/lib/tasks/qunit.rake index 98a99993586..ebccadf102e 100644 --- a/lib/tasks/qunit.rake +++ b/lib/tasks/qunit.rake @@ -85,7 +85,7 @@ task "qunit:test", [:timeout, :qunit_path] do |_, args| # wait for server to accept connections require 'net/http' - warmup_path = "/assets/discourse/tests/active-plugins.js" + warmup_path = "/srv/status" uri = URI("http://localhost:#{unicorn_port}/#{warmup_path}") puts "Warming up Rails server"