Skip to content

Commit

Permalink
Eliminate 'ignored' case from module resolver
Browse files Browse the repository at this point in the history
The ignored result was created to deal with the weird way vite resolves many things as "external" during dependency prescanning. But it was still complicating our ability to properly detect components during prescan.

This refactor manages to use the state capture we were already relying on to identify true "not_found" that vite was ignoring to also provide the true "found" that vite is ignoring, so our own use of the resolver never needs to deal with an "ignored" state.
  • Loading branch information
ef4 committed Dec 11, 2024
1 parent 528a2f4 commit 28ebc3c
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 43 deletions.
8 changes: 0 additions & 8 deletions packages/core/src/module-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
10 changes: 1 addition & 9 deletions packages/core/src/module-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ export class Resolver {

switch (resolution.type) {
case 'found':
case 'ignored':
return resolution;
case 'not_found':
break;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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));
}

Expand Down
4 changes: 0 additions & 4 deletions packages/core/src/node-resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
40 changes: 23 additions & 17 deletions packages/vite/src/esbuild-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -133,15 +131,23 @@ 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 };

return {
type: 'found',
filename: status.type === 'found' ? status.filename : result.path,
result,
isVirtual: false,
};
}
}
}
Expand All @@ -156,9 +162,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;
Expand All @@ -167,19 +171,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 };
1 change: 0 additions & 1 deletion packages/vite/src/esbuild-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions packages/vite/src/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}

Expand Down
1 change: 0 additions & 1 deletion packages/webpack/src/webpack-resolver-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export class EmbroiderPlugin {
callback(resolution.err);
break;
case 'found':
case 'ignored':
callback(null, undefined);
break;
default:
Expand Down

0 comments on commit 28ebc3c

Please sign in to comment.