parse errors shouldn't cause other import errors

embroider-build · Nov 3, 2020 · ebe58d5 · ebe58d5
1 parent 98567d2
commit ebe58d5
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 8 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -10,7 +10,8 @@
       "name": "Run tests",
       "program": "${workspaceFolder}/node_modules/.bin/jest",
       "cwd": "${workspaceFolder}/packages/compat",
-      "args": ["--runInBand", "--testPathPattern", "tests/stage2.test.js"]
+      "args": ["--runInBand", "--testPathPattern", "tests/audit.test.js"], //, "--test-name-pattern", "traverse"],
+      "outputCapture": "std"
     },
     {
       "type": "node",
@@ -134,7 +135,7 @@
       "name": "Audit static-app",
       "program": "${workspaceFolder}/node_modules/.bin/embroider-compat-audit",
       "cwd": "${workspaceFolder}/test-packages/static-app",
-      "args": ["--reuse-build"],
+      "args": [],
       "outputCapture": "std"
     },
     {

diff --git a/packages/compat/src/audit.ts b/packages/compat/src/audit.ts
@@ -45,6 +45,7 @@ interface InternalModule {
   isCJS: boolean;
   isAMD: boolean;
   templateFindings: { message: string; detail: string; codeFrameIndex?: number }[];
+  failedToParse: boolean;
 }
 
 export interface Import {
@@ -305,7 +306,7 @@ export class Audit {
       } else if (resolved) {
         let target = this.modules.get(resolved)!;
         for (let specifier of imp.specifiers) {
-          if (!this.moduleProvidesName(target, specifier.name)) {
+          if (!target.failedToParse && !this.moduleProvidesName(target, specifier.name)) {
             if (specifier.name === 'default') {
               let backtick = '`';
               this.findings.push({
@@ -382,8 +383,9 @@ export class Audit {
         this.pushFinding({
           filename,
           message: `failed to parse`,
-          detail: err.toString(),
+          detail: err.toString().replace(filename, explicitRelative(this.appDir, filename)),
         });
+        module.failedToParse = true;
         return [];
       } else {
         throw err;
@@ -400,8 +402,9 @@ export class Audit {
       this.pushFinding({
         filename,
         message: `failed to compile template`,
-        detail: err.toString(),
+        detail: err.toString().replace(filename, explicitRelative(this.appDir, filename)),
       });
+      module.failedToParse = true;
       return [];
     }
     module.templateFindings = this.templateResolver.errorsIn(filename).map(err => ({
@@ -421,8 +424,9 @@ export class Audit {
       this.pushFinding({
         filename,
         message: `failed to parse JSON`,
-        detail: err.toString(),
+        detail: err.toString().replace(filename, explicitRelative(this.appDir, filename)),
       });
+      module.failedToParse = true;
       return [];
     }
     return this.visitJS(filename, js, module);
@@ -462,6 +466,7 @@ export class Audit {
         isCJS: false,
         isAMD: false,
         templateFindings: [],
+        failedToParse: false,
       };
       this.modules.set(filename, record);
       this.moduleQueue.add(filename);

diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts
@@ -235,13 +235,14 @@ describe('audit', function () {
       'hello.hbs': `<NoSuchThing />`,
     });
     let result = await audit();
-    expect(result.findings).toEqual([
+    expect(withoutCodeFrames(result.findings)).toEqual([
       {
         message: 'Missing component',
         detail: 'NoSuchThing',
         filename: './hello.hbs',
       },
     ]);
+    expect(result.findings[0].codeFrame).toBeDefined();
     expect(Object.keys(result.modules).length).toBe(3);
   });
 
@@ -255,7 +256,7 @@ describe('audit', function () {
       },
     });
     let result = await audit();
-    expect(result.findings).toEqual([
+    expect(withoutCodeFrames(result.findings)).toEqual([
       {
         message: 'Missing component',
         detail: 'NoSuchThing',
@@ -264,6 +265,18 @@ describe('audit', function () {
     ]);
     expect(Object.keys(result.modules).length).toBe(4);
   });
+
+  test('failure to parse JS is reported and does not cause cascading errors', async function () {
+    merge(app.files, {
+      'app.js': `import thing from './has-parse-error'`,
+      'has-parse-error.js': `export default function() {`,
+    });
+    let result = await audit();
+    expect(result.findings.map(f => ({ filename: f.filename, message: f.message }))).toEqual([
+      { filename: './has-parse-error.js', message: 'failed to parse' },
+    ]);
+    expect(Object.keys(result.modules).length).toBe(3);
+  });
 });
 
 function withoutCodeFrames(findings: Finding[]): Finding[] {