diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 38e1e947c65..03d00a0f29b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,7 +17,7 @@ permissions: jobs: build: - name: ${{ matrix.target }} ${{ matrix.build_type }} + name: ${{ matrix.target }} ${{ matrix.build_type }} ${{ ((matrix.ember_cli_plugin_assets == '1') && '(ember-cli-compiled plugin js)') || ''}} runs-on: ubuntu-latest container: discourse/discourse_test:slim${{ startsWith(matrix.build_type, 'frontend') && '-browsers' || '' }} timeout-minutes: 60 @@ -29,6 +29,7 @@ jobs: PGUSER: discourse PGPASSWORD: discourse USES_PARALLEL_DATABASES: ${{ matrix.build_type == 'backend' && matrix.target == 'core' }} + EMBER_CLI_PLUGIN_ASSETS: ${{ matrix.ember_cli_plugin_assets }} strategy: fail-fast: false @@ -36,11 +37,17 @@ 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/discourse-hbr/raw-handlebars-compiler.js b/app/assets/javascripts/discourse-hbr/raw-handlebars-compiler.js index f6376ca5dba..06dce159431 100644 --- a/app/assets/javascripts/discourse-hbr/raw-handlebars-compiler.js +++ b/app/assets/javascripts/discourse-hbr/raw-handlebars-compiler.js @@ -134,7 +134,19 @@ TemplateCompiler.prototype.initializeFeatures = function initializeFeatures() {}; TemplateCompiler.prototype.processString = function (string, relativePath) { - let filename = relativePath.replace(/^templates\//, "").replace(/\.hbr$/, ""); + let filename; + + const pluginName = relativePath.match(/^discourse\/plugins\/([^\/]+)\//)?.[1]; + + if (pluginName) { + filename = relativePath + .replace(`discourse/plugins/${pluginName}/`, "") + .replace(/^(discourse\/)?templates\//, "javascripts/"); + } else { + filename = relativePath.replace(/^templates\//, ""); + } + + filename = filename.replace(/\.hbr$/, ""); return ( 'import { template as compiler } from "discourse-common/lib/raw-handlebars";\n' + diff --git a/app/assets/javascripts/discourse-plugins/index.js b/app/assets/javascripts/discourse-plugins/index.js new file mode 100644 index 00000000000..8e6742b87a1 --- /dev/null +++ b/app/assets/javascripts/discourse-plugins/index.js @@ -0,0 +1,120 @@ +"use strict"; + +const path = require("path"); +const WatchedDir = require("broccoli-source").WatchedDir; +const Funnel = require("broccoli-funnel"); +const mergeTrees = require("broccoli-merge-trees"); +const fs = require("fs"); +const concat = require("broccoli-concat"); +const RawHandlebarsCompiler = require("discourse-hbr/raw-handlebars-compiler"); + +function fixLegacyExtensions(tree) { + return new Funnel(tree, { + getDestinationPath: function (relativePath) { + if (relativePath.endsWith(".es6")) { + return relativePath.slice(0, -4); + } else if (relativePath.endsWith(".raw.hbs")) { + return relativePath.replace(".raw.hbs", ".hbr"); + } + return relativePath; + }, + }); +} + +const COLOCATED_CONNECTOR_REGEX = + /^(?.*)\/connectors\/(?[^\/]+)\/(?[^\/\.]+)\.(?.+)$/; + +// Having connector templates and js in the same directory causes a clash +// when outputting es6 modules. This shim separates colocated connectors +// into separate js / template locations. +function unColocateConnectors(tree) { + return new Funnel(tree, { + getDestinationPath: function (relativePath) { + const match = relativePath.match(COLOCATED_CONNECTOR_REGEX); + if ( + match && + match.groups.extension === "hbs" && + !match.groups.prefix.endsWith("/templates") + ) { + const { prefix, outlet, name } = match.groups; + return `${prefix}/templates/connectors/${outlet}/${name}.hbs`; + } + if ( + match && + match.groups.extension === "js" && + match.groups.prefix.endsWith("/templates") + ) { + // Some plugins are colocating connector JS under `/templates` + const { prefix, outlet, name } = match.groups; + const newPrefix = prefix.slice(0, -"/templates".length); + return `${newPrefix}/connectors/${outlet}/${name}.js`; + } + return relativePath; + }, + }); +} + +function namespaceModules(tree, pluginDirectoryName) { + return new Funnel(tree, { + getDestinationPath: function (relativePath) { + return `discourse/plugins/${pluginDirectoryName}/${relativePath}`; + }, + }); +} + +module.exports = { + name: require("./package").name, + + pluginInfos() { + const root = path.resolve("../../../../plugins"); + const pluginDirectories = fs + .readdirSync(root, { withFileTypes: true }) + .filter( + (dirent) => + (dirent.isDirectory() || dirent.isSymbolicLink()) && + !dirent.name.startsWith(".") + ); + + return pluginDirectories.map((directory) => { + const name = directory.name; + const jsDirectory = path.resolve(root, name, "assets/javascripts"); + const hasJs = fs.existsSync(jsDirectory); + return { name, jsDirectory, hasJs }; + }); + }, + + generatePluginsTree() { + const trees = this.pluginInfos() + .filter((p) => p.hasJs) + .map(({ name, jsDirectory }) => { + let tree = new WatchedDir(jsDirectory); + + tree = fixLegacyExtensions(tree); + tree = unColocateConnectors(tree); + tree = namespaceModules(tree, name); + + tree = RawHandlebarsCompiler(tree); + tree = this.compileTemplates(tree); + + tree = this.processedAddonJsFiles(tree); + + return concat(mergeTrees([tree]), { + inputFiles: ["**/*.js"], + outputFile: `assets/plugins/${name}.js`, + }); + }); + return mergeTrees(trees); + }, + + shouldCompileTemplates() { + // The base Addon implementation checks for template + // files in the addon directories. We need to override that + // check so that the template compiler always runs. + return true; + }, + + treeFor() { + // This addon doesn't contribute any 'real' trees to the app + return; + }, +}; diff --git a/app/assets/javascripts/discourse-plugins/package.json b/app/assets/javascripts/discourse-plugins/package.json new file mode 100644 index 00000000000..3bb0f9ed325 --- /dev/null +++ b/app/assets/javascripts/discourse-plugins/package.json @@ -0,0 +1,25 @@ +{ + "name": "discourse-plugins", + "version": "1.0.0", + "description": "An addon providing a broccoli tree for each Discourse plugin", + "author": "Discourse", + "license": "GPL-2.0-only", + "keywords": [ + "ember-addon" + ], + "repository": "", + "dependencies": { + "ember-auto-import": "^2.4.2", + "ember-cli-babel": "^7.26.10", + "ember-cli-htmlbars": "^6.1.0", + "discourse-widget-hbs": "1.0" + }, + "engines": { + "node": "16.* || >= 18", + "npm": "please-use-yarn", + "yarn": ">= 1.21.1" + }, + "ember": { + "edition": "default" + } +} diff --git a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js index abc6bc4ac6e..bcee944c053 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-connectors.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-connectors.js @@ -45,7 +45,12 @@ function findClass(outletName, uniqueName) { if (!_classPaths) { _classPaths = {}; findOutlets(require._eak_seen, (outlet, res, un) => { - _classPaths[`${outlet}/${un}`] = requirejs(res).default; + const possibleConnectorClass = requirejs(res).default; + if (possibleConnectorClass.__id) { + // This is the template, not the connector class + return; + } + _classPaths[`${outlet}/${un}`] = possibleConnectorClass; }); } diff --git a/app/assets/javascripts/discourse/app/lib/source-identifier.js b/app/assets/javascripts/discourse/app/lib/source-identifier.js index 555c1cac8c0..de0fbd7e373 100644 --- a/app/assets/javascripts/discourse/app/lib/source-identifier.js +++ b/app/assets/javascripts/discourse/app/lib/source-identifier.js @@ -30,10 +30,10 @@ export default function identifySource(error) { let plugin; - if (isDevelopment()) { - // Source-mapped: - plugin = plugin || error.stack.match(/plugins\/([\w-]+)\//)?.[1]; + // Source-mapped: + plugin = plugin || error.stack.match(/plugins\/([\w-]+)\//)?.[1]; + if (isDevelopment()) { // Un-source-mapped: plugin = plugin || error.stack.match(/assets\/plugins\/([\w-]+)\.js/)?.[1]; } diff --git a/app/assets/javascripts/discourse/ember-cli-build.js b/app/assets/javascripts/discourse/ember-cli-build.js index 15cc37a4ae0..f6a59caac3c 100644 --- a/app/assets/javascripts/discourse/ember-cli-build.js +++ b/app/assets/javascripts/discourse/ember-cli-build.js @@ -14,6 +14,7 @@ const SILENCED_WARN_PREFIXES = [ "Setting the `jquery-integration` optional feature flag", "The Ember Classic edition has been deprecated", "Setting the `template-only-glimmer-components` optional feature flag to `false`", + "DEPRECATION: Invoking the `` component with positional arguments is deprecated", ]; module.exports = function (defaults) { @@ -29,6 +30,16 @@ module.exports = function (defaults) { } }; + // Silence warnings which go straight to console.warn (e.g. template compiler deprecations) + /* eslint-disable no-console */ + const oldConsoleWarn = console.warn.bind(console); + console.warn = (message, ...args) => { + if (!SILENCED_WARN_PREFIXES.some((prefix) => message.startsWith(prefix))) { + return oldConsoleWarn(message, ...args); + } + }; + /* eslint-enable no-console */ + const isProduction = EmberApp.env().includes("production"); const isTest = EmberApp.env().includes("test"); @@ -134,6 +145,16 @@ module.exports = function (defaults) { "/app/assets/javascripts/discourse/public/assets/scripts/module-shims.js" ); + let discoursePluginsTree; + if (process.env.EMBER_CLI_PLUGIN_ASSETS === "1") { + discoursePluginsTree = app.project + .findAddonByName("discourse-plugins") + .generatePluginsTree(); + } else { + // Empty tree - no-op + discoursePluginsTree = mergeTrees([]); + } + const terserPlugin = app.project.findAddonByName("ember-cli-terser"); const applyTerser = (tree) => terserPlugin.postprocessTree("all", tree); @@ -164,5 +185,6 @@ module.exports = function (defaults) { inputFiles: [`discourse-boot.js`], }), generateScriptsTree(app), + applyTerser(discoursePluginsTree), ]); }; diff --git a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js index 50ebf5daa97..0eb9ce0b3de 100644 --- a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js +++ b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js @@ -5,7 +5,8 @@ const fetch = require("node-fetch"); const { encode } = require("html-entities"); const cleanBaseURL = require("clean-base-url"); const path = require("path"); -const { promises: fs } = require("fs"); +const fs = require("fs"); +const fsPromises = fs.promises; const { JSDOM } = require("jsdom"); const { shouldLoadPluginTestJs } = require("discourse/lib/plugin-js"); const { Buffer } = require("node:buffer"); @@ -231,7 +232,7 @@ async function applyBootstrap(bootstrap, template, response, baseURL, preload) { async function buildFromBootstrap(proxy, baseURL, req, response, preload) { try { - const template = await fs.readFile( + const template = await fsPromises.readFile( path.join(cwd(), "dist", "index.html"), "utf8" ); @@ -378,10 +379,31 @@ module.exports = { contentFor(type, config) { if (shouldLoadPluginTestJs() && type === "test-plugin-js") { - return ` - - - `; + const scripts = []; + + if (process.env.EMBER_CLI_PLUGIN_ASSETS === "1") { + const pluginInfos = this.app.project + .findAddonByName("discourse-plugins") + .pluginInfos(); + + for (const { name, hasJs } of pluginInfos) { + if (hasJs) { + scripts.push(`plugins/${name}.js`); + } + + if (fs.existsSync(`../plugins/${name}_extras.js.erb`)) { + scripts.push(`plugins/${name}_extras.js`); + } + } + } else { + scripts.push("discourse/tests/active-plugins.js"); + } + + scripts.push("admin-plugins.js"); + + return scripts + .map((s) => ``) + .join("\n"); } else if (shouldLoadPluginTestJs() && type === "test-plugin-tests-js") { return ``; } diff --git a/app/assets/javascripts/discourse/package.json b/app/assets/javascripts/discourse/package.json index 00b7b4c9fa6..ec34e659cdd 100644 --- a/app/assets/javascripts/discourse/package.json +++ b/app/assets/javascripts/discourse/package.json @@ -33,6 +33,7 @@ "@uppy/utils": "^4.1.0", "@uppy/xhr-upload": "^2.1.2", "admin": "^1.0.0", + "discourse-plugins": "^1.0.0", "bootstrap": "3.4.1", "broccoli-asset-rev": "^3.0.0", "deepmerge": "^4.2.2", diff --git a/app/assets/javascripts/discourse/public/assets/scripts/discourse-boot.js b/app/assets/javascripts/discourse/public/assets/scripts/discourse-boot.js index 7dab11a480b..b3f56a6dcc5 100644 --- a/app/assets/javascripts/discourse/public/assets/scripts/discourse-boot.js +++ b/app/assets/javascripts/discourse/public/assets/scripts/discourse-boot.js @@ -4,15 +4,34 @@ } // TODO: Remove this and have resolver find the templates - const prefix = "discourse/templates/"; + const discoursePrefix = "discourse/templates/"; const adminPrefix = "admin/templates/"; const wizardPrefix = "wizard/templates/"; - let len = prefix.length; + const discoursePrefixLength = discoursePrefix.length; + + const pluginRegex = /^discourse\/plugins\/([^\/]+)\//; + Object.keys(requirejs.entries).forEach(function (key) { - if (key.startsWith(prefix)) { - Ember.TEMPLATES[key.slice(len)] = require(key).default; + let templateKey; + let pluginName; + if (key.startsWith(discoursePrefix)) { + templateKey = key.slice(discoursePrefixLength); } else if (key.startsWith(adminPrefix) || key.startsWith(wizardPrefix)) { - Ember.TEMPLATES[key] = require(key).default; + templateKey = key; + } else if ( + (pluginName = key.match(pluginRegex)?.[1]) && + key.includes("/templates/") && + require(key).default.__id // really is a template + ) { + // This logic mimics the old sprockets compilation system which used to + // output templates directly to `Ember.TEMPLATES` with this naming logic + templateKey = key.slice(`discourse/plugins/${pluginName}/`.length); + templateKey = templateKey.replace("discourse/templates/", ""); + templateKey = `javascripts/${templateKey}`; + } + + if (templateKey) { + Ember.TEMPLATES[templateKey] = require(key).default; } }); diff --git a/app/assets/javascripts/discourse/testem.js b/app/assets/javascripts/discourse/testem.js index 8da35eb44e9..d49180f5c48 100644 --- a/app/assets/javascripts/discourse/testem.js +++ b/app/assets/javascripts/discourse/testem.js @@ -95,6 +95,9 @@ if (process.argv.includes("-t")) { } else if (shouldLoadPluginTestJs()) { // Running with ember cli, but we want to pass through plugin request to Rails module.exports.proxies = { + "/assets/plugins/*_extra.js": { + target, + }, "/assets/discourse/tests/active-plugins.js": { target, }, diff --git a/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js b/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js index 3ada4484ab8..c11fbcfeb57 100644 --- a/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js +++ b/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js @@ -9,7 +9,7 @@ setEnvironment("testing"); document.addEventListener("discourse-booted", () => { const script = document.getElementById("plugin-test-script"); - if (script && !requirejs.entries["discourse/tests/active-plugins"]) { + if (script && !requirejs.entries["discourse/tests/plugin-tests"]) { throw new Error( `Plugin JS payload failed to load from ${script.src}. Is the Rails server running?` ); diff --git a/app/assets/javascripts/package.json b/app/assets/javascripts/package.json index 43154f4212c..d5719f80010 100644 --- a/app/assets/javascripts/package.json +++ b/app/assets/javascripts/package.json @@ -3,6 +3,7 @@ "workspaces": [ "discourse", "admin", + "discourse-plugins", "discourse-common", "discourse-ensure-deprecation-order", "discourse-hbr", diff --git a/app/views/qunit/theme.html.erb b/app/views/qunit/theme.html.erb index c5e1071b5fa..c6d026d9edc 100644 --- a/app/views/qunit/theme.html.erb +++ b/app/views/qunit/theme.html.erb @@ -8,7 +8,9 @@ <%= preload_script "vendor" %> <%= preload_script "discourse" %> <%= preload_script "admin" %> - <%= preload_script "discourse/tests/active-plugins" %> + <%- 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" %> diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index 504a14de96c..829465d1a30 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -53,7 +53,7 @@ Rails.application.config.assets.precompile += %w{ scripts/discourse-boot } -Rails.application.config.assets.precompile += EmberCli::ASSETS.map { |name| name.sub('.js', '.map') } +Rails.application.config.assets.precompile += EmberCli.assets.map { |name| name.sub('.js', '.map') } # Precompile all available locales unless GlobalSetting.try(:omit_base_locales) diff --git a/lib/discourse.rb b/lib/discourse.rb index 43659294fdc..526f6b8ee35 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -382,12 +382,17 @@ module Discourse def self.find_plugin_js_assets(args) plugins = self.find_plugins(args).select do |plugin| - plugin.js_asset_exists? + plugin.js_asset_exists? || plugin.extra_js_asset_exists? end plugins = apply_asset_filters(plugins, :js, args[:request]) - plugins.map { |plugin| "plugins/#{plugin.directory_name}" } + plugins.flat_map do |plugin| + assets = [] + assets << "plugins/#{plugin.directory_name}" if plugin.js_asset_exists? + assets << "plugins/#{plugin.directory_name}_extra" if plugin.extra_js_asset_exists? + assets + end end def self.assets_digest diff --git a/lib/discourse_sourcemapping_url_processor.rb b/lib/discourse_sourcemapping_url_processor.rb index 51c3ba21b2a..7b6ae82d563 100644 --- a/lib/discourse_sourcemapping_url_processor.rb +++ b/lib/discourse_sourcemapping_url_processor.rb @@ -6,7 +6,7 @@ class DiscourseSourcemappingUrlProcessor < Sprockets::Rails::SourcemappingUrlProcessor def self.sourcemap_asset_path(sourcemap_logical_path, context:) result = super(sourcemap_logical_path, context: context) - if File.basename(sourcemap_logical_path) === sourcemap_logical_path + if (File.basename(sourcemap_logical_path) === sourcemap_logical_path) || sourcemap_logical_path.start_with?("plugins/") # If the original sourcemap reference is relative, keep it relative result = File.basename(result) end diff --git a/lib/ember_cli.rb b/lib/ember_cli.rb index efae6be1697..7135e96a46a 100644 --- a/lib/ember_cli.rb +++ b/lib/ember_cli.rb @@ -1,15 +1,33 @@ # frozen_string_literal: true module EmberCli - ASSETS = %w( - discourse.js - admin.js - wizard.js - ember_jquery.js - pretty-text-bundle.js - start-discourse.js - vendor.js - ) + Dir.glob("app/assets/javascripts/discourse/scripts/*.js").map { |f| File.basename(f) } + def self.plugin_assets? + ENV["EMBER_CLI_PLUGIN_ASSETS"] == "1" + end + + def self.assets + @assets ||= begin + assets = %w( + discourse.js + admin.js + wizard.js + ember_jquery.js + pretty-text-bundle.js + start-discourse.js + vendor.js + ) + 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 + end + + assets + end + end def self.script_chunks return @@chunk_infos if defined? @@chunk_infos @@ -29,6 +47,6 @@ module EmberCli end def self.is_ember_cli_asset?(name) - ASSETS.include?(name) || name.start_with?("chunk.") + assets.include?(name) || name.start_with?("chunk.") end end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 8b3eb2294f7..05d3685b296 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -715,19 +715,31 @@ class Plugin::Instance handlebars_includes.each { |hb| contents << "require_asset('#{hb}')" } javascript_includes.each { |js| contents << "require_asset('#{js}')" } - each_globbed_asset do |f, is_dir| - contents << (is_dir ? "depend_on('#{f}')" : "require_asset('#{f}')") + 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? - contents.insert(0, "<%") - contents << "%>" - Discourse::Utils.atomic_write_file(js_file_path, contents.join("\n")) - else - begin - File.delete(js_file_path) + if !contents.present? + [js_file_path, extra_js_file_path].each do |f| + File.delete(f) rescue Errno::ENOENT end + return + end + + 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")) + + begin + File.delete(delete_path) + rescue Errno::ENOENT end end @@ -838,7 +850,16 @@ class Plugin::Instance end def js_asset_exists? - File.exist?(js_file_path) + 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 + end + + def extra_js_asset_exists? + EmberCli.plugin_assets? && File.exist?(extra_js_file_path) end # Receives an array with two elements: @@ -1080,7 +1101,11 @@ class Plugin::Instance end def js_file_path - @file_path ||= "#{Plugin::Instance.js_path}/#{directory_name}.js.erb" + "#{Plugin::Instance.js_path}/#{directory_name}.js.erb" + end + + def extra_js_file_path + @extra_js_file_path ||= "#{Plugin::Instance.js_path}/#{directory_name}_extra.js.erb" end def register_assets!