WIP

still need tests

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,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])) {

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/core/src/app-files.ts

@@ -13,6 +13,7 @@ export class AppFiles {
   readonly tests: ReadonlyArray<string>;
   readonly components: ReadonlyArray<string>;
   readonly helpers: ReadonlyArray<string>;
+  readonly modifiers: ReadonlyArray<string>;
   private perRoute: RouteFiles;
   readonly otherAppFiles: ReadonlyArray<string>;
   readonly relocatedFiles: Map<string, string>;
@@ -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<string, string> = new Map();

packages/core/src/app.ts

@@ -1144,6 +1144,9 @@ export class AppBuilder<TreeNames> {
     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

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<Options> {
   let defaults = {
     staticHelpers: false,
+    staticModifiers: false,
     staticComponents: false,
     packageRules: [],
     splitAtRoutes: [],

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'],
   });

tests/scenarios/static-app-test.ts

@@ -281,6 +281,7 @@ appScenarios
             staticAddonTrees: true,
             staticComponents: true,
             staticHelpers: true,
+            staticModifiers: true,
             packageRules: [
               {
                 package: 'app-template',