Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate 'ignored' case from module resolver #2209

Merged
merged 2 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
50 changes: 33 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,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,
};
}
}
}
Expand All @@ -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;
Expand All @@ -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 };
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
Loading