Merge pull request #2209 from embroider-build/eliminate-ignored

Eliminate 'ignored' case from module resolver

packages/core/src/module-request.ts

@@ -3,14 +3,6 @@
 export type Resolution<T = unknown, E = unknown> =
   | { type: 'found'; filename: string; isVirtual: boolean; result: T }
 
-  // used for requests that are special and don't represent real files that
-  // embroider can possibly do anything custom with.
-  //
-  // the motivating use case for introducing this is Vite's depscan which marks
-  // almost everything as "external" as a way to tell esbuild to stop traversing
-  // once it has been seen the first time.
-  | { type: 'ignored'; result: T }
-
   // the important thing about this Resolution is that embroider should do its
   // fallback behaviors here.
   | { type: 'not_found'; err: E };

packages/core/src/module-resolver.ts

@@ -159,7 +159,6 @@ export class Resolver {
 
     switch (resolution.type) {
       case 'found':
-      case 'ignored':
         return resolution;
       case 'not_found':
         break;
@@ -585,10 +584,6 @@ export class Resolver {
         })
       );
 
-      if (resolution.type === 'ignored') {
-        return logTransition(`resolving to ignored component`, request, request.resolveTo(resolution));
-      }
-
       // .hbs is a resolvable extension for us, so we need to exclude it here.
       // It matches as a priority lower than .js, so finding an .hbs means
       // there's definitely not a .js.
@@ -631,10 +626,7 @@ export class Resolver {
       })
     );
 
-    // for the case of 'ignored' that means that esbuild found this helper in an external
-    // package so it should be considered found in this case and we should not look for a
-    // component with this name
-    if (helperMatch.type === 'found' || helperMatch.type === 'ignored') {
+    if (helperMatch.type === 'found') {
       return logTransition('resolve to ambiguous case matched a helper', request, request.resolveTo(helperMatch));
     }
 

packages/core/src/node-resolve.ts

@@ -125,10 +125,6 @@ export async function nodeResolve(
       return resolution;
     case 'found':
       return resolution.result;
-    case 'ignored':
-      throw new Error(
-        `bug: this is supposed to be impossible because NodeModuleRequest.prototype.defaultResove does not use "ignored"`
-      );
     default:
       throw assertNever(resolution);
   }

packages/vite/src/esbuild-request.ts

@@ -56,8 +56,8 @@ export class EsBuildRequestAdapter implements RequestAdapter<Resolution<OnResolv
   }
 
   notFoundResponse(
-    request: core.ModuleRequest<core.Resolution<OnResolveResult, OnResolveResult>>
-  ): core.Resolution<OnResolveResult, OnResolveResult> {
+    request: core.ModuleRequest<Resolution<OnResolveResult, OnResolveResult>>
+  ): Resolution<OnResolveResult, OnResolveResult> {
     return {
       type: 'not_found',
       err: {
@@ -67,9 +67,9 @@ export class EsBuildRequestAdapter implements RequestAdapter<Resolution<OnResolv
   }
 
   virtualResponse(
-    _request: core.ModuleRequest<core.Resolution<OnResolveResult, OnResolveResult>>,
+    _request: core.ModuleRequest<Resolution<OnResolveResult, OnResolveResult>>,
     virtualFileName: string
-  ): core.Resolution<OnResolveResult, OnResolveResult> {
+  ): Resolution<OnResolveResult, OnResolveResult> {
     return {
       type: 'found',
       filename: virtualFileName,
@@ -97,10 +97,8 @@ export class EsBuildRequestAdapter implements RequestAdapter<Resolution<OnResolv
 
     let status = readStatus(request.specifier);
 
-    if (result.errors.length > 0 || status === 'not_found') {
+    if (result.errors.length > 0 || status.type === 'not_found') {
       return { type: 'not_found', err: result };
-    } else if (result.external) {
-      return { type: 'ignored', result };
     } else {
       if (this.phase === 'bundling') {
         // we need to ensure that we don't traverse back into the app while
@@ -133,15 +131,33 @@ export class EsBuildRequestAdapter implements RequestAdapter<Resolution<OnResolv
             externalizedName = externalName(pkg.packageJSON, externalizedName) || externalizedName;
           }
           return {
-            type: 'ignored',
+            type: 'found',
+            filename: externalizedName,
+            isVirtual: false,
             result: {
               path: externalizedName,
               external: true,
             },
           };
         }
       }
-      return { type: 'found', filename: result.path, result, isVirtual: false };
+
+      let filename: string;
+      if (status.type === 'found' && result.external) {
+        // when we know that the file was really found, but vite has
+        // externalized it, report the true filename that was found, not the
+        // externalized request path.
+        filename = status.filename;
+      } else {
+        filename = result.path;
+      }
+
+      return {
+        type: 'found',
+        filename,
+        result,
+        isVirtual: false,
+      };
     }
   }
 }
@@ -156,9 +172,7 @@ export class EsBuildRequestAdapter implements RequestAdapter<Resolution<OnResolv
   plugin.
  */
 function sharedGlobalState() {
-  let channel = (globalThis as any).__embroider_vite_resolver_channel__ as
-    | undefined
-    | Map<string, 'pending' | 'found' | 'not_found'>;
+  let channel = (globalThis as any).__embroider_vite_resolver_channel__ as undefined | Map<string, InternalStatus>;
   if (!channel) {
     channel = new Map();
     (globalThis as any).__embroider_vite_resolver_channel__ = channel;
@@ -167,19 +181,21 @@ function sharedGlobalState() {
 }
 
 function requestStatus(id: string): void {
-  sharedGlobalState().set(id, 'pending');
+  sharedGlobalState().set(id, { type: 'pending' });
 }
 
-export function writeStatus(id: string, status: 'found' | 'not_found'): void {
+export function writeStatus(id: string, status: InternalStatus): void {
   let channel = sharedGlobalState();
-  if (channel.get(id) === 'pending') {
+  if (channel.get(id)?.type === 'pending') {
     channel.set(id, status);
   }
 }
 
-function readStatus(id: string): 'pending' | 'not_found' | 'found' {
+function readStatus(id: string): InternalStatus {
   let channel = sharedGlobalState();
-  let result = channel.get(id) ?? 'pending';
+  let result = channel.get(id) ?? { type: 'pending' };
   channel.delete(id);
   return result;
 }
+
+type InternalStatus = { type: 'pending' } | { type: 'not_found' } | { type: 'found'; filename: string };

packages/vite/src/esbuild-resolver.ts

@@ -67,7 +67,6 @@ export function esBuildResolver(): EsBuildPlugin {
         let resolution = await resolverLoader.resolver.resolve(request);
         switch (resolution.type) {
           case 'found':
-          case 'ignored':
             return resolution.result;
           case 'not_found':
             return resolution.err;

packages/vite/src/resolver.ts

@@ -86,8 +86,6 @@ export function resolver(): Plugin {
           } else {
             return await maybeCaptureNewOptimizedDep(this, resolverLoader.resolver, resolution.result, notViteDeps);
           }
-        case 'ignored':
-          return resolution.result;
         case 'not_found':
           return null;
         default:
@@ -159,7 +157,7 @@ async function observeDepScan(context: PluginContext, source: string, importer:
     ...options,
     skipSelf: true,
   });
-  writeStatus(source, result ? 'found' : 'not_found');
+  writeStatus(source, result ? { type: 'found', filename: result.id } : { type: 'not_found' });
   return result;
 }
 

packages/webpack/src/webpack-resolver-plugin.ts

@@ -62,7 +62,6 @@ export class EmbroiderPlugin {
                   callback(resolution.err);
                   break;
                 case 'found':
-                case 'ignored':
                   callback(null, undefined);
                   break;
                 default: