clean up todos

embroider-build · Apr 25, 2024 · 81946a0 · 81946a0
1 parent 45479dc
commit 81946a0
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 18 deletions.
diff --git a/packages/compat/src/audit.ts b/packages/compat/src/audit.ts
@@ -226,7 +226,7 @@ export class Audit {
     }
   };
 
-  private load = async (id: string): Promise<Finding[] | { content: string | Buffer; type: ContentType }> => {
+  private load = async (id: string): Promise<{ content: string | Buffer; type: ContentType } | undefined> => {
     let content: string | Buffer;
     if (this.virtualModules.has(id)) {
       content = this.virtualModules.get(id)!;
@@ -310,19 +310,18 @@ export class Audit {
     return isNamespaceMarker(name) || target.isCJS || target.isAMD || target.exports.includes(name);
   }
 
-  private handleJSON(filename: string, content: Buffer | string): { content: string; type: ContentType } | Finding[] {
+  private handleJSON(filename: string, content: Buffer | string): { content: string; type: ContentType } | undefined {
     let js;
     try {
       let structure = JSON.parse(content.toString('utf8'));
       js = `export default ${JSON.stringify(structure)}`;
     } catch (err) {
-      return [
-        {
-          filename,
-          message: `failed to parse JSON`,
-          detail: err.toString().replace(filename, explicitRelative(this.originAppRoot, filename)),
-        },
-      ];
+      this.findings.push({
+        filename,
+        message: `failed to parse JSON`,
+        detail: err.toString().replace(filename, explicitRelative(this.originAppRoot, filename)),
+      });
+      return;
     }
     return { content: js, type: 'javascript' };
   }

diff --git a/packages/compat/src/module-visitor.ts b/packages/compat/src/module-visitor.ts
@@ -1,6 +1,3 @@
-// TODO: split this so we have the import-analysis part as part of
-// module-visitor and Audit adds its problem-detectoin stuff in the (optional)
-// babel config
 import { explicitRelative } from '@embroider/core';
 import {
   type CodeFrameStorage,
@@ -13,7 +10,6 @@ import fromPairs from 'lodash/fromPairs';
 import assertNever from 'assert-never';
 import { JSDOM } from 'jsdom';
 
-// TODO: this is an audit concern
 import type { Finding } from './audit';
 import type { TransformOptions } from '@babel/core';
 
@@ -91,12 +87,10 @@ export interface Import {
 interface VisitorParams {
   base: string;
   resolveId: (specifier: string, fromFile: string) => Promise<string | undefined>;
-  // TODO: remove Finding[] from this type
-  load: (id: string) => Promise<Finding[] | { content: string | Buffer; type: ContentType }>;
+  load: (id: string) => Promise<{ content: string | Buffer; type: ContentType } | undefined>;
   entrypoints: string[];
   debug?: boolean;
 
-  // TODO: remove below this point
   findings: Finding[];
   frames: CodeFrameStorage;
   babelConfig: TransformOptions;
@@ -116,8 +110,7 @@ class ModuleVisitor {
   private base: string;
   private debugEnabled: boolean;
   private resolveId: (specifier: string, fromFile: string) => Promise<string | undefined>;
-  // TODO: remove Finding[] from return type
-  private load: (id: string) => Promise<Finding[] | { content: string | Buffer; type: ContentType }>;
+  private load: (id: string) => Promise<{ content: string | Buffer; type: ContentType } | undefined>;
   private entrypoints: string[];
 
   constructor(private params: VisitorParams) {
@@ -149,6 +142,10 @@ class ModuleVisitor {
         }
         continue;
       }
+      // if the load hook returned undefined we need to just skip it
+      if (loaded === undefined) {
+        continue;
+      }
       let { content, type } = loaded;
 
       let visitor = this.visitorFor(type);