Add staticModifiers option

This option makes it possible to tree-shake unused modifiers and is the modifier equivalent of staticHelpers and staticComponents.

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.
 

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,
   }),

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,30 @@ export function makeResolverTransform(resolver: Resolver) {
             }
           }
         },
+        ElementModifierStatement(node: ASTv1.ElementModifierStatement) {
+          if (node.path.type !== 'PathExpression') {
+            return;
+          }
+          if (scopeStack.inScope(node.path.parts[0])) {
+            return;
+          }
+          if (node.path.this === true) {
+            return;
+          }
+          if (node.path.data === true) {
+            return;
+          }
+          if (node.path.parts.length > 1) {
+            // paths with a dot in them (which therefore split into more than
+            // one "part") are classically understood by ember to be contextual
+            // components. With the introduction of `Template strict mode` in Ember 3.25
+            // it is also possible to pass modifiers this way which means there's nothing
+            // to resolve at this location.
+            return;
+          }
+
+          resolver.resolveElementModifierStatement(node.path.original, filename, node.path.loc);
+        },
         ElementNode: {
           enter(node: ASTv1.ElementNode) {
             if (!scopeStack.inScope(node.tag.split('.')[0])) {

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<Required<Options>, 'staticHelpers' | 'staticComponents' | 'allowUnsafeDynamicComponents'>;
+type ResolverOptions = Pick<
+  Required<Options>,
+  'staticHelpers' | 'staticModifiers' | 'staticComponents' | 'allowUnsafeDynamicComponents'
+>;
 
 function extractOptions(options: Required<Options> | 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;

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: {},

packages/compat/tests/resolver.test.ts

@@ -745,6 +745,59 @@ describe('compat-resolver', function () {
       },
     ]);
   });
+  test('missing modifier', function () {
+    let findDependencies = configure({ staticModifiers: true });
+    expect(() => {
+      findDependencies('templates/application.hbs', `<canvas {{fancy-drawing}}></canvas>`);
+    }).toThrow(new RegExp(`Missing modifier: fancy-drawing in templates/application.hbs`));
+  });
+  test('emits no modifiers when staticModifiers is off', function () {
+    let findDependencies = configure({ staticModifiers: false });
+    givenFile('modifiers/auto-focus.js');
+    expect(findDependencies('templates/application.hbs', `<input {{auto-focus}} />`)).toEqual([]);
+  });
+  test('modifier on html element', function () {
+    let findDependencies = configure({ staticModifiers: true });
+    givenFile('modifiers/auto-focus.js');
+    expect(findDependencies('templates/application.hbs', `<input {{auto-focus}} />`)).toEqual([
+      {
+        runtimeName: 'the-app/modifiers/auto-focus',
+        path: '../modifiers/auto-focus.js',
+      },
+    ]);
+  });
+  test('modifier on component', function () {
+    let findDependencies = configure({ staticModifiers: true });
+    givenFile('modifiers/auto-focus.js');
+    expect(findDependencies('templates/application.hbs', `<StyledInput {{auto-focus}} />`)).toEqual([
+      {
+        runtimeName: 'the-app/modifiers/auto-focus',
+        path: '../modifiers/auto-focus.js',
+      },
+    ]);
+  });
+  test('modifier on contextual component', function () {
+    let findDependencies = configure({ staticModifiers: true });
+    givenFile('modifiers/auto-focus.js');
+    expect(findDependencies('templates/application.hbs', `<Form as |f|> <f.Input {{auto-focus}} /></Form>`)).toEqual([
+      {
+        runtimeName: 'the-app/modifiers/auto-focus',
+        path: '../modifiers/auto-focus.js',
+      },
+    ]);
+  });
+  test('modifier provided as an argument', function () {
+    let findDependencies = configure({ staticModifiers: true });
+    givenFile('modifiers/auto-focus.js');
+    expect(findDependencies('components/test.hbs', `<input {{@auto-focus}} />`)).toEqual([]);
+  });
+  test('contextual modifier', function () {
+    let findDependencies = configure({ staticModifiers: true });
+    givenFile('modifiers/auto-focus.js');
+    expect(findDependencies('templates/application.hbs', `<Form as |f|> <input {{f.auto-focus}} /></Form>`)).toEqual(
+      []
+    );
+  });
   test('local binding takes precedence over helper in bare mustache', function () {
     let findDependencies = configure({ staticHelpers: true });
     givenFile('helpers/capitalize.js');
@@ -759,6 +812,16 @@ describe('compat-resolver', function () {
       findDependencies('templates/application.hbs', `{{#each things as |TheThing|}} <TheThing /> {{/each}}`)
     ).toEqual([]);
   });
+  test('local binding takes precedence over modifier', function () {
+    let findDependencies = configure({ staticModifiers: true });
+    givenFile('modifiers/some-modifier.js');
+    expect(
+      findDependencies(
+        'templates/application.hbs',
+        `{{#each modifiers as |some-modifier|}} <div {{some-modifier}}></div> {{/each}}`
+      )
+    ).toEqual([]);
+  });
   test('angle components can establish local bindings', function () {
     let findDependencies = configure({ staticHelpers: true });
     givenFile('helpers/capitalize.js');
@@ -767,18 +830,26 @@ describe('compat-resolver', function () {
     );
   });
   test('local binding only applies within block', function () {
-    let findDependencies = configure({ staticHelpers: true });
+    let findDependencies = configure({ staticHelpers: true, staticModifiers: true });
     givenFile('helpers/capitalize.js');
+    givenFile('modifiers/validate.js');
     expect(
       findDependencies(
         'templates/application.hbs',
-        `{{#each things as |capitalize|}} {{capitalize}} {{/each}} {{capitalize}}`
+        `
+        {{#each things as |capitalize|}} {{capitalize}} {{/each}} {{capitalize}}
+        <Form as |validate|><input {{validate}} /></Form> <input {{validate}} />
+        `
       )
     ).toEqual([
       {
         runtimeName: 'the-app/helpers/capitalize',
         path: '../helpers/capitalize.js',
       },
+      {
+        runtimeName: 'the-app/modifiers/validate',
+        path: '../modifiers/validate.js',
+      },
     ]);
   });
   test('ignores builtins', function () {
@@ -792,10 +863,12 @@ describe('compat-resolver', function () {
         {{#with (hash submit=(action doit)) as |thing| }}
         {{/with}}
         <LinkTo @route="index"/>
+        <form {{on "submit" doit}}></form>
       `
       )
     ).toEqual([]);
   });
+
   test('ignores dot-rule curly component invocation, inline', function () {
     let findDependencies = configure({ staticHelpers: true, staticComponents: true });
     expect(