diff --git a/app/assets/javascripts/discourse-common/addon/lib/loader-shim.js b/app/assets/javascripts/discourse-common/addon/lib/loader-shim.js new file mode 100644 index 00000000000..6fe66c42deb --- /dev/null +++ b/app/assets/javascripts/discourse-common/addon/lib/loader-shim.js @@ -0,0 +1,58 @@ +// Webpack has bugs, using globalThis is the safest +// https://github.com/embroider-build/embroider/issues/1545 +let { define: __define__, require: __require__ } = globalThis; + +// Traditionally, Ember compiled ES modules into AMD modules, which are then +// made usable in the browser at runtime via loader.js. In a classic build, all +// the modules, including any external ember-auto-imported dependencies, are +// added to the loader.js registry and therefore require()-able at runtime. +// +// Overtime, the AMD-ness of the modules, the ability to define arbitrarily +// named modules and the ability to require any modules and even enumerate the +// known modules at runtime (require.entries/_eak_seen) became heavily relied +// upon, which is problematic. For one thing, these features don't align well +// with ES modules semantics, and it is also impossible to perform tree-shaking +// as the presence of a particular module could end up being important even if +// it appears to be unused in the static analysis. +// +// For Discourse, the AMD/loader.js mechanism is an important glue. It is what +// allows Discourse core/admin/wizard/plugins to all be separate .js bundlers +// and be "glued back together" as full module graph in the browser. +// +// For instance, a plugin module can `import Post from "discourse/models/post"; +// because the babel plugin compiled discourse/models/post.js into an AMD +// module into app.js (`define("discourse/models/post", ...)`), which makes +// it available in the runtime loader.js registry, and the plugin module itself +// is also compiled into AMD with a dependency on the core module. +// +// This has similar drawbacks as the general problem in the ecosystem, but in +// addition, it has a particular bad side-effect that any external dependencies +// (NPM packages) we use in core will automatically become a defacto public API +// for plugins to use as well, making it difficult for core to upgrade/remove +// dependencies (and thus so as introducing them in the first place). +// +// Ember is aggressively moving away from AMD modules and there are active RFCs +// to explore the path to deprecating AMD/loader.js. While it would still be +// fine (in the medium term at least) for us to use AMD/loader.js as an interop +// mechanism between our bundles, we will have to be more conscious about what +// to make available to plugins via this mechanism. +// +// In the meantime Embroider no longer automatically add AMD shims for external +// dependencies. In order to preserve compatibility for plugins, this utility +// allows us to manually force a particular module to be included in loader.js +// and available to plugins. Overtime we should review this list and start +// deprecating any accidental leakages. +// +// The general way to use it is: +// +// import { importSync } from "@embroider/macros"; +// +// loaderShim("some-npm-pkg", () => importSync("some-npm-pkg")); +// +// Note that `importSync` is a macro which must be passed a string +// literal, therefore cannot be abstracted away. +export default function loaderShim(pkg, callback) { + if (!__require__.has(pkg)) { + __define__(pkg, callback); + } +} diff --git a/app/assets/javascripts/discourse/app/app.js b/app/assets/javascripts/discourse/app/app.js index 8b8b2ae09cb..b781dd8d9f6 100644 --- a/app/assets/javascripts/discourse/app/app.js +++ b/app/assets/javascripts/discourse/app/app.js @@ -1,4 +1,5 @@ import "./global-compat"; +import "./loader-shims"; import require from "require"; import Application from "@ember/application"; diff --git a/app/assets/javascripts/discourse/app/loader-shims.js b/app/assets/javascripts/discourse/app/loader-shims.js new file mode 100644 index 00000000000..92cf12d4f97 --- /dev/null +++ b/app/assets/javascripts/discourse/app/loader-shims.js @@ -0,0 +1,32 @@ +import { importSync } from "@embroider/macros"; +import loaderShim from "discourse-common/lib/loader-shim"; + +// AMD shims for the app bunndle, see the comment in loader-shim.js +// These effectively become public APIs for plugins, so add/remove them carefully +loaderShim("@discourse/itsatrap", () => importSync("@discourse/itsatrap")); +loaderShim("@ember-compat/tracked-built-ins", () => + importSync("@ember-compat/tracked-built-ins") +); +loaderShim("@popperjs/core", () => importSync("@popperjs/core")); +loaderShim("@uppy/aws-s3", () => importSync("@uppy/aws-s3")); +loaderShim("@uppy/aws-s3-multipart", () => + importSync("@uppy/aws-s3-multipart") +); +loaderShim("@uppy/core", () => importSync("@uppy/core")); +loaderShim("@uppy/drop-target", () => importSync("@uppy/drop-target")); +loaderShim("@uppy/utils/lib/AbortController", () => + importSync("@uppy/utils/lib/AbortController") +); +loaderShim("@uppy/utils/lib/delay", () => importSync("@uppy/utils/lib/delay")); +loaderShim("@uppy/utils/lib/EventTracker", () => + importSync("@uppy/utils/lib/EventTracker") +); +loaderShim("@uppy/xhr-upload", () => importSync("@uppy/xhr-upload")); +loaderShim("a11y-dialog", () => importSync("a11y-dialog")); +loaderShim("ember-modifier", () => importSync("ember-modifier")); +loaderShim("handlebars", () => importSync("handlebars")); +loaderShim("js-yaml", () => importSync("js-yaml")); +loaderShim("message-bus-client", () => importSync("message-bus-client")); +loaderShim("tippy.js", () => importSync("tippy.js")); +loaderShim("virtual-dom", () => importSync("virtual-dom")); +loaderShim("xss", () => importSync("xss")); diff --git a/app/assets/javascripts/discourse/ember-cli-build.js b/app/assets/javascripts/discourse/ember-cli-build.js index 238bdce4a73..d684e9caf95 100644 --- a/app/assets/javascripts/discourse/ember-cli-build.js +++ b/app/assets/javascripts/discourse/ember-cli-build.js @@ -1,7 +1,7 @@ "use strict"; const EmberApp = require("ember-cli/lib/broccoli/ember-app"); -const resolve = require("path").resolve; +const { resolve, join } = require("path"); const mergeTrees = require("broccoli-merge-trees"); const concat = require("broccoli-concat"); const { createI18nTree } = require("./lib/translation-plugin"); @@ -138,6 +138,7 @@ module.exports = function (defaults) { const testHelpers = concat(appTestTrees, { inputFiles: [ + "**/tests/loader-shims.js", "**/tests/test-boot-ember-cli.js", "**/tests/helpers/**/*.js", "**/tests/fixtures/**/*.js", @@ -178,6 +179,10 @@ module.exports = function (defaults) { "/app/assets/javascripts/discourse/public/assets/scripts/module-shims.js" ); + // See: https://github.com/embroider-build/embroider/issues/1574 + // Specifically, markdownItBundleTree is triggering the MacrosConfig error + finalizeEmbroiderMacrosConfigs(app, resolve("."), app.project); + const discoursePluginsTree = app.project .findAddonByName("discourse-plugins") .generatePluginsTree(); @@ -215,3 +220,21 @@ module.exports = function (defaults) { discoursePluginsTree, ]); }; + +// See: https://github.com/embroider-build/embroider/issues/1574 +function finalizeEmbroiderMacrosConfigs(appInstance, appRoot, parent) { + parent.initializeAddons?.(); + + for (let addon of parent.addons) { + if (addon.name === "@embroider/macros") { + const MacrosConfig = require(join( + addon.packageRoot, + "src", + "macros-config" + )).default; + MacrosConfig.for(appInstance, appRoot).finalize(); + } else { + finalizeEmbroiderMacrosConfigs(appInstance, appRoot, addon); + } + } +} diff --git a/app/assets/javascripts/discourse/package.json b/app/assets/javascripts/discourse/package.json index 68e3dc2a137..f11044dce21 100644 --- a/app/assets/javascripts/discourse/package.json +++ b/app/assets/javascripts/discourse/package.json @@ -35,6 +35,7 @@ "@ember/render-modifiers": "^2.1.0", "@ember/string": "^3.1.1", "@ember/test-helpers": "^2.9.4", + "@embroider/macros": "^1.13.1", "@glimmer/component": "^1.1.2", "@glimmer/tracking": "^1.1.2", "@popperjs/core": "^2.11.8", diff --git a/app/assets/javascripts/discourse/tests/loader-shims.js b/app/assets/javascripts/discourse/tests/loader-shims.js new file mode 100644 index 00000000000..8a6d0821b5c --- /dev/null +++ b/app/assets/javascripts/discourse/tests/loader-shims.js @@ -0,0 +1,7 @@ +import { importSync } from "@embroider/macros"; +import loaderShim from "discourse-common/lib/loader-shim"; + +// AMD shims for the test bunndle, see the comment in loader-shim.js +loaderShim("pretender", () => importSync("pretender")); +loaderShim("qunit", () => importSync("qunit")); +loaderShim("sinon", () => importSync("sinon")); diff --git a/app/assets/javascripts/discourse/tests/setup-tests.js b/app/assets/javascripts/discourse/tests/setup-tests.js index a16401e2ac9..45dd3b69e46 100644 --- a/app/assets/javascripts/discourse/tests/setup-tests.js +++ b/app/assets/javascripts/discourse/tests/setup-tests.js @@ -1,3 +1,4 @@ +import "./loader-shims"; import { applyPretender, exists, diff --git a/app/assets/javascripts/pretty-text/addon/loader-shims.js b/app/assets/javascripts/pretty-text/addon/loader-shims.js new file mode 100644 index 00000000000..dcc89a6d2e2 --- /dev/null +++ b/app/assets/javascripts/pretty-text/addon/loader-shims.js @@ -0,0 +1,7 @@ +import { importSync } from "@embroider/macros"; +import loaderShim from "discourse-common/lib/loader-shim"; + +// AMD shims for the addon, see the comment in loader-shim.js +// These effectively become public APIs for plugins, so add/remove them carefully +// Note that this is included into the main app bundle for core +loaderShim("xss", () => importSync("xss")); diff --git a/app/assets/javascripts/pretty-text/addon/pretty-text.js b/app/assets/javascripts/pretty-text/addon/pretty-text.js index f0e93ae1282..525bfe8efcb 100644 --- a/app/assets/javascripts/pretty-text/addon/pretty-text.js +++ b/app/assets/javascripts/pretty-text/addon/pretty-text.js @@ -1,3 +1,4 @@ +import "./loader-shims"; import { cook as cookIt, setup as setupIt, diff --git a/app/assets/javascripts/pretty-text/package.json b/app/assets/javascripts/pretty-text/package.json index 213d0167478..47ab6684cd1 100644 --- a/app/assets/javascripts/pretty-text/package.json +++ b/app/assets/javascripts/pretty-text/package.json @@ -14,6 +14,7 @@ "start": "ember serve" }, "dependencies": { + "@embroider/macros": "^1.13.1", "discourse-common": "1.0.0", "ember-auto-import": "^2.6.3", "ember-cli-babel": "^7.26.11", diff --git a/app/assets/javascripts/yarn.lock b/app/assets/javascripts/yarn.lock index 1e3a12ef5c3..d4b27c4e6fa 100644 --- a/app/assets/javascripts/yarn.lock +++ b/app/assets/javascripts/yarn.lock @@ -1244,6 +1244,20 @@ resolve "^1.20.0" semver "^7.3.2" +"@embroider/macros@^1.13.1": + version "1.13.1" + resolved "https://registry.yarnpkg.com/@embroider/macros/-/macros-1.13.1.tgz#aee17e5af0e0086bd36873bdb4e49ea346bab3fa" + integrity sha512-4htraP/rNIht8uCxXoc59Bw2EsBFfc4YUQD9XSpzJ4xUr1V0GQf9wL/noeSuYSxIhwRfZOErnJhsdyf1hH+I/A== + dependencies: + "@embroider/shared-internals" "2.4.0" + assert-never "^1.2.1" + babel-import-util "^2.0.0" + ember-cli-babel "^7.26.6" + find-up "^5.0.0" + lodash "^4.17.21" + resolve "^1.20.0" + semver "^7.3.2" + "@embroider/shared-internals@2.2.0", "@embroider/shared-internals@^2.0.0", "@embroider/shared-internals@^2.1.0": version "2.2.0" resolved "https://registry.yarnpkg.com/@embroider/shared-internals/-/shared-internals-2.2.0.tgz#7c4f5cf8f7289b36ffbfc04f6b4c71876713c869" @@ -1258,6 +1272,21 @@ semver "^7.3.5" typescript-memoize "^1.0.1" +"@embroider/shared-internals@2.4.0": + version "2.4.0" + resolved "https://registry.yarnpkg.com/@embroider/shared-internals/-/shared-internals-2.4.0.tgz#0e9fdb0b2df9bad45fab8c54cbb70d8a2cbf01fc" + integrity sha512-pFE05ebenWMC9XAPRjadYCXXb6VmqjkhYN5uqkhPo+VUmMHnx7sZYYxqGjxfVuhC/ghS/BNlOffOCXDOoE7k7g== + dependencies: + babel-import-util "^2.0.0" + debug "^4.3.2" + ember-rfc176-data "^0.3.17" + fs-extra "^9.1.0" + js-string-escape "^1.0.1" + lodash "^4.17.21" + resolve-package-path "^4.0.1" + semver "^7.3.5" + typescript-memoize "^1.0.1" + "@embroider/test-setup@^3.0.1": version "3.0.1" resolved "https://registry.yarnpkg.com/@embroider/test-setup/-/test-setup-3.0.1.tgz#603b21a809708ac928fe9f002905ee3711dc6864" @@ -2278,6 +2307,11 @@ babel-import-util@^1.1.0, babel-import-util@^1.2.2, babel-import-util@^1.3.0, ba resolved "https://registry.yarnpkg.com/babel-import-util/-/babel-import-util-1.4.1.tgz#1df6fd679845df45494bac9ca12461d49497fdd4" integrity sha512-TNdiTQdPhXlx02pzG//UyVPSKE7SNWjY0n4So/ZnjQpWwaM5LvWBLkWa1JKll5u06HNscHD91XZPuwrMg1kadQ== +babel-import-util@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/babel-import-util/-/babel-import-util-2.0.0.tgz#99a2e7424bcde01898bc61bb19700ff4c74379a3" + integrity sha512-pkWynbLwru0RZmA9iKeQL63+CkkW0RCP3kL5njCtudd6YPUKb5Pa0kL4fb3bmuKn2QDBFwY5mvvhEK/+jv2Ynw== + babel-loader@^8.0.6: version "8.3.0" resolved "https://registry.yarnpkg.com/babel-loader/-/babel-loader-8.3.0.tgz#124936e841ba4fe8176786d6ff28add1f134d6a8" diff --git a/lib/pretty_text/vendor-shims.js b/lib/pretty_text/vendor-shims.js index cd0518870f3..1fe6efcf48f 100644 --- a/lib/pretty_text/vendor-shims.js +++ b/lib/pretty_text/vendor-shims.js @@ -1,3 +1,21 @@ +define("@embroider/macros", ["exports", "require"], function ( + __require__, + __exports__ +) { + __exports__.importSync = __require__; +}); + +define("discourse-common/lib/loader-shim", ["exports", "require"], function ( + __exports__, + __require__ +) { + __exports__.default = (id, callback) => { + if (!__require__.has(id)) { + define(id, callback); + } + }; +}); + define("xss", ["exports"], function (__exports__) { __exports__.default = window.filterXSS; });