Enable Embroider/Webpack code spliting for Wizard (#24919)

(extracted from #23678)

* Move Wizard back into main app, remove Wizard addon
* Remove Wizard-related resolver or build hacks
* Install and enable `@embroider/router`
* Add "wizard" to `splitAtRoutes`

In a fully optimized Embroider app, route-based code splitting more
or less Just Work™ – install `@embroider/router`, subclass from it,
configure which routes you want to split and that's about it.

However, our app is not "fully optimized", by which I mean we are
not able to turn on all the `static*` flags.

In Embroider, "static" means "statically analyzable". Specifically
it means that all inter-dependencies between modules (files) are
explicitly expressed as `import`s, as opposed to `{{i18n ...}}`
magically means "look for the default export in app/helpers/i18n.js"
or something even more dynamic with the resolver.

Without turning on those flags, Embroider behaves conservatively,
slurps up all `app` files eagerly into the primary bundle/chunks.
So, while you _could_ turn on route-based code splitting, there
won't be much to split.

The commits leading up to this involves a bunch of refactors and
cleanups that 1) works perfectly fine in the classic build, 2) are
good and useful in their own right, but also 3) re-arranged things
such that most dependencies are now explicit.

With those in place, I was able to move all the wizard code into
the "app/static" folder. Embroider does not eagerly pull things from
this folder into any bundle, unless something explicitly "asks" for
them via `imports`. Conversely, things from this folder are not
registered with the resolver and are not added to the `loader.js`
registry.

In conjunction with route-based code splitting, we now have the
ability to split out islands of on-demand functionalities from the
main app bundle.

When you split a route in Embroider, it automatically creates a
bundle/entrypoint with the relevant routes/templates/controllers
matching that route prefix. Anything they import will be added to
the bundle as well, assuming they are not already in the main app
bundle, which is where the "app/static" folder comes into play.

The "app/static" folder name is not special. It is configured in
ember-cli-build.js. Alternatively, we could have left everything
in their normal locations, and add more fine-grained paths to the
`staticAppPaths` array. I just thought it would be easy to manage
and scale, and less error-prone to do it this way.

Note that putting things in `app/static` does not guarantee that
it would not be part of the main app bundle. For example, if we
were to add an `import ... from "app/static/wizard/...";` in a
main bundle file (say, `app.js`), then that chunk of the module
graph would be pulled in. (Consider using `await import(...)`?)

Overtime, we can build better tooling (e.g. lint rules and babel
macros to make things less repetitive) as we expand the use of
this pattern, but this is a start.

Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
This commit is contained in:
David Taylor 2023-12-20 13:15:06 +00:00 committed by GitHub
parent 2f40d9b07b
commit cbc28e8e33
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
56 changed files with 53 additions and 296 deletions

View File

@ -8,7 +8,7 @@ function appendToCache(cache, key, value) {
cache.set(key, cachedValue);
}
const NAMESPACES = ["discourse/", "wizard/", "admin/"];
const NAMESPACES = ["discourse/", "admin/"];
function isInRecognisedNamespace(moduleName) {
for (const ns of NAMESPACES) {

View File

@ -16,7 +16,7 @@ let { define: __define__, require: __require__ } = globalThis;
// 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
// allows Discourse core/admin/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";

View File

@ -135,7 +135,6 @@ function lookupModuleBySuffix(suffix) {
"discourse-common/",
"select-kit/",
"admin/",
"wizard/",
"truth-helpers/",
];
Object.keys(requirejs.entries).forEach((name) => {
@ -215,18 +214,12 @@ export function buildResolver(baseName) {
const dashed = dasherize(split[1].replace(/[\.\/]/g, "-"));
const adminBase = `admin/${type}s/`;
const wizardBase = `wizard/${type}s/`;
if (
lookupModuleBySuffix(`${type}s/${dashed}`) ||
requirejs.entries[adminBase + dashed] ||
requirejs.entries[adminBase + dashed.replace(/^admin[-]/, "")] ||
requirejs.entries[
adminBase + dashed.replace(/^admin[-]/, "").replace(/-/g, "_")
] ||
requirejs.entries[wizardBase + dashed] ||
requirejs.entries[wizardBase + dashed.replace(/^wizard[-]/, "")] ||
requirejs.entries[
wizardBase + dashed.replace(/^wizard[-]/, "").replace(/-/g, "_")
]
) {
corrected = type + ":" + dashed;
@ -282,7 +275,6 @@ export function buildResolver(baseName) {
this.findMobileTemplate(parsedName) ||
this.findTemplate(parsedName) ||
this.findAdminTemplate(parsedName) ||
this.findWizardTemplate(parsedName) ||
this.findLoadingTemplate(parsedName) ||
this.findConnectorTemplate(parsedName) ||
this.discourseTemplateModule("not_found")
@ -386,28 +378,5 @@ export function buildResolver(baseName) {
return resolved;
}
findWizardTemplate(parsedName) {
if (parsedName.fullNameWithoutType === "wizard") {
return this.discourseTemplateModule("wizard/templates/wizard");
}
let namespaced;
if (parsedName.fullNameWithoutType.startsWith("components/")) {
// Look up components as-is
namespaced = parsedName.fullNameWithoutType;
} else if (/^wizard[_\.-]/.test(parsedName.fullNameWithoutType)) {
// This may only get hit for the loading routes and may be removable.
namespaced = parsedName.fullNameWithoutType.slice(7);
}
if (namespaced) {
let wizardParsedName = this.parseName(
`template:wizard/templates/${namespaced}`
);
return this.findTemplate(wizardParsedName);
}
}
};
}

View File

@ -1,10 +1,10 @@
import EmberRouter from "@ember/routing/router";
import EmbroiderRouter from "@embroider/router";
import Site from "discourse/models/site";
import { isTesting } from "discourse-common/config/environment";
import getURL from "discourse-common/lib/get-url";
import applyRouterHomepageOverrides from "./lib/homepage-router-overrides";
class BareRouter extends EmberRouter {
class BareRouter extends EmbroiderRouter {
location = isTesting() ? "none" : "history";
setupRouter() {

View File

@ -276,4 +276,8 @@ export default function () {
this.route("invites", { resetNamespace: true }, function () {
this.route("show", { path: "/:token" });
});
this.route("wizard", function () {
this.route("step", { path: "/steps/:step_id" });
});
}

View File

@ -1,6 +1,6 @@
import Route from "@ember/routing/route";
import DisableSidebar from "discourse/mixins/disable-sidebar";
import Wizard from "wizard/models/wizard";
import Wizard from "discourse/static/wizard/models/wizard";
export default class WizardRoute extends Route.extend(DisableSidebar) {
model() {

View File

@ -1,6 +1,6 @@
import RouteTemplate from "ember-route-template";
import hideApplicationFooter from "discourse/helpers/hide-application-footer";
import DiscourseLogo from "../components/discourse-logo";
import DiscourseLogo from "discourse/static/wizard/components/discourse-logo";
export default RouteTemplate(<template>
{{hideApplicationFooter}}

View File

@ -2,9 +2,9 @@ import Component from "@glimmer/component";
import { action } from "@ember/object";
import { inject as service } from "@ember/service";
import RouteTemplate from "ember-route-template";
import WizardCanvas from "discourse/static/wizard/components/wizard-canvas";
import WizardStep from "discourse/static/wizard/components/wizard-step";
import getUrl from "discourse-common/lib/get-url";
import WizardCanvas from "../components/wizard-canvas";
import WizardStep from "../components/wizard-step";
export default RouteTemplate(
class extends Component {

View File

@ -122,8 +122,6 @@ module.exports = function (defaults) {
const adminTree = app.project.findAddonByName("admin").treeForAddonBundle();
const wizardTree = app.project.findAddonByName("wizard").treeForAddonBundle();
const testStylesheetTree = mergeTrees([
discourseScss(`${discourseRoot}/app/assets/stylesheets`, "qunit.scss"),
discourseScss(
@ -147,12 +145,6 @@ module.exports = function (defaults) {
outputFile: `assets/admin.js`,
})
),
applyTerser(
concat(wizardTree, {
inputFiles: ["**/*.js"],
outputFile: `assets/wizard.js`,
})
),
applyTerser(generateScriptsTree(app)),
applyTerser(discoursePluginsTree),
testStylesheetTree,
@ -166,6 +158,7 @@ module.exports = function (defaults) {
.slice(0, 8);
const appTree = compatBuild(app, Webpack, {
splitAtRoutes: ["wizard"],
staticAppPaths: ["static"],
packagerOptions: {
webpackConfig: {
@ -195,7 +188,6 @@ module.exports = function (defaults) {
// TODO: delete special case for jquery when removing app.import() above
((EMBER_MAJOR_VERSION < 4 && request === "jquery") ||
request.startsWith("admin/") ||
request.startsWith("wizard/") ||
request.startsWith("discourse/plugins/") ||
request.startsWith("discourse/theme-"))
) {

View File

@ -41,6 +41,7 @@
"@embroider/compat": "^3.4.2",
"@embroider/core": "^3.4.3",
"@embroider/macros": "^1.13.1",
"@embroider/router": "^2.1.4",
"@embroider/webpack": "^3.2.1",
"@floating-ui/dom": "^1.5.0",
"@glimmer/component": "^1.1.2",
@ -133,7 +134,6 @@
"virtual-dom": "^2.1.1",
"webpack": "^5.89.0",
"webpack-stats-plugin": "^1.1.3",
"wizard": "1.0.0",
"workbox-cacheable-response": "^7.0.0",
"workbox-core": "^7.0.0",
"workbox-expiration": "^7.0.0",

View File

@ -57,7 +57,6 @@
<script src="{{rootURL}}assets/test-i18n.js" data-embroider-ignore></script>
<script src="{{rootURL}}assets/test-site-settings.js" data-embroider-ignore></script>
<script src="{{rootURL}}assets/admin.js" data-embroider-ignore></script>
<script src="{{rootURL}}assets/wizard.js" data-embroider-ignore></script>
<template id="dynamic-test-js">
{{content-for "test-plugin-css"}}

View File

@ -598,96 +598,6 @@ module("Unit | Ember | resolver", function (hooks) {
);
});
test("resolves templates with 'wizard' prefix", function (assert) {
setTemplates([
"wizard/templates/foo",
"discourse/templates/wizard_bar",
"discourse/templates/wizard.bar",
"wizard/templates/bar",
"wizard/templates/dashboard_general",
"discourse/templates/wizard-baz-qux",
"javascripts/wizard/plugin-template",
]);
// Switches prefix to wizard/templates when underscored
lookupTemplate(
assert,
"template:wizard_foo",
"wizard/templates/foo",
"when prefix is separated by underscore"
);
// Switches prefix to wizard/templates when dotted
lookupTemplate(
assert,
"template:wizard.foo",
"wizard/templates/foo",
"when prefix is separated by dot"
);
// Doesn't match unseparated prefix
lookupTemplate(
assert,
"template:wizardfoo",
undefined,
"but not when prefix is not separated in any way"
);
// Prioritized the default match when underscored
lookupTemplate(
assert,
"template:wizard_bar",
"discourse/templates/wizard_bar",
"but not when template with the exact underscored name exists"
);
// Prioritized the default match when dotted
lookupTemplate(
assert,
"template:wizard.bar",
"discourse/templates/wizard.bar",
"but not when template with the exact dotted name exists"
);
lookupTemplate(
assert,
"template:wizard-dashboard-general",
"wizard/templates/dashboard_general",
"finds namespaced and underscored version"
);
lookupTemplate(
assert,
"template:wizard-baz/qux",
"discourse/templates/wizard-baz-qux",
"also tries dasherized"
);
});
test("resolves component templates with 'wizard' prefix to 'wizard/templates/' namespace", function (assert) {
setTemplates([
"wizard/templates/components/foo",
"discourse/templates/components/bar",
"wizard/templates/components/bar",
]);
// Looks for components in wizard/templates
lookupTemplate(
assert,
"template:components/foo",
"wizard/templates/components/foo",
"uses wizard template component when no standard match"
);
// Prioritized non-wizard component
lookupTemplate(
assert,
"template:components/bar",
"discourse/templates/components/bar",
"uses standard match when both exist"
);
});
test("resolves plugin/theme components with and without /index", function (assert) {
registerTemporaryModule(
"discourse/plugins/my-fake-plugin/discourse/components/my-component",

View File

@ -1,6 +1,6 @@
import { setupTest } from "ember-qunit";
import { module, test } from "qunit";
import { Field } from "wizard/models/wizard";
import { Field } from "discourse/static/wizard/models/wizard";
module("Unit | Model | Wizard | wizard-field", function (hooks) {
setupTest(hooks);

View File

@ -21,8 +21,7 @@
"pretty-text",
"select-kit",
"theme-transpiler",
"truth-helpers",
"wizard"
"truth-helpers"
],
"resolutions": {
"**/unset-value": "2.0.1"

View File

@ -21,8 +21,7 @@
"pretty-text",
"select-kit",
"theme-transpiler",
"truth-helpers",
"wizard"
"truth-helpers"
],
"resolutions": {
"**/unset-value": "2.0.1",

View File

@ -1 +0,0 @@
engine-strict = true

View File

@ -1,5 +0,0 @@
export default function () {
this.route("wizard", function () {
this.route("step", { path: "/steps/:step_id" });
});
}

View File

@ -1,20 +0,0 @@
{
"schemaVersion": "1.0.0",
"packages": [
{
"name": "ember-cli",
"version": "5.0.0",
"blueprints": [
{
"name": "addon",
"outputRepo": "https://github.com/ember-cli/ember-addon-output",
"codemodsSource": "ember-addon-codemods-manifest@1",
"isBaseBlueprint": true,
"options": [
"--no-welcome"
]
}
]
}
]
}

View File

@ -1,20 +0,0 @@
"use strict";
const EmberAddon = require("ember-cli/lib/broccoli/ember-addon");
module.exports = function (defaults) {
const app = new EmberAddon(defaults, {
autoImport: {
publicAssetURL: "",
},
});
const { maybeEmbroider } = require("@embroider/test-setup");
return maybeEmbroider(app, {
skipBabel: [
{
package: "qunit",
},
],
});
};

View File

@ -1,31 +0,0 @@
"use strict";
const calculateCacheKeyForTree = require("calculate-cache-key-for-tree");
const path = require("path");
module.exports = {
name: require("./package").name,
// return an empty tree here as we do not want the addon modules to be
// included into vendor.js; instead, we will produce a separate bundle
// (wizard.js) to be included via a script tag as needed
treeForAddon() {
return;
},
// custom method to produce the tree for wizard.js
// called by ember-cli-build.js in discourse core
treeForAddonBundle() {
let addonTreePath = path.resolve(this.root, this.treePaths.addon);
let addonTree = this.treeGenerator(addonTreePath);
return this._super.treeForAddon.call(this, addonTree);
},
cacheKeyForTree(tree) {
return calculateCacheKeyForTree(tree, this);
},
isDevelopingAddon() {
return true;
},
};

View File

@ -1,69 +0,0 @@
{
"name": "wizard",
"version": "1.0.0",
"description": "Discourse's setup wizard",
"author": "Discourse",
"license": "GPL-2.0-only",
"keywords": [
"ember-addon"
],
"scripts": {
"build": "ember build",
"lint:hbs": "ember-template-lint .",
"lint:js": "eslint .",
"start": "ember serve"
},
"dependencies": {
"ember-cli-babel": "^8.2.0",
"ember-cli-htmlbars": "^6.3.0",
"ember-template-imports": "^4.0.0"
},
"devDependencies": {
"@babel/core": "^7.23.6",
"@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@embroider/test-setup": "^3.0.3",
"@glimmer/component": "^1.1.2",
"@types/ember": "^4.0.10",
"@types/ember__application": "^4.0.10",
"@types/ember__array": "^4.0.9",
"@types/ember__component": "^4.0.21",
"@types/ember__controller": "^4.0.11",
"@types/ember__debug": "^4.0.7",
"@types/ember__destroyable": "^4.0.4",
"@types/ember__engine": "^4.0.10",
"@types/ember__error": "^4.0.5",
"@types/ember__object": "^4.0.11",
"@types/ember__polyfills": "^4.0.5",
"@types/ember__routing": "^4.0.19",
"@types/ember__runloop": "^4.0.8",
"@types/ember__service": "^4.0.8",
"@types/ember__string": "^3.0.13",
"@types/ember__template": "^4.0.5",
"@types/ember__test": "^4.0.5",
"@types/ember__utils": "^4.0.6",
"@types/jquery": "^3.5.29",
"@types/qunit": "^2.19.9",
"@types/rsvp": "^4.0.8",
"broccoli-asset-rev": "^3.0.0",
"ember-cli": "~5.0.0",
"ember-cli-inject-live-reload": "^2.1.0",
"ember-cli-sri": "^2.1.1",
"ember-cli-terser": "^4.0.2",
"ember-disable-prototype-extensions": "^1.1.3",
"ember-load-initializers": "^2.1.1",
"ember-resolver": "^10.1.1",
"ember-source": "~3.28.12",
"ember-source-channel-url": "^3.0.0",
"loader.js": "^4.7.0",
"webpack": "^5.89.0"
},
"engines": {
"node": "16.* || >= 18",
"npm": "please-use-yarn",
"yarn": ">= 1.21.1"
},
"ember": {
"edition": "default"
}
}

View File

@ -1201,6 +1201,16 @@
ember-cli-version-checker "^5.1.2"
semver "^7.3.5"
"@ember/test-waiters@^3.0.2":
version "3.1.0"
resolved "https://registry.yarnpkg.com/@ember/test-waiters/-/test-waiters-3.1.0.tgz#61399919cbf793978da0b8bfdfdb7bca0cb80e9e"
integrity sha512-bb9h95ktG2wKY9+ja1sdsFBdOms2lB19VWs8wmNpzgHv1NCetonBoV5jHBV4DHt0uS1tg9z66cZqhUVlYs96KQ==
dependencies:
calculate-cache-key-for-tree "^2.0.0"
ember-cli-babel "^7.26.6"
ember-cli-version-checker "^5.1.2"
semver "^7.3.5"
"@embroider/addon-shim@^1.0.0", "@embroider/addon-shim@^1.8.3", "@embroider/addon-shim@^1.8.4", "@embroider/addon-shim@^1.8.7":
version "1.8.7"
resolved "https://registry.yarnpkg.com/@embroider/addon-shim/-/addon-shim-1.8.7.tgz#ba2dcb0647ed2cb0c500c835326266b89ceec595"
@ -1314,6 +1324,14 @@
resolve "^1.20.0"
semver "^7.3.2"
"@embroider/router@^2.1.4":
version "2.1.6"
resolved "https://registry.yarnpkg.com/@embroider/router/-/router-2.1.6.tgz#c706794075ecc214fa456ce0131e8be3aab15733"
integrity sha512-geEJF0iKpGQ4YcP2oc30BGPgxer7Umeeg/C07m6xe2OYltQxIzSNWm//A7F/ZrwhVwSSai1W7YyFw8r5wS3OsA==
dependencies:
"@ember/test-waiters" "^3.0.2"
"@embroider/addon-shim" "^1.8.7"
"@embroider/shared-internals@2.5.1", "@embroider/shared-internals@^2.0.0", "@embroider/shared-internals@^2.5.1":
version "2.5.1"
resolved "https://registry.yarnpkg.com/@embroider/shared-internals/-/shared-internals-2.5.1.tgz#a4d8c057cbff293ef6eb29ee6537f263d206b444"

View File

@ -1201,6 +1201,16 @@
ember-cli-version-checker "^5.1.2"
semver "^7.3.5"
"@ember/test-waiters@^3.0.2":
version "3.1.0"
resolved "https://registry.yarnpkg.com/@ember/test-waiters/-/test-waiters-3.1.0.tgz#61399919cbf793978da0b8bfdfdb7bca0cb80e9e"
integrity sha512-bb9h95ktG2wKY9+ja1sdsFBdOms2lB19VWs8wmNpzgHv1NCetonBoV5jHBV4DHt0uS1tg9z66cZqhUVlYs96KQ==
dependencies:
calculate-cache-key-for-tree "^2.0.0"
ember-cli-babel "^7.26.6"
ember-cli-version-checker "^5.1.2"
semver "^7.3.5"
"@embroider/addon-shim@^1.0.0", "@embroider/addon-shim@^1.8.3", "@embroider/addon-shim@^1.8.4", "@embroider/addon-shim@^1.8.7":
version "1.8.7"
resolved "https://registry.yarnpkg.com/@embroider/addon-shim/-/addon-shim-1.8.7.tgz#ba2dcb0647ed2cb0c500c835326266b89ceec595"
@ -1314,6 +1324,14 @@
resolve "^1.20.0"
semver "^7.3.2"
"@embroider/router@^2.1.4":
version "2.1.6"
resolved "https://registry.yarnpkg.com/@embroider/router/-/router-2.1.6.tgz#c706794075ecc214fa456ce0131e8be3aab15733"
integrity sha512-geEJF0iKpGQ4YcP2oc30BGPgxer7Umeeg/C07m6xe2OYltQxIzSNWm//A7F/ZrwhVwSSai1W7YyFw8r5wS3OsA==
dependencies:
"@ember/test-waiters" "^3.0.2"
"@embroider/addon-shim" "^1.8.7"
"@embroider/shared-internals@2.5.1", "@embroider/shared-internals@^2.0.0", "@embroider/shared-internals@^2.5.1":
version "2.5.1"
resolved "https://registry.yarnpkg.com/@embroider/shared-internals/-/shared-internals-2.5.1.tgz#a4d8c057cbff293ef6eb29ee6537f263d206b444"

View File

@ -18,9 +18,6 @@
"admin/*": [
"./app/assets/javascripts/admin/addon/*"
],
"wizard/*": [
"./app/assets/javascripts/wizard/addon/*"
],
"pretty-text/*": [
"./app/assets/javascripts/pretty-text/addon/*"
],
@ -79,7 +76,6 @@
"./app/assets/javascripts/discourse/tests",
"./app/assets/javascripts/discourse-common/addon",
"./app/assets/javascripts/admin/addon",
"./app/assets/javascripts/wizard/addon",
"./app/assets/javascripts/pretty-text/addon",
"./app/assets/javascripts/select-kit/addon",
"./app/assets/javascripts/float-kit/addon",

View File

@ -50,7 +50,6 @@ RSpec.describe "Coding style" do
{
"discourse" => "app/assets/javascripts/discourse/app/templates/components",
"admin" => "app/assets/javascripts/admin/addon/templates/components",
"wizard" => "app/assets/javascripts/wizard/addon/templates/components",
"chat/discourse" => "plugins/chat/assets/javascripts/discourse/templates/components",
"chat/admin" => "plugins/chat/assets/javascripts/admin/templates/components",
"styleguide" => "plugins/styleguide/assets/javascripts/discourse/templates/components",