From 119e753ced4f9d548fe22ec533c98d245ad470a8 Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Sat, 14 Dec 2024 10:13:26 +0000 Subject: [PATCH] use staticInvoakables instead of staticHelpers, staticModifiers, and staticComponents --- README.md | 9 +-- packages/compat/src/compat-app-builder.ts | 48 +++++++++--- packages/compat/src/options.ts | 4 +- packages/compat/src/resolver-transform.ts | 22 +++--- packages/compat/src/template-tag-codemod.ts | 4 +- packages/compat/tests/audit.test.ts | 4 +- packages/core/src/options.ts | 84 ++++++++++++++------- 7 files changed, 109 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index a3f404a64..06db49eb6 100644 --- a/README.md +++ b/README.md @@ -82,9 +82,7 @@ You can pass options into Embroider by passing them into the `compatBuild` funct return require('@embroider/compat').compatBuild(app, Webpack, { // staticAddonTestSupportTrees: true, // staticAddonTrees: true, - // staticHelpers: true, - // staticModifiers: true, - // staticComponents: true, + // staticInvokables: true, // staticEmberSource: true, // splitAtRoutes: ['route.name'], // can also be a RegExp // packagerOptions: { @@ -99,9 +97,8 @@ The recommended steps when introducing Embroider into an existing app are: 1. First make it work with no options. This is the mode that supports maximum backward compatibility. If you're hitting errors, first look at the "Compatibility with Classic Builds" section below. 2. Enable `staticAddonTestSupportTrees` and `staticAddonTrees` and test your application. This is usually safe, because most code in these trees gets consumed via `import` statements that we can analyze. But you might find exceptional cases where some code is doing a more dynamic thing. -3. Enable `staticHelpers` and `staticModifiers` and test. This is usually safe because addon helpers and modifiers get invoked declaratively in templates and we can see all invocations. -4. Enable `staticComponents`, and work to eliminate any resulting build warnings about dynamic component invocation. You may need to add `packageRules` that declare where invocations like `{{component someComponent}}` are getting `someComponent` from. -5. Once your app is working with all of the above, you can enable `splitAtRoutes` and add the `@embroider/router` and code splitting should work. See the packages/router/README.md for details and limitations. +3. Enable `staticInvokables` and work to eliminate any resulting build warnings about dynamic component invocation. You may need to add `packageRules` that declare where invocations like `{{component someComponent}}` are getting `someComponent` from. +4. Once your app is working with all of the above, you can enable `splitAtRoutes` and add the `@embroider/router` and code splitting should work. See the packages/router/README.md for details and limitations. ## Configuring asset URLs diff --git a/packages/compat/src/compat-app-builder.ts b/packages/compat/src/compat-app-builder.ts index 7589c4628..8efa972e0 100644 --- a/packages/compat/src/compat-app-builder.ts +++ b/packages/compat/src/compat-app-builder.ts @@ -87,6 +87,35 @@ export class CompatAppBuilder { } } + private get staticHelpers() { + if (this.options.staticHelpers !== undefined) { + // TODO it doesn't seem like we have any real deprecation functionality in this package yet. + // do we need it? + console.error(`Setting 'staticHelpers' is deprecated. Use 'staticInvokables' instead`); + return this.options.staticHelpers; + } else { + return this.options.staticInvokables; + } + } + + private get staticModifiers() { + if (this.options.staticModifiers !== undefined) { + console.error(`Setting 'staticModifiers' is deprecated. Use 'staticInvokables' instead`); + return this.options.staticModifiers; + } else { + return this.options.staticInvokables; + } + } + + private get staticComponents() { + if (this.options.staticComponents !== undefined) { + console.error(`Setting 'staticComponents' is deprecated. Use 'staticInvokables' instead`); + return this.options.staticComponents; + } else { + return this.options.staticInvokables; + } + } + private extractAssets(treePaths: OutputPaths): Asset[] { let assets: Asset[] = []; @@ -269,9 +298,9 @@ export class CompatAppBuilder { } let options: CompatResolverOptions['options'] = { - staticHelpers: this.options.staticHelpers, - staticModifiers: this.options.staticModifiers, - staticComponents: this.options.staticComponents, + staticHelpers: this.staticHelpers, + staticModifiers: this.staticModifiers, + staticComponents: this.staticComponents, allowUnsafeDynamicComponents: this.options.allowUnsafeDynamicComponents, }; @@ -1021,12 +1050,7 @@ export class CompatAppBuilder { transforms.push(macroPlugin as any); } - if ( - this.options.staticComponents || - this.options.staticHelpers || - this.options.staticModifiers || - (globalThis as any).embroider_audit - ) { + if (this.staticComponents || this.staticHelpers || this.staticModifiers || (globalThis as any).embroider_audit) { let opts: ResolverTransformOptions = { appRoot: resolverConfig.appRoot, emberVersion: this.emberVersion(), @@ -1211,13 +1235,13 @@ export class CompatAppBuilder { let eagerModules: string[] = []; let requiredAppFiles = [this.requiredOtherFiles(appFiles)]; - if (!this.options.staticComponents) { + if (!this.staticComponents) { requiredAppFiles.push(appFiles.components); } - if (!this.options.staticHelpers) { + if (!this.staticHelpers) { requiredAppFiles.push(appFiles.helpers); } - if (!this.options.staticModifiers) { + if (!this.staticModifiers) { requiredAppFiles.push(appFiles.modifiers); } diff --git a/packages/compat/src/options.ts b/packages/compat/src/options.ts index dabc89786..a5a7bf660 100644 --- a/packages/compat/src/options.ts +++ b/packages/compat/src/options.ts @@ -121,9 +121,7 @@ export const recommendedOptions: { [name: string]: Options } = Object.freeze({ optimized: Object.freeze({ staticAddonTrees: true, staticAddonTestSupportTrees: true, - staticHelpers: true, - staticModifiers: true, - staticComponents: true, + staticInvokables: true, staticEmberSource: true, allowUnsafeDynamicComponents: false, }), diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts index 4cf4fad1a..1bca22d55 100644 --- a/packages/compat/src/resolver-transform.ts +++ b/packages/compat/src/resolver-transform.ts @@ -32,7 +32,7 @@ type Env = WithJSUtils & { // whole thing. type UserConfig = Pick< Required, - 'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'allowUnsafeDynamicComponents' + 'staticInvokables' | 'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'allowUnsafeDynamicComponents' >; export interface CompatResolverOptions extends CoreResolverOptions { @@ -324,15 +324,15 @@ class TemplateResolver implements ASTPlugin { } private get staticComponentsEnabled(): boolean { - return this.config.options.staticComponents || Boolean(this.auditHandler); + return (this.config.options.staticInvokables ?? this.config.options.staticComponents) || Boolean(this.auditHandler); } private get staticHelpersEnabled(): boolean { - return this.config.options.staticHelpers || Boolean(this.auditHandler); + return (this.config.options.staticInvokables ?? this.config.options.staticHelpers) || Boolean(this.auditHandler); } private get staticModifiersEnabled(): boolean { - return this.config.options.staticModifiers || Boolean(this.auditHandler); + return (this.config.options.staticInvokables ?? this.config.options.staticModifiers) || Boolean(this.auditHandler); } private isIgnoredComponent(dasherizedName: string) { @@ -549,17 +549,17 @@ class TemplateResolver implements ASTPlugin { 2. Have a mustache statement like: `{{something}}`, where `something` is: - a. Not a variable in scope (for example, there's no preceeding line + a. Not a variable in scope (for example, there's no preceeding line like ``) b. Does not start with `@` because that must be an argument from outside this template. - c. Does not contain a dot, like `some.thing` (because that case is classically + c. Does not contain a dot, like `some.thing` (because that case is classically never a global component resolution that we would need to handle) - d. Does not start with `this` (this rule is mostly redundant with the previous rule, + d. Does not start with `this` (this rule is mostly redundant with the previous rule, but even a standalone `this` is never a component invocation). - e. Does not have any arguments. If there are argument like `{{something a=b}}`, - there is still ambiguity between helper vs component, but there is no longer + e. Does not have any arguments. If there are argument like `{{something a=b}}`, + there is still ambiguity between helper vs component, but there is no longer the possibility that this was just rendering some data. - f. Does not take a block, like `{{#something}}{{/something}}` (because that is + f. Does not take a block, like `{{#something}}{{/something}}` (because that is always a component, no ambiguity.) We can't tell if this problematic case is really: @@ -573,7 +573,7 @@ class TemplateResolver implements ASTPlugin { 2. A component invocation, which you could have written `` instead. Angle-bracket invocation has been available and easy-to-adopt - for a very long time. + for a very long time. 3. Property-this-fallback for `{{this.something}}`. Property-this-fallback is eliminated at Ember 4.0, so people have been heavily pushed to get diff --git a/packages/compat/src/template-tag-codemod.ts b/packages/compat/src/template-tag-codemod.ts index 8305a258f..4fad95d10 100644 --- a/packages/compat/src/template-tag-codemod.ts +++ b/packages/compat/src/template-tag-codemod.ts @@ -42,9 +42,7 @@ export default function templateTagCodemod( compatBuild(emberApp, undefined, { staticAddonTrees: true, staticAddonTestSupportTrees: true, - staticComponents: true, - staticHelpers: true, - staticModifiers: true, + staticInvokables: true, staticEmberSource: true, amdCompatibility: { es: [], diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts index 4efa75db9..bee44ac5b 100644 --- a/packages/compat/tests/audit.test.ts +++ b/packages/compat/tests/audit.test.ts @@ -34,9 +34,7 @@ describe('audit', function () { appRoot: app.baseDir, modulePrefix: 'audit-this-app', options: { - staticComponents: true, - staticHelpers: true, - staticModifiers: true, + staticInvokables: true, allowUnsafeDynamicComponents: false, }, activePackageRules: [], diff --git a/packages/core/src/options.ts b/packages/core/src/options.ts index b89b5672f..43ebebe98 100644 --- a/packages/core/src/options.ts +++ b/packages/core/src/options.ts @@ -1,34 +1,62 @@ export default interface Options { - // When true, we statically resolve all template helpers at build time. This - // causes unused helpers to be left out of the build ("tree shaking" of - // helpers). - // - // Defaults to false, which gives you greater compatibility with classic Ember - // apps at the cost of bigger builds. - // - // Enabling this is a prerequisite for route splitting. + /** + * When true, we statically resolve all template helpers at build time. This + * causes unused helpers to be left out of the build ("tree shaking" of + * helpers). + * + * Defaults to false, which gives you greater compatibility with classic Ember + * apps at the cost of bigger builds. + * + * Enabling this is a prerequisite for route splitting. + * + * @deprecated use staticInvokables instead + */ staticHelpers?: boolean; - // When true, we statically resolve all modifiers at build time. This - // causes unused modifiers to be left out of the build ("tree shaking" of - // modifiers). - // - // Defaults to false, which gives you greater compatibility with classic Ember - // apps at the cost of bigger builds. - // - // Enabling this is a prerequisite for route splitting. + /** + * When true, we statically resolve all modifiers at build time. This + * causes unused modifiers to be left out of the build ("tree shaking" of + * modifiers). + * + * Defaults to false, which gives you greater compatibility with classic Ember + * apps at the cost of bigger builds. + * + * Enabling this is a prerequisite for route splitting. + * + * @deprecated use staticInvokables instead + */ staticModifiers?: boolean; - // When true, we statically resolve all components at build time. This causes - // unused components to be left out of the build ("tree shaking" of - // components). - // - // Defaults to false, which gives you greater compatibility with classic Ember - // apps at the cost of bigger builds. - // - // Enabling this is a prerequisite for route splitting. + /** + * When true, we statically resolve all components at build time. This causes + * unused components to be left out of the build ("tree shaking" of + * components). + * + * Defaults to false, which gives you greater compatibility with classic Ember + * apps at the cost of bigger builds. + * + * Enabling this is a prerequisite for route splitting. + * + * @deprecated use staticInvokables instead + */ staticComponents?: boolean; + /** + * When true, we statically resolve all components, modifiers, and helpers (collectively + * knows as Invokables) at build time. This causes any unused Invokables to be left out + * of the build if they are unused i.e. "tree shaking". + * + * Defaults to false which gives you greater compatibility with classic Ember apps at the + * cost of bigger builds. + * + * This setting takes over from `staticHelpers`, `staticModifiers`, and `staticComponents` + * because the Developer Experience was less than ideal if any of these settings did not + * agree i.e. they all needed to be true or they all needed to be false. + * + * Enabling this is a prerequisite for route splitting. + */ + staticInvokables?: boolean; + // Enables per-route code splitting. Any route names that match these patterns // will be split out of the initial app payload. If you use this, you must // also add @embroider/router to your app. See [@embroider/router's @@ -124,11 +152,11 @@ export default interface Options { }; } -export function optionsWithDefaults(options?: Options): Required { +export function optionsWithDefaults( + options?: Options +): Required> { let defaults = { - staticHelpers: false, - staticModifiers: false, - staticComponents: false, + staticInvokables: false, splitAtRoutes: [], staticAppPaths: [], skipBabel: [],