From 455cc03b138b2db93c63eb91fba8f5ca3fdaef51 Mon Sep 17 00:00:00 2001 From: Sam Van Campenhout Date: Mon, 15 Nov 2021 09:49:33 +0100 Subject: [PATCH] WIP still need tests --- README.md | 3 +- packages/compat/src/options.ts | 1 + packages/compat/src/resolver-transform.ts | 12 +++- packages/compat/src/resolver.ts | 81 +++++++++++++++++++++-- packages/compat/tests/audit.test.ts | 7 +- packages/core/src/app-files.ts | 8 +++ packages/core/src/app.ts | 3 + packages/core/src/options.ts | 19 ++++-- packages/router/ember-cli-build.js | 3 +- tests/scenarios/static-app-test.ts | 1 + 10 files changed, 125 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 4a1c473bb4..2d67242c91 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,7 @@ return require('@embroider/compat').compatBuild(app, Webpack, { // staticAddonTestSupportTrees: true, // staticAddonTrees: true, // staticHelpers: true, + // staticModifiers: true, // staticComponents: true, // splitAtRoutes: ['route.name'], // can also be a RegExp // packagerOptions: { @@ -95,7 +96,7 @@ 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. 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 test. This is usually safe because addons get invoke declarative in templates and we can see all invocations. +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. diff --git a/packages/compat/src/options.ts b/packages/compat/src/options.ts index 98c8486f9c..ce8a6b1f99 100644 --- a/packages/compat/src/options.ts +++ b/packages/compat/src/options.ts @@ -115,6 +115,7 @@ export const recommendedOptions: { [name: string]: Options } = Object.freeze({ staticAddonTrees: true, staticAddonTestSupportTrees: true, staticHelpers: true, + staticModifiers: true, staticComponents: true, allowUnsafeDynamicComponents: false, }), diff --git a/packages/compat/src/resolver-transform.ts b/packages/compat/src/resolver-transform.ts index f0e7c0daae..f9f1a65305 100644 --- a/packages/compat/src/resolver-transform.ts +++ b/packages/compat/src/resolver-transform.ts @@ -1,7 +1,7 @@ import { default as Resolver, ComponentResolution, ComponentLocator } from './resolver'; import type { ASTv1 } from '@glimmer/syntax'; -// This is the AST transform that resolves components and helpers at build time +// This is the AST transform that resolves components, helpers and modifiers at build time // and puts them into `dependencies`. export function makeResolverTransform(resolver: Resolver) { function resolverTransform({ filename }: { filename: string }) { @@ -111,6 +111,16 @@ export function makeResolverTransform(resolver: Resolver) { } } }, + ElementModifierStatement(node: ASTv1.ElementModifierStatement) { + if (node.path.type !== 'PathExpression') { + return; + } + if (node.path.this === true) { + return; + } + + resolver.resolveElementModifierStatement(node.path.original, filename, node.path.loc); + }, ElementNode: { enter(node: ASTv1.ElementNode) { if (!scopeStack.inScope(node.tag.split('.')[0])) { diff --git a/packages/compat/src/resolver.ts b/packages/compat/src/resolver.ts index 9b50d82680..06cfb730c6 100644 --- a/packages/compat/src/resolver.ts +++ b/packages/compat/src/resolver.ts @@ -39,7 +39,12 @@ export interface HelperResolution { modules: ResolvedDep[]; } -export type ResolutionResult = ComponentResolution | HelperResolution; +export interface ModifierResolution { + type: 'modifier'; + modules: ResolvedDep[]; +} + +export type ResolutionResult = ComponentResolution | HelperResolution | ModifierResolution; export interface ResolutionFail { type: 'error'; @@ -104,14 +109,20 @@ const builtInHelpers = [ const builtInComponents = ['input', 'link-to', 'textarea']; +const builtInModifiers = ['action', 'on']; + // this is a subset of the full Options. We care about serializability, and we // only needs parts that are easily serializable, which is why we don't keep the // whole thing. -type ResolverOptions = Pick, 'staticHelpers' | 'staticComponents' | 'allowUnsafeDynamicComponents'>; +type ResolverOptions = Pick< + Required, + 'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'allowUnsafeDynamicComponents' +>; function extractOptions(options: Required | ResolverOptions): ResolverOptions { return { staticHelpers: options.staticHelpers, + staticModifiers: options.staticModifiers, staticComponents: options.staticComponents, allowUnsafeDynamicComponents: options.allowUnsafeDynamicComponents, }; @@ -334,13 +345,13 @@ export default class CompatResolver implements Resolver { astTransformer(templateCompiler: TemplateCompiler): unknown { this.templateCompiler = templateCompiler; - if (this.staticComponentsEnabled || this.staticHelpersEnabled) { + if (this.staticComponentsEnabled || this.staticHelpersEnabled || this.staticModifiersEnabled) { return makeResolverTransform(this); } } - // called by our audit tool. Forces staticComponents and staticHelpers to - // activate so we can audit their behavior, while making their errors silent + // called by our audit tool. Forces staticComponents, staticHelpers and staticModifiers + // to activate so we can audit their behavior, while making their errors silent // until we can gather them up at the end of the build for the audit results. enableAuditMode() { this.auditMode = true; @@ -436,6 +447,10 @@ export default class CompatResolver implements Resolver { return this.params.options.staticHelpers || this.auditMode; } + private get staticModifiersEnabled(): boolean { + return this.params.options.staticModifiers || this.auditMode; + } + private tryHelper(path: string, from: string): Resolution | null { let parts = path.split('@'); if (parts.length > 1 && parts[0].length > 0) { @@ -470,6 +485,40 @@ export default class CompatResolver implements Resolver { return null; } + private tryModifier(path: string, from: string): Resolution | null { + let parts = path.split('@'); + if (parts.length > 1 && parts[0].length > 0) { + let cache = PackageCache.shared('embroider-stage3'); + let packageName = parts[0]; + let renamed = this.adjustImportsOptions.renamePackages[packageName]; + if (renamed) { + packageName = renamed; + } + return this._tryModifier(parts[1], from, cache.resolve(packageName, cache.ownerOfFile(from)!)); + } else { + return this._tryModifier(path, from, this.appPackage); + } + } + + private _tryModifier(path: string, from: string, targetPackage: Package | AppPackagePlaceholder): Resolution | null { + for (let extension of this.adjustImportsOptions.resolvableExtensions) { + let absPath = join(targetPackage.root, 'modifiers', path) + extension; + if (pathExistsSync(absPath)) { + return { + type: 'modifier', + modules: [ + { + runtimeName: this.absPathToRuntimeName(absPath, targetPackage), + path: explicitRelative(dirname(from), absPath), + absPath, + }, + ], + }; + } + } + return null; + } + @Memoize() private get appPackage(): AppPackagePlaceholder { return { root: this.params.root, name: this.params.modulePrefix }; @@ -651,6 +700,28 @@ export default class CompatResolver implements Resolver { } } + resolveElementModifierStatement(path: string, from: string, loc: Loc): Resolution | null { + if (!this.staticModifiersEnabled) { + return null; + } + let found = this.tryModifier(path, from); + if (found) { + return this.add(found, from); + } + if (builtInModifiers.includes(path)) { + return null; + } + return this.add( + { + type: 'error', + message: `Missing modifier`, + detail: path, + loc, + }, + from + ); + } + resolveElement(tagName: string, from: string, loc: Loc): Resolution | null { if (!this.staticComponentsEnabled) { return null; diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts index 1275b8e633..fe174ed41d 100644 --- a/packages/compat/tests/audit.test.ts +++ b/packages/compat/tests/audit.test.ts @@ -30,7 +30,12 @@ describe('audit', function () { resolver: new CompatResolver({ root: app.baseDir, modulePrefix: 'audit-this-app', - options: { staticComponents: false, staticHelpers: false, allowUnsafeDynamicComponents: false }, + options: { + staticComponents: false, + staticHelpers: false, + staticModifiers: false, + allowUnsafeDynamicComponents: false, + }, activePackageRules: [], adjustImportsOptions: { renamePackages: {}, diff --git a/packages/core/src/app-files.ts b/packages/core/src/app-files.ts index 230bc69777..3cd13c9ad7 100644 --- a/packages/core/src/app-files.ts +++ b/packages/core/src/app-files.ts @@ -13,6 +13,7 @@ export class AppFiles { readonly tests: ReadonlyArray; readonly components: ReadonlyArray; readonly helpers: ReadonlyArray; + readonly modifiers: ReadonlyArray; private perRoute: RouteFiles; readonly otherAppFiles: ReadonlyArray; readonly relocatedFiles: Map; @@ -22,6 +23,7 @@ export class AppFiles { let tests: string[] = []; let components: string[] = []; let helpers: string[] = []; + let modifiers: string[] = []; let otherAppFiles: string[] = []; this.perRoute = { children: new Map() }; for (let relativePath of appDiffer.files.keys()) { @@ -64,6 +66,11 @@ export class AppFiles { continue; } + if (relativePath.startsWith('modifiers/')) { + modifiers.push(relativePath); + continue; + } + if ( this.handleClassicRouteFile(relativePath) || (podModulePrefix !== undefined && this.handlePodsRouteFile(relativePath, podModulePrefix)) @@ -76,6 +83,7 @@ export class AppFiles { this.tests = tests; this.components = components; this.helpers = helpers; + this.modifiers = modifiers; this.otherAppFiles = otherAppFiles; let relocatedFiles: Map = new Map(); diff --git a/packages/core/src/app.ts b/packages/core/src/app.ts index 63791a7bf1..46c65cec93 100644 --- a/packages/core/src/app.ts +++ b/packages/core/src/app.ts @@ -1144,6 +1144,9 @@ export class AppBuilder { if (!this.options.staticHelpers) { requiredAppFiles.push(appFiles.helpers); } + if (!this.options.staticModifiers) { + requiredAppFiles.push(appFiles.modifiers); + } let styles = []; // only import styles from engines with a parent (this excludeds the parent application) as their styles diff --git a/packages/core/src/options.ts b/packages/core/src/options.ts index 168413dfad..5a7734c8e3 100644 --- a/packages/core/src/options.ts +++ b/packages/core/src/options.ts @@ -9,6 +9,16 @@ export default interface Options { // Enabling this is a prerequisite for route splitting. 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. + 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). @@ -26,7 +36,7 @@ export default interface Options { splitAtRoutes?: (RegExp | string)[]; // Every file within your application's `app` directory is categorized as a - // component, helper, route, route template, controller, or "other". + // component, helper, modifier, route, route template, controller, or "other". // // This option lets you decide which "other" files should be loaded // statically. By default, all "other" files will be included in the build and @@ -44,9 +54,9 @@ export default interface Options { // means that everything under your-project/app/lib will be loaded statically. // // This option has no effect on components (which are governed by - // staticComponents), helpers (which are governed by staticHelpers), or the - // route-specific files (routes, route templates, and controllers which are - // governed by splitAtRoutes). + // staticComponents), helpers (which are governed by staticHelpers), modifiers + // (which are governed by staticModifiers) or the route-specific files (routes, + // route templates, and controllers which are governed by splitAtRoutes). staticAppPaths?: string[]; // By default, all modules that get imported into the app go through Babel, so @@ -100,6 +110,7 @@ export default interface Options { export function optionsWithDefaults(options?: Options): Required { let defaults = { staticHelpers: false, + staticModifiers: false, staticComponents: false, packageRules: [], splitAtRoutes: [], diff --git a/packages/router/ember-cli-build.js b/packages/router/ember-cli-build.js index bd376eb1dc..e8f2676e73 100644 --- a/packages/router/ember-cli-build.js +++ b/packages/router/ember-cli-build.js @@ -2,7 +2,7 @@ const EmberAddon = require('ember-cli/lib/broccoli/ember-addon'); -module.exports = function(defaults) { +module.exports = function (defaults) { let app = new EmberAddon(defaults, { // Add options here }); @@ -17,6 +17,7 @@ module.exports = function(defaults) { staticAddonTrees: true, staticComponents: true, staticHelpers: true, + staticModifiers: true, splitRouteClasses: true, splitAtRoutes: ['split-me'], }); diff --git a/tests/scenarios/static-app-test.ts b/tests/scenarios/static-app-test.ts index 50f89e0446..7d4f60a88f 100644 --- a/tests/scenarios/static-app-test.ts +++ b/tests/scenarios/static-app-test.ts @@ -281,6 +281,7 @@ appScenarios staticAddonTrees: true, staticComponents: true, staticHelpers: true, + staticModifiers: true, packageRules: [ { package: 'app-template',