Splitting reusable module visitor out of Audit

packages/compat/tests/audit.test.ts

@@ -4,7 +4,6 @@ import type { AppMeta } from '@embroider/core';
 import { throwOnWarnings } from '@embroider/core';
 import merge from 'lodash/merge';
 import fromPairs from 'lodash/fromPairs';
-import type { Finding } from '../src/audit';
 import { Audit } from '../src/audit';
 import type { CompatResolverOptions } from '../src/resolver-transform';
 import type { TransformOptions } from '@babel/core';
@@ -223,7 +222,11 @@ describe('audit', function () {
     });
     let result = await audit();
     expect(result.findings).toEqual([]);
-    let exports = result.modules['./app.js'].exports;
+    let appModule = result.modules['./app.js'];
+    if (appModule.type !== 'complete') {
+      throw new Error('./app.js module did not parse or resolve correctly');
+    }
+    let exports = appModule.exports;
     expect(exports).toContain('a');
     expect(exports).toContain('b');
     expect(exports).toContain('c');
@@ -270,17 +273,21 @@ describe('audit', function () {
 
     let result = await audit();
     expect(result.findings).toEqual([]);
-    let exports = result.modules['./app.js'].exports;
+    let appModule = result.modules['./app.js'];
+    if (appModule.type !== 'complete') {
+      throw new Error('./app.js module did not parse or resolve correctly');
+    }
+    let exports = appModule.exports;
     expect(exports).toContain('a');
     expect(exports).toContain('b');
     expect(exports).not.toContain('thing');
     expect(exports).toContain('c');
     expect(exports).toContain('alpha');
     expect(exports).toContain('beta');
     expect(exports).toContain('libC');
-    expect(result.modules['./app.js'].imports.length).toBe(3);
-    let imports = fromPairs(result.modules['./app.js'].imports.map(imp => [imp.source, imp.specifiers]));
-    expect(imports).toEqual({
+    expect(appModule.imports.length).toBe(3);
+    let imports = fromPairs(appModule.imports.map(imp => [imp.source, imp.specifiers]));
+    expect(withoutCodeFrames(imports)).toEqual({
       './lib-a': [
         { name: 'default', local: null },
         { name: 'b', local: null },
@@ -397,10 +404,25 @@ describe('audit', function () {
   });
 });
 
-function withoutCodeFrames(findings: Finding[]): Finding[] {
-  return findings.map(f => {
-    let result = Object.assign({}, f);
-    delete result.codeFrame;
-    return result;
-  });
+function withoutCodeFrames<T extends { codeFrameIndex?: any }>(modules: Record<string, T[]>): Record<string, T[]>;
+function withoutCodeFrames<T extends { codeFrame?: any }>(findings: T[]): T[];
+function withoutCodeFrames<T extends { codeFrameIndex?: any }, U extends { codeFrame?: any }>(
+  input: Record<string, T[]> | U[]
+): Record<string, T[]> | U[] {
+  if (Array.isArray(input)) {
+    return input.map(f => {
+      let result = Object.assign({}, f);
+      delete result.codeFrame;
+      return result;
+    });
+  }
+  const result: Record<string, T[]> = {};
+
+  const knownInput = input;
+
+  for (let item in knownInput) {
+    result[item] = knownInput[item].map(i => ({ ...i, codeFrameIndex: undefined }));
+  }
+
+  return result;
 }

test-packages/support/audit-assertions.ts

@@ -114,6 +114,10 @@ export class ExpectModule {
       this.emitMissingModule();
       return;
     }
+    if (this.module.type === 'unparseable') {
+      this.emitUnparsableModule(message);
+      return;
+    }
     const result = fn(this.module.content);
     this.expectAudit.assert.pushResult({
       result,
@@ -123,11 +127,24 @@ export class ExpectModule {
     });
   }
 
+  private emitUnparsableModule(message?: string) {
+    this.expectAudit.assert.pushResult({
+      result: false,
+      actual: `${this.inputName} failed to parse`,
+      expected: true,
+      message: `${this.inputName} failed to parse${message ? `: (${message})` : ''}`,
+    });
+  }
+
   codeEquals(expectedSource: string) {
     if (!this.module) {
       this.emitMissingModule();
       return;
     }
+    if (this.module.type === 'unparseable') {
+      this.emitUnparsableModule();
+      return;
+    }
     this.expectAudit.assert.codeEqual(this.module.content, expectedSource);
   }
 
@@ -136,6 +153,10 @@ export class ExpectModule {
       this.emitMissingModule();
       return;
     }
+    if (this.module.type === 'unparseable') {
+      this.emitUnparsableModule();
+      return;
+    }
     this.expectAudit.assert.codeContains(this.module.content, expectedSource);
   }
 
@@ -144,6 +165,12 @@ export class ExpectModule {
       this.emitMissingModule();
       return new EmptyExpectResolution();
     }
+
+    if (this.module.type === 'unparseable') {
+      this.emitUnparsableModule();
+      return new EmptyExpectResolution();
+    }
+
     if (!(specifier in this.module.resolutions)) {
       this.expectAudit.assert.pushResult({
         result: false,

tests/scenarios/compat-exclude-dot-files-test.ts

@@ -75,7 +75,7 @@ appScenarios
         // but not be picked up in the entrypoint
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/app-template.js')
+          .resolves('/assets/app-template.js')
           .toModule()
           .withContents(content => {
             assert.notOk(/app-template\/\.foobar/.test(content), '.foobar is not in the entrypoint');

tests/scenarios/compat-renaming-test.ts

@@ -256,7 +256,7 @@ appScenarios
       test('renamed modules keep their classic runtime name when used as implicit-modules', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/app-template.js')
+          .resolves('/assets/app-template.js')
           .toModule()
           .resolves('./-embroider-implicit-modules.js')
           .toModule()

tests/scenarios/compat-stage2-test.ts

@@ -132,7 +132,7 @@ stage2Scenarios
         // check that the app trees with in repo addon are combined correctly
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           //TODO investigate removing this @embroider-dep
           .resolves('my-app/service/in-repo.js')
@@ -143,7 +143,7 @@ stage2Scenarios
         // secondary in-repo-addon was correctly detected and activated
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           //TODO investigate removing this @embroider-dep
           .resolves('my-app/services/secondary.js')
@@ -210,7 +210,7 @@ stage2Scenarios
       test('verifies that the correct lexigraphically sorted addons win', function () {
         let expectModule = expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule();
         expectModule.resolves('my-app/service/in-repo.js').to('./lib/in-repo-b/_app_/service/in-repo.js');
         expectModule.resolves('my-app/service/addon.js').to('./node_modules/dep-b/_app_/service/addon.js');
@@ -220,7 +220,7 @@ stage2Scenarios
       test('addons declared as dependencies should win over devDependencies', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           .resolves('my-app/service/dep-wins-over-dev.js')
           .to('./node_modules/dep-b/_app_/service/dep-wins-over-dev.js');
@@ -229,7 +229,7 @@ stage2Scenarios
       test('in repo addons declared win over dependencies', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           .resolves('my-app/service/in-repo-over-deps.js')
           .to('./lib/in-repo-a/_app_/service/in-repo-over-deps.js');
@@ -238,7 +238,7 @@ stage2Scenarios
       test('ordering with before specified', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           .resolves('my-app/service/test-before.js')
           .to('./node_modules/dev-d/_app_/service/test-before.js');
@@ -247,7 +247,7 @@ stage2Scenarios
       test('ordering with after specified', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           .resolves('my-app/service/test-after.js')
           .to('./node_modules/dev-b/_app_/service/test-after.js');
@@ -709,7 +709,7 @@ stage2Scenarios
       test('non-static other paths are included in the entrypoint', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule().codeContains(`d("my-app/non-static-dir/another-library", function () {
             return i("my-app/non-static-dir/another-library.js");
           });`);
@@ -718,7 +718,7 @@ stage2Scenarios
       test('static other paths are not included in the entrypoint', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           .withContents(content => {
             return !/my-app\/static-dir\/my-library\.js"/.test(content);
@@ -728,7 +728,7 @@ stage2Scenarios
       test('top-level static other paths are not included in the entrypoint', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           .withContents(content => {
             return !content.includes('my-app/top-level-static.js');
@@ -738,7 +738,7 @@ stage2Scenarios
       test('staticAppPaths do not match partial path segments', function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule()
           .withContents(content => {
             return content.includes('my-app/static-dir-not-really/something.js');

tests/scenarios/compat-template-colocation-test.ts

@@ -133,7 +133,7 @@ scenarios
       test(`app's colocated components are implicitly included correctly`, function () {
         expectAudit
           .module('./node_modules/.embroider/rewritten-app/index.html')
-          .resolves('./assets/my-app.js')
+          .resolves('/assets/my-app.js')
           .toModule().codeContains(`d("my-app/components/has-colocated-template", function () {
             return i("my-app/components/has-colocated-template.js");
           });`);