From 28ebc3cd172337ed0bea57969d0cedb9d8bc283e Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Wed, 11 Dec 2024 17:54:11 -0500 Subject: [PATCH 1/2] Eliminate 'ignored' case from module resolver 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. --- packages/core/src/module-request.ts | 8 ---- packages/core/src/module-resolver.ts | 10 +---- packages/core/src/node-resolve.ts | 4 -- packages/vite/src/esbuild-request.ts | 40 +++++++++++-------- packages/vite/src/esbuild-resolver.ts | 1 - packages/vite/src/resolver.ts | 4 +- .../webpack/src/webpack-resolver-plugin.ts | 1 - 7 files changed, 25 insertions(+), 43 deletions(-) diff --git a/packages/core/src/module-request.ts b/packages/core/src/module-request.ts index c456910eb..4d67d1fd6 100644 --- a/packages/core/src/module-request.ts +++ b/packages/core/src/module-request.ts @@ -3,14 +3,6 @@ export type Resolution = | { 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 }; diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 284e287bd..750980b76 100644 --- a/packages/core/src/module-resolver.ts +++ b/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)); } diff --git a/packages/core/src/node-resolve.ts b/packages/core/src/node-resolve.ts index a724cfe2d..5b6a7fbaa 100644 --- a/packages/core/src/node-resolve.ts +++ b/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); } diff --git a/packages/vite/src/esbuild-request.ts b/packages/vite/src/esbuild-request.ts index e17c3d7bb..548547384 100644 --- a/packages/vite/src/esbuild-request.ts +++ b/packages/vite/src/esbuild-request.ts @@ -56,8 +56,8 @@ export class EsBuildRequestAdapter implements RequestAdapter> - ): core.Resolution { + request: core.ModuleRequest> + ): Resolution { return { type: 'not_found', err: { @@ -67,9 +67,9 @@ export class EsBuildRequestAdapter implements RequestAdapter>, + _request: core.ModuleRequest>, virtualFileName: string - ): core.Resolution { + ): Resolution { return { type: 'found', filename: virtualFileName, @@ -97,10 +97,8 @@ export class EsBuildRequestAdapter implements RequestAdapter 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,7 +131,9 @@ export class EsBuildRequestAdapter implements RequestAdapter; + let channel = (globalThis as any).__embroider_vite_resolver_channel__ as undefined | Map; if (!channel) { channel = new Map(); (globalThis as any).__embroider_vite_resolver_channel__ = channel; @@ -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 }; diff --git a/packages/vite/src/esbuild-resolver.ts b/packages/vite/src/esbuild-resolver.ts index 94097c94d..94ec23adc 100644 --- a/packages/vite/src/esbuild-resolver.ts +++ b/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; diff --git a/packages/vite/src/resolver.ts b/packages/vite/src/resolver.ts index bb0b090b1..eff039f00 100644 --- a/packages/vite/src/resolver.ts +++ b/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; } diff --git a/packages/webpack/src/webpack-resolver-plugin.ts b/packages/webpack/src/webpack-resolver-plugin.ts index 86b79d6c0..1471b4457 100644 --- a/packages/webpack/src/webpack-resolver-plugin.ts +++ b/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: From 1892fb85026801219d271865734dd7d068c2788b Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Wed, 11 Dec 2024 19:47:44 -0500 Subject: [PATCH 2/2] only override response filename when vite has messed with it --- packages/vite/src/esbuild-request.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/esbuild-request.ts b/packages/vite/src/esbuild-request.ts index 548547384..41a39dd34 100644 --- a/packages/vite/src/esbuild-request.ts +++ b/packages/vite/src/esbuild-request.ts @@ -142,9 +142,19 @@ export class EsBuildRequestAdapter implements RequestAdapter