Merge pull request #1372 from embroider-build/restore-component-rules

restore component invokes rules support

packages/compat/src/audit.ts

@@ -67,12 +67,11 @@ interface InternalModule {
     isCJS: boolean;
     isAMD: boolean;
     dependencies: string[];
+    transpiledContent: string | Buffer;
   };
 
   resolved?: Map<string, string | ResolutionFailure>;
 
-  content?: string | Buffer;
-
   linked?: {
     exports: Set<string>;
   };
@@ -140,7 +139,7 @@ export class AuditResults {
             }))
           : [],
         exports: module.linked?.exports ? [...module.linked.exports] : [],
-        content: module.content ? module.content.toString() : '',
+        content: module.parsed?.transpiledContent ? module.parsed?.transpiledContent.toString() : '',
       };
       results.modules[explicitRelative(baseDir, filename)] = publicModule;
     }
@@ -332,7 +331,6 @@ export class Audit {
       } else {
         module.parsed = visitResult;
         module.resolved = await this.resolveDeps(visitResult.dependencies, filename);
-        module.content = content;
       }
     }
   }
@@ -479,6 +477,7 @@ export class Audit {
       isCJS: false,
       isAMD: false,
       dependencies,
+      transpiledContent: content,
     };
   }
 
@@ -504,6 +503,7 @@ export class Audit {
         isCJS: result.isCJS,
         isAMD: result.isAMD,
         dependencies: result.imports.map(i => i.source),
+        transpiledContent: result.transpiledContent,
       };
     } catch (err) {
       if (['BABEL_PARSE_ERROR', 'BABEL_TRANSFORM_ERROR'].includes(err.code)) {

packages/compat/src/audit/babel-visitor.ts

@@ -41,10 +41,10 @@ export function auditJS(rawSource: string, filename: string, babelConfig: Transf
   let sawDefine: boolean = false;
   /* eslint-enable @typescript-eslint/no-inferrable-types */
 
-  let ast = transformSync(rawSource, Object.assign({ filename: filename }, babelConfig))!.ast!;
+  let { ast, code } = transformSync(rawSource, Object.assign({ filename: filename }, babelConfig))!;
   let saveCodeFrame = frames.forSource(rawSource);
 
-  traverse(ast, {
+  traverse(ast!, {
     Identifier(path: NodePath<t.Identifier>) {
       if (path.node.name === 'module' && isFreeVariable(path)) {
         sawModule = true;
@@ -154,7 +154,7 @@ export function auditJS(rawSource: string, filename: string, babelConfig: Transf
 
   let isCJS = imports.length === 0 && exports.size === 0 && (sawModule || sawExports);
   let isAMD = imports.length === 0 && exports.size === 0 && sawDefine;
-  return { imports, exports, isCJS, isAMD, problems };
+  return { imports, exports, isCJS, isAMD, problems, transpiledContent: code! };
 }
 
 export class CodeFrameStorage {

packages/compat/src/babel-plugin-adjust-imports.ts

@@ -5,9 +5,9 @@ import type { types as t } from '@babel/core';
 import { ImportUtil } from 'babel-import-util';
 import { readJSONSync } from 'fs-extra';
 import { CompatResolverOptions } from './resolver-transform';
-import { Resolver } from '@embroider/core';
+import { Package, packageName, Resolver, unrelativize } from '@embroider/core';
 import { snippetToDasherizedName } from './dasherize-component-name';
-import { ModuleRules, TemplateRules } from './dependency-rules';
+import { ActivePackageRules, ComponentRules, ModuleRules, TemplateRules } from './dependency-rules';
 
 export type Options = { appRoot: string };
 
@@ -16,15 +16,23 @@ interface State {
 }
 
 type BabelTypes = typeof t;
+
+interface ExtraImports {
+  [key: string]: {
+    dependsOnComponents?: string[]; // these are already standardized in dasherized form
+    dependsOnModules?: string[];
+  };
+}
+
 type InternalConfig = {
   resolverOptions: CompatResolverOptions;
   resolver: Resolver;
-  extraImports: {
-    [absPath: string]: {
-      dependsOnComponents?: string[]; // these are already standardized in dasherized form
-      dependsOnModules?: string[];
-    };
-  };
+
+  // rule-based extra dependencies, indexed by filename
+  extraImports: ExtraImports;
+
+  // rule-based extra dependencies, indexed by classical component name
+  componentExtraImports: ExtraImports;
 };
 
 export default function main(babel: typeof Babel) {
@@ -39,6 +47,7 @@ export default function main(babel: typeof Babel) {
       resolverOptions,
       resolver: new Resolver(resolverOptions),
       extraImports: preprocessExtraImports(resolverOptions),
+      componentExtraImports: preprocessComponentExtraImports(resolverOptions),
     };
     return cached;
   }
@@ -61,35 +70,57 @@ export default function main(babel: typeof Babel) {
 function addExtraImports(t: BabelTypes, path: NodePath<t.Program>, config: InternalConfig) {
   let filename: string = path.hub.file.opts.filename;
   let entry = config.extraImports[filename];
+  let adder = new ImportUtil(t, path);
   if (entry) {
-    let adder = new ImportUtil(t, path);
-    if (entry.dependsOnModules) {
-      for (let target of entry.dependsOnModules) {
-        path.node.body.unshift(amdDefine(t, adder, path, target, target));
-      }
+    applyRules(t, path, entry, adder, config, filename);
+  }
+
+  let componentName = config.resolver.reverseComponentLookup(filename);
+  if (componentName) {
+    let rules = config.componentExtraImports[componentName];
+    if (rules) {
+      applyRules(t, path, rules, adder, config, filename);
     }
-    if (entry.dependsOnComponents) {
-      for (let dasherizedName of entry.dependsOnComponents) {
-        let pkg = config.resolver.owningPackage(filename);
-        if (pkg) {
-          let owningEngine = config.resolver.owningEngine(pkg);
-          if (owningEngine) {
-            path.node.body.unshift(
-              amdDefine(
-                t,
-                adder,
-                path,
-                `#embroider_compat/components/${dasherizedName}`,
-                `${owningEngine.packageName}/components/${dasherizedName}`
-              )
-            );
-          }
+  }
+}
+
+function applyRules(
+  t: BabelTypes,
+  path: NodePath<t.Program>,
+  rules: ExtraImports[string],
+  adder: ImportUtil,
+  config: InternalConfig,
+  filename: string
+) {
+  let lookup = lazyPackageLookup(config, filename);
+  if (rules.dependsOnModules) {
+    for (let target of rules.dependsOnModules) {
+      if (lookup.owningPackage) {
+        let runtimeName: string;
+        if (packageName(target)) {
+          runtimeName = target;
+        } else {
+          runtimeName = unrelativize(lookup.owningPackage, target, filename);
         }
+        path.node.body.unshift(amdDefine(t, adder, path, target, runtimeName));
+      }
+    }
+  }
+  if (rules.dependsOnComponents) {
+    for (let dasherizedName of rules.dependsOnComponents) {
+      if (lookup.owningEngine) {
+        path.node.body.unshift(
+          amdDefine(
+            t,
+            adder,
+            path,
+            `#embroider_compat/components/${dasherizedName}`,
+            `${lookup.owningEngine.packageName}/components/${dasherizedName}`
+          )
+        );
       }
     }
   }
-
-  //let componentName = config.resolver.reverseComponentLookup(filename);
 }
 
 function amdDefine(t: BabelTypes, adder: ImportUtil, path: NodePath<t.Program>, target: string, runtimeName: string) {
@@ -102,8 +133,8 @@ function amdDefine(t: BabelTypes, adder: ImportUtil, path: NodePath<t.Program>,
   );
 }
 
-function preprocessExtraImports(config: CompatResolverOptions): InternalConfig['extraImports'] {
-  let extraImports: InternalConfig['extraImports'] = {};
+function preprocessExtraImports(config: CompatResolverOptions): ExtraImports {
+  let extraImports: ExtraImports = {};
   for (let rule of config.activePackageRules) {
     if (rule.addonModules) {
       for (let [filename, moduleRules] of Object.entries(rule.addonModules)) {
@@ -133,6 +164,60 @@ function preprocessExtraImports(config: CompatResolverOptions): InternalConfig['
   return extraImports;
 }
 
+function lazyPackageLookup(config: InternalConfig, filename: string) {
+  let owningPackage: { result: Package | undefined } | undefined;
+  let owningEngine: { result: ReturnType<Resolver['owningEngine']> | undefined } | undefined;
+  return {
+    get owningPackage() {
+      if (!owningPackage) {
+        owningPackage = { result: config.resolver.owningPackage(filename) };
+      }
+      return owningPackage.result;
+    },
+    get owningEngine() {
+      if (!owningEngine) {
+        owningEngine = { result: undefined };
+        let p = this.owningPackage;
+        if (p) {
+          owningEngine.result = config.resolver.owningEngine(p);
+        }
+      }
+      return owningEngine.result;
+    },
+  };
+}
+
+function preprocessComponentExtraImports(config: CompatResolverOptions): ExtraImports {
+  let extraImports: ExtraImports = {};
+  for (let rule of config.activePackageRules) {
+    if (rule.components) {
+      for (let [componentName, rules] of Object.entries(rule.components)) {
+        if (rules.invokes) {
+          extraImports[dasherizeComponent(componentName, rule)] = {
+            dependsOnComponents: Object.values(rules.invokes)
+              .flat()
+              .map(c => dasherizeComponent(c, rules)),
+          };
+        }
+      }
+    }
+  }
+  return extraImports;
+}
+
+function dasherizeComponent(
+  componentSnippet: string,
+  rules: ModuleRules | ComponentRules | ActivePackageRules
+): string {
+  let d = snippetToDasherizedName(componentSnippet);
+  if (!d) {
+    throw new Error(
+      `unable to parse component snippet "${componentSnippet}" from rule ${JSON.stringify(rules, null, 2)}`
+    );
+  }
+  return d;
+}
+
 function expandDependsOnRules(
   root: string,
   filename: string,
@@ -145,13 +230,7 @@ function expandDependsOnRules(
       entry.dependsOnModules = rules.dependsOnModules;
     }
     if (rules.dependsOnComponents) {
-      entry.dependsOnComponents = rules.dependsOnComponents.map(c => {
-        let d = snippetToDasherizedName(c);
-        if (!d) {
-          throw new Error(`unable to parse component snippet "${c}" from rule ${JSON.stringify(rules, null, 2)}`);
-        }
-        return d;
-      });
+      entry.dependsOnComponents = rules.dependsOnComponents.map(c => dasherizeComponent(c, rules));
     }
     extraImports[join(root, filename)] = entry;
   }

packages/compat/src/resolver-transform.ts

@@ -308,6 +308,16 @@ class TemplateResolver implements ASTPlugin {
           let processedRules = preprocessComponentRule(rules);
           let dasherizedName = this.standardDasherize(snippet, rule);
           components.set(dasherizedName, processedRules);
+          if (rules.layout) {
+            for (let root of rule.roots) {
+              if (rules.layout.addonPath) {
+                files.set(join(root, rules.layout.addonPath), processedRules);
+              }
+              if (rules.layout.appPath) {
+                files.set(join(root, rules.layout.appPath), processedRules);
+              }
+            }
+          }
         }
       }
       if (rule.appTemplates) {

packages/compat/tests/audit.test.ts

@@ -316,23 +316,21 @@ describe('audit', function () {
     expect(Object.keys(result.modules).length).toBe(3);
   });
 
-  test.skip('finds missing component in standalone hbs', async function () {
+  test('finds missing component in standalone hbs', async function () {
     merge(app.files, {
       'hello.hbs': `<NoSuchThing />`,
     });
     let result = await audit();
     expect(withoutCodeFrames(result.findings)).toEqual([
       {
-        message: 'Missing component',
-        detail: 'NoSuchThing',
+        message: 'unable to resolve dependency',
+        detail: '#embroider_compat/components/no-such-thing',
         filename: './hello.hbs',
       },
     ]);
-    expect(result.findings[0].codeFrame).toBeDefined();
-    expect(Object.keys(result.modules).length).toBe(3);
   });
 
-  test.skip('finds missing component in inline hbs', async function () {
+  test('finds missing component in inline hbs', async function () {
     merge(app.files, {
       'app.js': `
         import { hbs } from 'ember-cli-htmlbars';
@@ -342,16 +340,14 @@ describe('audit', function () {
     let result = await audit();
     expect(withoutCodeFrames(result.findings)).toEqual([
       {
-        message: 'Missing component',
-        detail: 'NoSuchThing',
+        message: 'unable to resolve dependency',
+        detail: '#embroider_compat/components/no-such-thing',
         filename: './app.js',
       },
     ]);
-    expect(result.findings[0].codeFrame).toBeDefined();
-    expect(Object.keys(result.modules).length).toBe(2);
   });
 
-  test.skip('traverse through template even when it has some errors', async function () {
+  test('traverse through template even when it has some errors', async function () {
     merge(app.files, {
       'hello.hbs': `<NoSuchThing /><Second />`,
       components: {
@@ -363,12 +359,12 @@ describe('audit', function () {
     let result = await audit();
     expect(withoutCodeFrames(result.findings)).toEqual([
       {
-        message: 'Missing component',
-        detail: 'NoSuchThing',
+        message: 'unable to resolve dependency',
+        detail: '#embroider_compat/components/no-such-thing',
         filename: './hello.hbs',
       },
     ]);
-    expect(Object.keys(result.modules).length).toBe(4);
+    expect(Object.keys(result.modules)).toContain('./components/second.js');
   });
 
   test('failure to parse JS is reported and does not cause cascading errors', async function () {

packages/shared-internals/src/index.ts

@@ -1,5 +1,5 @@
 export { AppMeta, AddonMeta, PackageInfo } from './metadata';
-export { explicitRelative, extensionsPattern } from './paths';
+export { explicitRelative, extensionsPattern, unrelativize } from './paths';
 export { getOrCreate } from './get-or-create';
 export { default as Package, V2AddonPackage as AddonPackage, V2AppPackage as AppPackage, V2Package } from './package';
 export { default as PackageCache } from './package-cache';

packages/shared-internals/src/paths.ts

@@ -1,4 +1,5 @@
-import { relative, isAbsolute, dirname, join, basename } from 'path';
+import { relative, isAbsolute, dirname, join, basename, resolve, sep } from 'path';
+import type Package from './package';
 
 // by "explicit", I mean that we want "./local/thing" instead of "local/thing"
 // because
@@ -29,3 +30,14 @@ export function explicitRelative(fromDir: string, toFile: string) {
 export function extensionsPattern(extensions: string[]): RegExp {
   return new RegExp(`(${extensions.map(e => `${e.replace('.', '\\.')}`).join('|')})$`, 'i');
 }
+
+export function unrelativize(pkg: Package, specifier: string, fromFile: string) {
+  if (pkg.packageJSON.exports) {
+    throw new Error(`unsupported: engines cannot use package.json exports`);
+  }
+  let result = resolve(dirname(fromFile), specifier).replace(pkg.root, pkg.name);
+  if (sep !== '/') {
+    result = result.split(sep).join('/');
+  }
+  return result;
+}