From eb423146be73b91ac501c8fd43a0669bbba3a5f1 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Fri, 6 Dec 2024 09:54:12 -0500 Subject: [PATCH] reduce ModuleRequest API --- packages/core/src/module-request.ts | 61 ++++++------------ packages/core/src/module-resolver.ts | 63 +++++++++---------- packages/core/src/node-resolve.ts | 46 +++++++------- packages/vite/src/esbuild-request.ts | 43 +++++++------ packages/vite/src/request.ts | 43 +++++++------ .../webpack/src/webpack-resolver-plugin.ts | 44 +++++++++---- 6 files changed, 153 insertions(+), 147 deletions(-) diff --git a/packages/core/src/module-request.ts b/packages/core/src/module-request.ts index 9c8630272..c456910eb 100644 --- a/packages/core/src/module-request.ts +++ b/packages/core/src/module-request.ts @@ -22,6 +22,12 @@ export type RequestAdapterCreate = ( export interface RequestAdapter { readonly debugType: string; resolve(request: ModuleRequest): Promise; + + // the function-returning variants of both of these are only because webpack + // plugins are a pain in the butt. Integrators are encouraged to use the plain + // Response-returning variants in all sane build environments. + notFoundResponse(request: ModuleRequest): Res | (() => Promise); + virtualResponse(request: ModuleRequest, virtualFileName: string): Res | (() => Promise); } export interface InitialRequestState { @@ -44,23 +50,14 @@ export class ModuleRequest implements Modul #adapter: RequestAdapter; #specifier: string; #fromFile: string; - #isVirtual: boolean; #meta: Record | undefined; - #isNotFound: boolean; - #resolvedTo: Res | undefined; - - private constructor( - adapter: RequestAdapter, - initialize: InitialRequestState, - propagate?: { isVirtual: boolean; isNotFound: boolean; resolvedTo: Res | undefined } - ) { + #resolvedTo: Res | (() => Promise) | undefined; + + private constructor(adapter: RequestAdapter, initialize: InitialRequestState) { this.#adapter = adapter; this.#specifier = initialize.specifier; this.#fromFile = initialize.fromFile; this.#meta = initialize.meta; - this.#isVirtual = propagate?.isVirtual ?? false; - this.#isNotFound = propagate?.isNotFound ?? false; - this.#resolvedTo = propagate?.resolvedTo; } get specifier(): string { @@ -71,10 +68,6 @@ export class ModuleRequest implements Modul return this.#fromFile; } - get isVirtual(): boolean { - return this.#isVirtual; - } - get debugType(): string { return this.#adapter.debugType; } @@ -83,11 +76,7 @@ export class ModuleRequest implements Modul return this.#meta; } - get isNotFound(): boolean { - return this.#isNotFound; - } - - get resolvedTo(): Res | undefined { + get resolvedTo(): Res | (() => Promise) | undefined { return this.#resolvedTo; } @@ -95,10 +84,8 @@ export class ModuleRequest implements Modul if (this.#specifier === newSpecifier) { return this; } - let result = this.#clone(); + let result = this.clone(); result.#specifier = newSpecifier; - result.#isNotFound = false; - result.#resolvedTo = undefined; return result; } @@ -106,36 +93,28 @@ export class ModuleRequest implements Modul if (this.#fromFile === newFromFile) { return this; } - let result = this.#clone(); + let result = this.clone(); result.#fromFile = newFromFile; - result.#isNotFound = false; - result.#resolvedTo = undefined; return result; } virtualize(virtualFileName: string): this { - let result = this.#clone(); - result.#specifier = virtualFileName; - result.#isVirtual = true; - result.#isNotFound = false; - result.#resolvedTo = undefined; - return result; + return this.resolveTo(this.#adapter.virtualResponse(this, virtualFileName)); } withMeta(meta: Record | undefined): this { - let result = this.#clone(); + let result = this.clone(); result.#meta = meta; + result.#resolvedTo = this.#resolvedTo; return result; } notFound(): this { - let result = this.#clone(); - result.#isNotFound = true; - return result; + return this.resolveTo(this.#adapter.notFoundResponse(this)); } - resolveTo(res: Res): this { - let result = this.#clone(); + resolveTo(res: Res | (() => Promise)): this { + let result = this.clone(); result.#resolvedTo = res; return result; } @@ -144,7 +123,7 @@ export class ModuleRequest implements Modul return this.#adapter.resolve(this); } - #clone(): this { - return new ModuleRequest(this.#adapter, this, this) as this; + clone(): this { + return new ModuleRequest(this.#adapter, this) as this; } } diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 6845a3e49..284e287bd 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -37,9 +37,7 @@ makeDebug.formatters.p = (s: string) => { }; function logTransition(reason: string, before: R, after: R = before): R { - if (after.isVirtual) { - debug(`[%s:virtualized] %s because %s\n in %p`, before.debugType, before.specifier, reason, before.fromFile); - } else if (after.resolvedTo) { + if (after.resolvedTo) { debug(`[%s:resolvedTo] %s because %s\n in %p`, before.debugType, before.specifier, reason, before.fromFile); } else if (before.specifier !== after.specifier) { if (before.fromFile !== after.fromFile) { @@ -64,18 +62,12 @@ function logTransition(reason: string, before: R, after before.fromFile, after.fromFile ); - } else if (after.isNotFound) { - debug(`[%s:not-found] %s because %s\n in %p`, before.debugType, before.specifier, reason, before.fromFile); } else { debug(`[%s:unchanged] %s because %s\n in %p`, before.debugType, before.specifier, reason, before.fromFile); } return after; } -function isTerminal(request: ModuleRequest): boolean { - return request.isVirtual || request.isNotFound || Boolean(request.resolvedTo); -} - type MergeEntry = | { type: 'app-only'; @@ -152,12 +144,19 @@ export class Resolver { request: ModuleRequest ): Promise { request = await this.beforeResolve(request); + + let resolution: ResolveResolution; + if (request.resolvedTo) { - return request.resolvedTo; + if (typeof request.resolvedTo === 'function') { + resolution = await request.resolvedTo(); + } else { + resolution = request.resolvedTo; + } + } else { + resolution = await request.defaultResolve(); } - let resolution = await request.defaultResolve(); - switch (resolution.type) { case 'found': case 'ignored': @@ -167,6 +166,9 @@ export class Resolver { default: throw assertNever(resolution); } + + request = request.clone(); + let nextRequest = await this.fallbackResolve(request); if (nextRequest === request) { // no additional fallback is available. @@ -174,7 +176,11 @@ export class Resolver { } if (nextRequest.resolvedTo) { - return nextRequest.resolvedTo; + if (typeof nextRequest.resolvedTo === 'function') { + return await nextRequest.resolvedTo(); + } else { + return nextRequest.resolvedTo; + } } if (nextRequest.fromFile === request.fromFile && nextRequest.specifier === request.specifier) { @@ -183,13 +189,6 @@ export class Resolver { ); } - if (nextRequest.isVirtual || nextRequest.isNotFound) { - // virtual and NotFound requests are terminal, there is no more - // beforeResolve or fallbackResolve around them. The defaultResolve is - // expected to know how to implement them. - return nextRequest.defaultResolve(); - } - return this.resolve(nextRequest); } @@ -224,7 +223,7 @@ export class Resolver { } private generateFastbootSwitch(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } let pkg = this.packageCache.ownerOfFile(request.fromFile); @@ -271,7 +270,7 @@ export class Resolver { } private handleFastbootSwitch(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } let match = decodeFastbootSwitch(request.fromFile); @@ -330,7 +329,7 @@ export class Resolver { } private handleImplicitModules(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } let im = decodeImplicitModules(request.specifier); @@ -361,7 +360,7 @@ export class Resolver { } private handleEntrypoint(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } @@ -410,7 +409,7 @@ export class Resolver { } private handleRouteEntrypoint(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } @@ -487,7 +486,7 @@ export class Resolver { } private async handleGlobalsCompat(request: R): Promise { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } let match = compatPattern.exec(request.specifier); @@ -852,7 +851,7 @@ export class Resolver { } private handleRewrittenPackages(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } let requestingPkg = this.packageCache.ownerOfFile(request.fromFile); @@ -915,7 +914,7 @@ export class Resolver { } private handleRenaming(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } let packageName = getPackageName(request.specifier); @@ -1044,7 +1043,7 @@ export class Resolver { } private preHandleExternal(request: R): R { - if (isTerminal(request)) { + if (request.resolvedTo) { return request; } let { specifier, fromFile } = request; @@ -1149,10 +1148,8 @@ export class Resolver { } private async fallbackResolve(request: R): Promise { - if (request.isVirtual) { - throw new Error( - 'Build tool bug detected! Fallback resolve should never see a virtual request. It is expected that the defaultResolve for your bundler has already resolved this request' - ); + if (request.resolvedTo) { + throw new Error('Build tool bug detected! Fallback resolve should never see an already-resolved request.'); } if (request.specifier === '@embroider/macros') { diff --git a/packages/core/src/node-resolve.ts b/packages/core/src/node-resolve.ts index ce2f6a86e..a724cfe2d 100644 --- a/packages/core/src/node-resolve.ts +++ b/packages/core/src/node-resolve.ts @@ -28,28 +28,32 @@ export class NodeRequestAdapter implements RequestAdapter>): Promise> { - if (request.isVirtual) { - return { - type: 'found', - filename: request.specifier, - isVirtual: true, - result: { - type: 'virtual' as 'virtual', - content: virtualContent(request.specifier, this.resolver).src, - filename: request.specifier, - }, - }; - } - if (request.isNotFound) { - let err = new Error(`module not found ${request.specifier}`); - (err as any).code = 'MODULE_NOT_FOUND'; - return { - type: 'not_found', - err, - }; - } + notFoundResponse(request: ModuleRequest>): Resolution { + let err = new Error(`module not found ${request.specifier}`); + (err as any).code = 'MODULE_NOT_FOUND'; + return { + type: 'not_found', + err, + }; + } + virtualResponse( + _request: ModuleRequest>, + virtualFileName: string + ): Resolution { + return { + type: 'found', + filename: virtualFileName, + isVirtual: true, + result: { + type: 'virtual' as 'virtual', + content: virtualContent(virtualFileName, this.resolver).src, + filename: virtualFileName, + }, + }; + } + + async resolve(request: ModuleRequest>): Promise> { // require.resolve does not like when we resolve from virtual paths. // That is, a request like "../thing.js" from // "/a/real/path/VIRTUAL_SUBDIR/virtual.js" has an unambiguous target of diff --git a/packages/vite/src/esbuild-request.ts b/packages/vite/src/esbuild-request.ts index 37a8cb614..0360f6a05 100644 --- a/packages/vite/src/esbuild-request.ts +++ b/packages/vite/src/esbuild-request.ts @@ -55,27 +55,32 @@ export class EsBuildRequestAdapter implements RequestAdapter> + ): core.Resolution { + return { + type: 'not_found', + err: { + errors: [{ text: `module not found ${request.specifier}` }], + }, + }; + } + + virtualResponse( + _request: core.ModuleRequest>, + virtualFileName: string + ): core.Resolution { + return { + type: 'found', + filename: virtualFileName, + result: { path: virtualFileName, namespace: 'embroider-virtual' }, + isVirtual: true, + }; + } + async resolve( request: ModuleRequest> ): Promise> { - if (request.isVirtual) { - return { - type: 'found', - filename: request.specifier, - result: { path: request.specifier, namespace: 'embroider-virtual' }, - isVirtual: request.isVirtual, - }; - } - if (request.isNotFound) { - // todo: make sure this looks correct to users - return { - type: 'not_found', - err: { - errors: [{ text: `module not found ${request.specifier}` }], - }, - }; - } - requestStatus(request.specifier); let result = await this.context.resolve(request.specifier, { @@ -129,7 +134,7 @@ export class EsBuildRequestAdapter implements RequestAdapter>, + virtualFileName: string + ): Resolution { + let specifier = virtualPrefix + virtualFileName; + return { + type: 'found', + filename: specifier, + result: { + id: this.specifierWithQueryParams(specifier), + resolvedBy: this.fromFileWithQueryParams(request.fromFile), + }, + isVirtual: true, + }; + } + + notFoundResponse(_request: ModuleRequest>): Resolution { + let err = new Error(`module not found ${this.specifierWithQueryParams}`); + (err as any).code = 'MODULE_NOT_FOUND'; + return { type: 'not_found', err }; + } + async resolve(request: ModuleRequest>): Promise> { - if (request.isVirtual) { - let specifier = virtualPrefix + request.specifier; - return { - type: 'found', - filename: specifier, - result: { - id: this.specifierWithQueryParams(specifier), - resolvedBy: this.fromFileWithQueryParams(request.fromFile), - }, - isVirtual: request.isVirtual, - }; - } - if (request.isNotFound) { - // TODO: we can make sure this looks correct in rollup & vite output when a - // user encounters it - let err = new Error(`module not found ${this.specifierWithQueryParams}`); - (err as any).code = 'MODULE_NOT_FOUND'; - return { type: 'not_found', err }; - } let result = await this.context.resolve( this.specifierWithQueryParams(request.specifier), this.fromFileWithQueryParams(request.fromFile), @@ -100,7 +103,7 @@ export class RollupRequestAdapter implements RequestAdapter { // requests evolve through the module-resolver they aren't actually all // mutating the shared state. Only when a request is allowed to bubble back // out to webpack does that happen. - toWebpackResolveData(request: ModuleRequest): ExtendedResolveData { + toWebpackResolveData( + request: ModuleRequest, + virtualFileName: string | undefined + ): ExtendedResolveData { let specifier = request.specifier; - if (request.isVirtual) { + if (virtualFileName) { let params = new URLSearchParams(); - params.set('f', specifier); + params.set('f', virtualFileName); params.set('a', this.appRoot); specifier = `${this.babelLoaderPrefix}${virtualLoaderName}?${params.toString()}!`; } @@ -195,7 +198,7 @@ class WebpackRequestAdapter implements RequestAdapter { this.originalState.context = dirname(request.fromFile); this.originalState.contextInfo.issuer = request.fromFile; this.originalState.contextInfo._embroiderMeta = request.meta; - if (request.resolvedTo) { + if (request.resolvedTo && typeof request.resolvedTo !== 'function') { if (request.resolvedTo.type === 'found') { this.originalState.createData = request.resolvedTo.result; } @@ -203,16 +206,31 @@ class WebpackRequestAdapter implements RequestAdapter { return this.originalState; } + notFoundResponse(request: ModuleRequest): WebpackResolution { + let err = new Error(`module not found ${request.specifier}`); + (err as any).code = 'MODULE_NOT_FOUND'; + return { type: 'not_found', err }; + } + + virtualResponse( + request: ModuleRequest, + virtualFileName: string + ): () => Promise { + return () => { + return this._resolve(request, virtualFileName); + }; + } + async resolve(request: ModuleRequest): Promise { - if (request.isNotFound) { - // TODO: we can make sure this looks correct in webpack output when a - // user encounters it - let err = new Error(`module not found ${request.specifier}`); - (err as any).code = 'MODULE_NOT_FOUND'; - return { type: 'not_found', err }; - } + return this._resolve(request, undefined); + } + + async _resolve( + request: ModuleRequest, + virtualFileName: string | undefined + ): Promise { return await new Promise(resolve => - this.resolveFunction(this.toWebpackResolveData(request), err => { + this.resolveFunction(this.toWebpackResolveData(request, virtualFileName), err => { if (err) { // unfortunately webpack doesn't let us distinguish between Not Found // and other unexpected exceptions here. @@ -221,7 +239,7 @@ class WebpackRequestAdapter implements RequestAdapter { resolve({ type: 'found', result: this.originalState.createData, - isVirtual: request.isVirtual, + isVirtual: Boolean(virtualFileName), filename: this.originalState.createData.resource!, }); }