diff --git a/packages/compat/src/audit.ts b/packages/compat/src/audit.ts index cad3d8a96..499313fce 100644 --- a/packages/compat/src/audit.ts +++ b/packages/compat/src/audit.ts @@ -534,6 +534,12 @@ export class Audit { private async resolveDeps(deps: string[], fromFile: string): Promise { let resolved = new Map() as NonNullable; for (let dep of deps) { + if (['@embroider/macros'].includes(dep)) { + // the audit process deliberately removes the @embroider/macros babel + // plugins, so the imports are still present and should be left alone. + continue; + } + let resolution = await this.resolver.nodeResolve(dep, fromFile); switch (resolution.type) { case 'virtual': @@ -542,11 +548,6 @@ export class Audit { this.scheduleVisit(resolution.filename, fromFile); break; case 'not_found': - if (['@embroider/macros', '@ember/template-factory'].includes(dep)) { - // the audit process deliberately removes the @embroider/macros babel - // plugins, so the imports are still present and should be left alone. - continue; - } resolved.set(dep, { isResolutionFailure: true as true }); break; case 'real': diff --git a/packages/compat/src/compat-app-builder.ts b/packages/compat/src/compat-app-builder.ts index 5f74ffa08..00f475f88 100644 --- a/packages/compat/src/compat-app-builder.ts +++ b/packages/compat/src/compat-app-builder.ts @@ -263,11 +263,6 @@ export class CompatAppBuilder { let renamePackages = Object.assign({}, ...this.allActiveAddons.map(dep => dep.meta['renamed-packages'])); let renameModules = Object.assign({}, ...this.allActiveAddons.map(dep => dep.meta['renamed-modules'])); - let activeAddons: CompatResolverOptions['activeAddons'] = {}; - for (let addon of this.allActiveAddons) { - activeAddons[addon.name] = addon.root; - } - let options: CompatResolverOptions['options'] = { staticHelpers: this.options.staticHelpers, staticModifiers: this.options.staticModifiers, @@ -277,7 +272,6 @@ export class CompatAppBuilder { let config: CompatResolverOptions = { // this part is the base ModuleResolverOptions as required by @embroider/core - activeAddons, renameModules, renamePackages, resolvableExtensions: this.resolvableExtensions(), diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts index b0a9ea286..d8474cf03 100644 --- a/packages/compat/tests/audit.test.ts +++ b/packages/compat/tests/audit.test.ts @@ -42,7 +42,6 @@ describe('audit', function () { activePackageRules: [], renamePackages: {}, renameModules: {}, - activeAddons: {}, engines: [ { packageName: 'audit-this-app', diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 09eac8015..83aa78ae7 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -5,19 +5,17 @@ import { packageName as getPackageName, packageName, } from '@embroider/shared-internals'; -import { dirname, resolve } from 'path'; +import { dirname, resolve, posix } from 'path'; import type { Package, V2Package } from '@embroider/shared-internals'; import { explicitRelative, RewrittenPackageCache } from '@embroider/shared-internals'; import makeDebug from 'debug'; import assertNever from 'assert-never'; -import resolveModule from 'resolve'; import reversePackageExports from '@embroider/reverse-exports'; import { virtualExternalESModule, virtualExternalCJSModule, virtualPairComponent, - virtualContent, fastbootSwitch, decodeFastbootSwitch, decodeImplicitModules, @@ -26,6 +24,7 @@ import { Memoize } from 'typescript-memoize'; import { describeExports } from './describe-exports'; import { readFileSync } from 'fs'; import type UserOptions from './options'; +import { nodeResolve } from './node-resolve'; const debug = makeDebug('embroider:resolver'); @@ -59,12 +58,15 @@ function logTransition(reason: string, before: R, after } } else if (before.fromFile !== after.fromFile) { debug( - `[%s:rehomed] because %s\n from %p\n to %p`, + `[%s:rehomed] %s, because %s\n from %p\n to %p`, before.debugType, + before.specifier, reason, 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); } @@ -78,9 +80,6 @@ export interface Options { renameModules: { [fromName: string]: string; }; - activeAddons: { - [packageName: string]: string; - }; resolvableExtensions: string[]; appRoot: string; engines: EngineConfig[]; @@ -137,40 +136,12 @@ export interface ModuleRequest { readonly isVirtual: boolean; readonly meta: Record | undefined; readonly debugType: string; + readonly isNotFound: boolean; alias(newSpecifier: string): this; rehome(newFromFile: string): this; virtualize(virtualFilename: string): this; withMeta(meta: Record | undefined): this; -} - -class NodeModuleRequest implements ModuleRequest { - constructor( - readonly specifier: string, - readonly fromFile: string, - readonly isVirtual: boolean, - readonly meta: Record | undefined - ) {} - - get debugType() { - return 'node'; - } - - alias(specifier: string): this { - return new NodeModuleRequest(specifier, this.fromFile, false, this.meta) as this; - } - rehome(fromFile: string): this { - if (this.fromFile === fromFile) { - return this; - } else { - return new NodeModuleRequest(this.specifier, fromFile, false, this.meta) as this; - } - } - virtualize(filename: string): this { - return new NodeModuleRequest(filename, this.fromFile, true, this.meta) as this; - } - withMeta(meta: Record | undefined): this { - return new NodeModuleRequest(this.specifier, this.fromFile, this.isVirtual, meta) as this; - } + notFound(): this; } // This is generic because different build systems have different ways of @@ -280,10 +251,10 @@ export class Resolver { ); } - if (nextRequest.isVirtual) { - // virtual requests are terminal, there is no more beforeResolve or - // fallbackResolve around them. The defaultResolve is expected to know how - // to implement them. + 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 yield defaultResolve(nextRequest); } return yield* this.internalResolve(nextRequest, defaultResolve); @@ -299,38 +270,7 @@ export class Resolver { | { type: 'virtual'; filename: string; content: string } | { type: 'real'; filename: string } | { type: 'not_found'; err: Error } { - let resolution = this.resolveSync(new NodeModuleRequest(specifier, fromFile, false, undefined), request => { - if (request.isVirtual) { - return { - type: 'found', - result: { - type: 'virtual' as 'virtual', - content: virtualContent(request.specifier, this), - filename: request.specifier, - }, - }; - } - try { - let filename = resolveModule.sync(request.specifier, { - basedir: dirname(request.fromFile), - extensions: this.options.resolvableExtensions, - }); - return { type: 'found', result: { type: 'real' as 'real', filename } }; - } catch (err) { - if (err.code !== 'MODULE_NOT_FOUND') { - throw err; - } - return { type: 'not_found', err }; - } - }); - switch (resolution.type) { - case 'not_found': - return resolution; - case 'found': - return resolution.result; - default: - throw assertNever(resolution); - } + return nodeResolve(this, specifier, fromFile); } get packageCache() { @@ -626,9 +566,9 @@ export class Resolver { private parseGlobalPath(path: string, inEngine: EngineConfig) { let parts = path.split('@'); if (parts.length > 1 && parts[0].length > 0) { - return { packageName: parts[0], memberName: parts[1], from: resolve(inEngine.root, 'pacakge.json') }; + return { packageName: parts[0], memberName: parts[1], from: resolve(inEngine.root, 'package.json') }; } else { - return { packageName: inEngine.packageName, memberName: path, from: resolve(inEngine.root, 'pacakge.json') }; + return { packageName: inEngine.packageName, memberName: path, from: resolve(inEngine.root, 'package.json') }; } } @@ -755,7 +695,7 @@ export class Resolver { } private handleRewrittenPackages(request: R): R { - if (request.isVirtual) { + if (request.isVirtual || request.isNotFound) { return request; } let requestingPkg = this.packageCache.ownerOfFile(request.fromFile); @@ -774,10 +714,6 @@ export class Resolver { try { targetPkg = this.packageCache.resolve(packageName, requestingPkg); } catch (err) { - // this is not the place to report resolution failures. If the thing - // doesn't resolve, we're just not interested in redirecting it for - // backward-compat, that's all. The rest of the system will take care of - // reporting a failure to resolve (or handling it a different way) if (err.code !== 'MODULE_NOT_FOUND') { throw err; } @@ -798,20 +734,31 @@ export class Resolver { this.resolveWithinMovedPackage(request, targetPkg) ); } else if (originalRequestingPkg !== requestingPkg) { - // in this case, the requesting package is moved but its destination is - // not, so we need to rehome the request back to the original location. - return logTransition( - 'outbound request from moved package', - request, - request.withMeta({ wasMovedTo: request.fromFile }).rehome(resolve(originalRequestingPkg.root, 'package.json')) - ); + if (targetPkg) { + // in this case, the requesting package is moved but its destination is + // not, so we need to rehome the request back to the original location. + return logTransition( + 'outbound request from moved package', + request, + request + // setting meta here because if this fails, we want the fallback + // logic to revert our rehome and continue from the *moved* package. + .withMeta({ originalFromFile: request.fromFile }) + .rehome(resolve(originalRequestingPkg.root, 'package.json')) + ); + } else { + // requesting package was moved and we failed to find its target. We + // can't let that accidentally succeed in the defaultResolve because we + // could escape the moved package system. + return logTransition('missing outbound request from moved package', request, request.notFound()); + } } return request; } private handleRenaming(request: R): R { - if (request.isVirtual) { + if (request.isVirtual || request.isNotFound) { return request; } let packageName = getPackageName(request.specifier); @@ -857,10 +804,15 @@ export class Resolver { // packages get this help, v2 packages are natively supposed to make their // own modules resolvable, and we want to push them all to do that // correctly. + + // "my-package/foo" -> "./foo" + // "my-package" -> "./" (this can't be just "." because node's require.resolve doesn't reliable support that) + let selfImportPath = request.specifier === pkg.name ? './' : request.specifier.replace(pkg.name, '.'); + return logTransition( `v1 self-import`, request, - request.alias(request.specifier.replace(pkg.name, '.')).rehome(resolve(pkg.root, 'package.json')) + request.alias(selfImportPath).rehome(resolve(pkg.root, 'package.json')) ); } @@ -872,19 +824,20 @@ export class Resolver { if (pkg.name.startsWith('@')) { levels.push('..'); } + let originalFromFile = request.fromFile; let newRequest = request.rehome(resolve(pkg.root, ...levels, 'moved-package-target.js')); if (newRequest === request) { return request; } - return newRequest.withMeta({ - resolvedWithinPackage: pkg.root, - }); + // setting meta because if this fails, we want the fallback to pick up back + // in the original requesting package. + return newRequest.withMeta({ originalFromFile }); } private preHandleExternal(request: R): R { - if (request.isVirtual) { + if (request.isVirtual || request.isNotFound) { return request; } let { specifier, fromFile } = request; @@ -924,7 +877,15 @@ export class Resolver { return logTransition( 'beforeResolve: relative import in app-js', request, - request.rehome(resolve(logicalLocation.owningEngine.root, logicalLocation.inAppName)) + request + .alias('./' + posix.join(dirname(logicalLocation.inAppName), request.specifier)) + // it's important that we're rehoming this to the root of the engine + // (which we know really exists), and not to a subdir like + // logicalLocation.inAppName (which might not physically exist), + // because some environments (including node's require.resolve) will + // refuse to do resolution from a notional path that doesn't + // physically exist. + .rehome(resolve(logicalLocation.owningEngine.root, 'package.json')) ); } @@ -944,14 +905,13 @@ export class Resolver { if (emberVirtualPeerDeps.has(packageName) && !pkg.hasDependency(packageName)) { // addons (whether auto-upgraded or not) may use the app's // emberVirtualPeerDeps, like "@glimmer/component" etc. - if (!this.options.activeAddons[packageName]) { - throw new Error(`${pkg.name} is trying to import the app's ${packageName} package, but it seems to be missing`); + let addon = this.locateActiveAddon(packageName); + if (!addon) { + throw new Error( + `${pkg.name} is trying to import the emberVirtualPeerDep "${packageName}", but it seems to be missing` + ); } - let newHome = resolve( - this.packageCache.maybeMoved(this.packageCache.get(this.options.appRoot)).root, - 'package.json' - ); - return logTransition(`emberVirtualPeerDeps in v2 addon`, request, request.rehome(newHome)); + return logTransition(`emberVirtualPeerDeps`, request, request.rehome(addon.canResolveFromFile)); } // if this file is part of an addon's app-js, it's really the logical @@ -989,6 +949,26 @@ export class Resolver { return request; } + private locateActiveAddon(packageName: string): { root: string; canResolveFromFile: string } | undefined { + if (packageName === this.options.modulePrefix) { + // the app itself is something that addon's can classically resolve if they know it's name. + return { + root: this.options.appRoot, + canResolveFromFile: resolve( + this.packageCache.maybeMoved(this.packageCache.get(this.options.appRoot)).root, + 'package.json' + ), + }; + } + for (let engine of this.options.engines) { + for (let addon of engine.activeAddons) { + if (addon.name === packageName) { + return addon; + } + } + } + } + private external(label: string, request: R, specifier: string): R { if (this.options.amdCompatibility === 'cjs') { let filename = virtualExternalCJSModule(specifier); @@ -1035,9 +1015,7 @@ export class Resolver { return logTransition('fallback early exit', request); } - let { specifier, fromFile } = request; - - if (compatPattern.test(specifier)) { + if (compatPattern.test(request.specifier)) { // Some kinds of compat requests get rewritten into other things // deterministically. For example, "#embroider_compat/helpers/whatever" // means only "the-current-engine/helpers/whatever", and if that doesn't @@ -1054,28 +1032,21 @@ export class Resolver { return request; } - if (fromFile.endsWith('moved-package-target.js')) { - if (!request.meta?.resolvedWithinPackage) { - throw new Error(`bug: embroider resolver's meta is not propagating`); - } - fromFile = resolve(request.meta?.resolvedWithinPackage as string, 'package.json'); - } - - let pkg = this.packageCache.ownerOfFile(fromFile); + let pkg = this.packageCache.ownerOfFile(request.fromFile); if (!pkg) { return logTransition('no identifiable owningPackage', request); } - // if we rehomed this request to its un-rewritten location in order to try - // to do the defaultResolve from there, now we refer back to the rewritten - // location because that's what we want to use when asking things like - // isV2Ember() + // meta.originalFromFile gets set when we want to try to rehome a request + // but then come back to the original location here in the fallback when the + // rehomed request fails let movedPkg = this.packageCache.maybeMoved(pkg); if (movedPkg !== pkg) { - if (!request.meta?.wasMovedTo) { + let originalFromFile = request.meta?.originalFromFile; + if (typeof originalFromFile !== 'string') { throw new Error(`bug: embroider resolver's meta is not propagating`); } - fromFile = request.meta.wasMovedTo as string; + request = request.rehome(originalFromFile); pkg = movedPkg; } @@ -1083,7 +1054,7 @@ export class Resolver { return logTransition('fallbackResolve: not in an ember package', request); } - let packageName = getPackageName(specifier); + let packageName = getPackageName(request.specifier); if (!packageName) { // this is a relative import @@ -1094,7 +1065,7 @@ export class Resolver { let appJSMatch = this.searchAppTree( request, withinEngine, - explicitRelative(pkg.root, resolve(dirname(fromFile), specifier)) + explicitRelative(pkg.root, resolve(dirname(request.fromFile), request.specifier)) ); if (appJSMatch) { return logTransition('fallbackResolve: relative appJsMatch', request, appJSMatch); @@ -1108,18 +1079,17 @@ export class Resolver { } // auto-upgraded packages can fall back to the set of known active addons - if (pkg.meta['auto-upgraded'] && this.options.activeAddons[packageName]) { - const rehomed = this.resolveWithinMovedPackage( - request, - this.packageCache.get(this.options.activeAddons[packageName]) - ); - - if (rehomed !== request) { - return logTransition(`activeAddons`, request, rehomed); + if (pkg.meta['auto-upgraded']) { + let addon = this.locateActiveAddon(packageName); + if (addon) { + const rehomed = request.rehome(addon.canResolveFromFile); + if (rehomed !== request) { + return logTransition(`activeAddons`, request, rehomed); + } } } - let logicalLocation = this.reverseSearchAppTree(pkg, fromFile); + let logicalLocation = this.reverseSearchAppTree(pkg, request.fromFile); if (logicalLocation) { // the requesting file is in an addon's appTree. We didn't succeed in // resolving this (non-relative) request from inside the actual addon, so @@ -1128,13 +1098,18 @@ export class Resolver { return logTransition( 'fallbackResolve: retry from logical home of app-js file', request, - request.rehome(resolve(logicalLocation.owningEngine.root, logicalLocation.inAppName)) + // it might look more precise to rehome into logicalLocation.inAppName + // rather than package.json. But that logical location may not actually + // exist, and some systems (including node's require.resolve) will be + // mad about trying to resolve from notional paths that don't really + // exist. + request.rehome(resolve(logicalLocation.owningEngine.root, 'package.json')) ); } let targetingEngine = this.engineConfig(packageName); if (targetingEngine) { - let appJSMatch = this.searchAppTree(request, targetingEngine, specifier.replace(packageName, '.')); + let appJSMatch = this.searchAppTree(request, targetingEngine, request.specifier.replace(packageName, '.')); if (appJSMatch) { return logTransition('fallbackResolve: non-relative appJsMatch', request, appJSMatch); } @@ -1145,13 +1120,13 @@ export class Resolver { // runtime. Native v2 packages can only get this behavior in the // isExplicitlyExternal case above because they need to explicitly ask for // externals. - return this.external('v1 catch-all fallback', request, specifier); + return this.external('v1 catch-all fallback', request, request.specifier); } else { // native v2 packages don't automatically externalize *everything* the way // auto-upgraded packages do, but they still externalize known and approved // ember virtual packages (like @ember/component) if (emberVirtualPackages.has(packageName)) { - return this.external('emberVirtualPackages', request, specifier); + return this.external('emberVirtualPackages', request, request.specifier); } } diff --git a/packages/core/src/node-resolve.ts b/packages/core/src/node-resolve.ts new file mode 100644 index 000000000..17e2919a2 --- /dev/null +++ b/packages/core/src/node-resolve.ts @@ -0,0 +1,112 @@ +import { virtualContent } from './virtual-content'; +import { dirname, resolve, isAbsolute } from 'path'; +import { explicitRelative } from '@embroider/shared-internals'; +import assertNever from 'assert-never'; + +// these would be circular, but they're type-only so it's fine +import type { ModuleRequest, Resolver } from './module-resolver'; + +export class NodeModuleRequest implements ModuleRequest { + constructor( + readonly specifier: string, + readonly fromFile: string, + readonly isVirtual: boolean, + readonly meta: Record | undefined, + readonly isNotFound: boolean + ) {} + + get debugType() { + return 'node'; + } + + alias(specifier: string): this { + return new NodeModuleRequest(specifier, this.fromFile, false, this.meta, false) as this; + } + rehome(fromFile: string): this { + if (this.fromFile === fromFile) { + return this; + } else { + return new NodeModuleRequest(this.specifier, fromFile, false, this.meta, false) as this; + } + } + virtualize(filename: string): this { + return new NodeModuleRequest(filename, this.fromFile, true, this.meta, false) as this; + } + withMeta(meta: Record | undefined): this { + return new NodeModuleRequest(this.specifier, this.fromFile, this.isVirtual, meta, this.isNotFound) as this; + } + notFound(): this { + return new NodeModuleRequest(this.specifier, this.fromFile, this.isVirtual, this.meta, true) as this; + } +} + +export function nodeResolve( + resolver: Resolver, + specifier: string, + fromFile: string +): + | { type: 'virtual'; filename: string; content: string } + | { type: 'real'; filename: string } + | { type: 'not_found'; err: Error } { + let resolution = resolver.resolveSync( + new NodeModuleRequest(specifier, fromFile, false, undefined, false), + request => { + if (request.isVirtual) { + return { + type: 'found', + result: { + type: 'virtual' as 'virtual', + content: virtualContent(request.specifier, resolver), + 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, + }; + } + try { + // 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 + // "/a/real/path/thing.js", but require.resolve won't do that path + // adjustment until after checking whether VIRTUAL_SUBDIR actually + // exists. + // + // We can do the path adjustments before doing require.resolve. + let { specifier } = request; + let fromDir = dirname(request.fromFile); + if (!isAbsolute(specifier) && specifier.startsWith('.')) { + let targetPath = resolve(fromDir, specifier); + let newFromDir = dirname(targetPath); + if (fromDir !== newFromDir) { + specifier = explicitRelative(newFromDir, targetPath); + fromDir = newFromDir; + } + } + + let filename = require.resolve(specifier, { + paths: [fromDir], + }); + return { type: 'found', result: { type: 'real' as 'real', filename } }; + } catch (err) { + if (err.code !== 'MODULE_NOT_FOUND') { + throw err; + } + return { type: 'not_found', err }; + } + } + ); + switch (resolution.type) { + case 'not_found': + return resolution; + case 'found': + return resolution.result; + default: + throw assertNever(resolution); + } +} diff --git a/packages/vite/src/addons.ts b/packages/vite/src/addons.ts deleted file mode 100644 index c52313ef2..000000000 --- a/packages/vite/src/addons.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { ResolverLoader, packageName } from '@embroider/core'; - -export function addons(root: string): string[] { - let rloader = new ResolverLoader(root); - let { options } = rloader.resolver; - let names = new Set(); - for (let from of Object.keys(options.renameModules)) { - let pName = packageName(from); - if (pName) { - names.add(pName); - } - } - for (let from of Object.keys(options.renamePackages)) { - names.add(from); - } - for (let name of Object.keys(options.activeAddons)) { - names.add(name); - } - return [...names]; -} diff --git a/packages/vite/src/esbuild-request.ts b/packages/vite/src/esbuild-request.ts index bdbd559c2..3a86fb9c9 100644 --- a/packages/vite/src/esbuild-request.ts +++ b/packages/vite/src/esbuild-request.ts @@ -14,7 +14,7 @@ export class EsBuildModuleRequest implements ModuleRequest { if (source && importer && source[0] !== '\0') { let fromFile = cleanUrl(importer); - return new EsBuildModuleRequest(source, fromFile, pluginData?.embroider?.meta, false); + return new EsBuildModuleRequest(source, fromFile, pluginData?.embroider?.meta, false, false); } } @@ -22,7 +22,8 @@ export class EsBuildModuleRequest implements ModuleRequest { readonly specifier: string, readonly fromFile: string, readonly meta: Record | undefined, - readonly isVirtual: boolean + readonly isVirtual: boolean, + readonly isNotFound: boolean ) {} get debugType() { @@ -30,19 +31,22 @@ export class EsBuildModuleRequest implements ModuleRequest { } alias(newSpecifier: string) { - return new EsBuildModuleRequest(newSpecifier, this.fromFile, this.meta, this.isVirtual) as this; + return new EsBuildModuleRequest(newSpecifier, this.fromFile, this.meta, this.isVirtual, false) as this; } rehome(newFromFile: string) { if (this.fromFile === newFromFile) { return this; } else { - return new EsBuildModuleRequest(this.specifier, newFromFile, this.meta, this.isVirtual) as this; + return new EsBuildModuleRequest(this.specifier, newFromFile, this.meta, this.isVirtual, false) as this; } } virtualize(filename: string) { - return new EsBuildModuleRequest(filename, this.fromFile, this.meta, true) as this; + return new EsBuildModuleRequest(filename, this.fromFile, this.meta, true, false) as this; } withMeta(meta: Record | undefined): this { - return new EsBuildModuleRequest(this.specifier, this.fromFile, meta, this.isVirtual) as this; + return new EsBuildModuleRequest(this.specifier, this.fromFile, meta, this.isVirtual, this.isNotFound) as this; + } + notFound(): this { + return new EsBuildModuleRequest(this.specifier, this.fromFile, this.meta, this.isVirtual, true) as this; } } diff --git a/packages/vite/src/esbuild-resolver.ts b/packages/vite/src/esbuild-resolver.ts index c95c5e274..8f3855770 100644 --- a/packages/vite/src/esbuild-resolver.ts +++ b/packages/vite/src/esbuild-resolver.ts @@ -112,6 +112,15 @@ function defaultResolve( result: { path: request.specifier, namespace: 'embroider' }, }; } + if (request.isNotFound) { + // todo: make sure this looks correct to users + return { + type: 'not_found', + err: { + errors: [{ text: `module not found ${request.specifier}` }], + }, + }; + } let result = await context.resolve(request.specifier, { importer: request.fromFile, resolveDir: dirname(request.fromFile), diff --git a/packages/vite/src/request.ts b/packages/vite/src/request.ts index d6a9c22c1..eb1ee3f9b 100644 --- a/packages/vite/src/request.ts +++ b/packages/vite/src/request.ts @@ -26,14 +26,15 @@ export class RollupModuleRequest implements ModuleRequest { // strip query params off the importer let fromFile = cleanUrl(nonVirtual); - return new RollupModuleRequest(source, fromFile, custom?.embroider?.meta); + return new RollupModuleRequest(source, fromFile, custom?.embroider?.meta, false); } } private constructor( readonly specifier: string, readonly fromFile: string, - readonly meta: Record | undefined + readonly meta: Record | undefined, + readonly isNotFound: boolean ) {} get debugType() { @@ -45,19 +46,22 @@ export class RollupModuleRequest implements ModuleRequest { } alias(newSpecifier: string) { - return new RollupModuleRequest(newSpecifier, this.fromFile, this.meta) as this; + return new RollupModuleRequest(newSpecifier, this.fromFile, this.meta, false) as this; } rehome(newFromFile: string) { if (this.fromFile === newFromFile) { return this; } else { - return new RollupModuleRequest(this.specifier, newFromFile, this.meta) as this; + return new RollupModuleRequest(this.specifier, newFromFile, this.meta, false) as this; } } virtualize(filename: string) { - return new RollupModuleRequest(virtualPrefix + filename, this.fromFile, this.meta) as this; + return new RollupModuleRequest(virtualPrefix + filename, this.fromFile, this.meta, false) as this; } withMeta(meta: Record | undefined): this { - return new RollupModuleRequest(this.specifier, this.fromFile, meta) as this; + return new RollupModuleRequest(this.specifier, this.fromFile, meta, this.isNotFound) as this; + } + notFound(): this { + return new RollupModuleRequest(this.specifier, this.fromFile, this.meta, true) as this; } } diff --git a/packages/vite/src/resolver.ts b/packages/vite/src/resolver.ts index 96c56ca96..89ff3f56e 100644 --- a/packages/vite/src/resolver.ts +++ b/packages/vite/src/resolver.ts @@ -43,6 +43,13 @@ function defaultResolve(context: PluginContext): ResolverFunction { - let request = WebpackModuleRequest.from(state, this.#babelLoaderPrefix, this.#appRoot); - if (!request) { - defaultResolve(state, callback); - return; - } + nmf.hooks.resolve.tapAsync( + { name: '@embroider/webpack', stage: 50 }, + (state: ExtendedResolveData, callback: CB) => { + let request = WebpackModuleRequest.from(state, this.#babelLoaderPrefix, this.#appRoot); + if (!request) { + defaultResolve(state, callback); + return; + } - this.#resolver.resolve(request, adaptedResolve).then( - resolution => { - switch (resolution.type) { - case 'not_found': - callback(resolution.err); - break; - case 'found': - callback(null, resolution.result); - break; - default: - throw assertNever(resolution); - } - }, - err => callback(err) - ); - }); + this.#resolver.resolve(request, adaptedResolve).then( + resolution => { + switch (resolution.type) { + case 'not_found': + callback(resolution.err); + break; + case 'found': + callback(null, resolution.result); + break; + default: + throw assertNever(resolution); + } + }, + err => callback(err) + ); + } + ); }); } } @@ -89,7 +92,14 @@ function getAdaptedResolve( ): ResolverFunction> { return function (request: WebpackModuleRequest): Promise> { return new Promise(resolve => { - defaultResolve(request.state, (err, value) => { + 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'; + resolve({ type: 'not_found', err }); + } + defaultResolve(request.toWebpackResolveData(), (err, value) => { if (err) { // unfortunately webpack doesn't let us distinguish between Not Found // and other unexpected exceptions here. @@ -102,88 +112,168 @@ function getAdaptedResolve( }; } +type ExtendedResolveData = ResolveData & { + contextInfo: ResolveData['contextInfo'] & { _embroiderMeta?: Record }; +}; + class WebpackModuleRequest implements ModuleRequest { - readonly specifier: string; - readonly fromFile: string; - readonly meta: Record | undefined; - - static from(state: any, babelLoaderPrefix: string, appRoot: string): WebpackModuleRequest | undefined { - // when the files emitted from our virtual-loader try to import things, - // those requests show in webpack as having no issuer. But we can see here - // which requests they are and adjust the issuer so they resolve things from - // the correct logical place. - if (!state.contextInfo?.issuer && Array.isArray(state.dependencies)) { + static from( + state: ExtendedResolveData, + babelLoaderPrefix: string, + appRoot: string + ): WebpackModuleRequest | undefined { + let specifier = state.request; + if ( + specifier.includes(virtualLoaderName) || // prevents recursion on requests we have already sent to our virtual loader + specifier.startsWith('!') // ignores internal webpack resolvers + ) { + return; + } + + let fromFile: string | undefined; + if (state.contextInfo.issuer) { + fromFile = state.contextInfo.issuer; + } else { + // when the files emitted from our virtual-loader try to import things, + // those requests show in webpack as having no issuer. But we can see here + // which requests they are and adjust the issuer so they resolve things from + // the correct logical place. for (let dep of state.dependencies) { - let match = virtualRequestPattern.exec(dep._parentModule?.userRequest); + let match = virtualRequestPattern.exec((dep as any)._parentModule?.userRequest); if (match) { - state.contextInfo.issuer = new URLSearchParams(match.groups!.query).get('f'); - state.context = dirname(state.contextInfo.issuer); + fromFile = new URLSearchParams(match.groups!.query).get('f')!; + break; } } } - - if ( - typeof state.request === 'string' && - typeof state.context === 'string' && - typeof state.contextInfo?.issuer === 'string' && - state.contextInfo.issuer !== '' && - !state.request.includes(virtualLoaderName) && // prevents recursion on requests we have already sent to our virtual loader - !state.request.startsWith('!') // ignores internal webpack resolvers - ) { - return new WebpackModuleRequest(babelLoaderPrefix, appRoot, state); + if (!fromFile) { + return; } + + return new WebpackModuleRequest( + babelLoaderPrefix, + appRoot, + specifier, + fromFile, + state.contextInfo._embroiderMeta, + false, + false, + state + ); } - constructor( + private constructor( private babelLoaderPrefix: string, private appRoot: string, - public state: { - request: string; - context: string; - contextInfo: { - issuer: string; - _embroiderMeta?: Record | undefined; - }; - }, - public isVirtual = false - ) { - // these get copied here because we mutate the underlying state as we - // convert one request into the next, and it seems better for debuggability - // if the fields on the previous request don't change when you make a new - // one (although it is true that only the newest one has a a valid `state` - // that can actually be handed back to webpack) - this.specifier = state.request; - this.fromFile = state.contextInfo.issuer; - this.meta = state.contextInfo._embroiderMeta ? { ...state.contextInfo._embroiderMeta } : undefined; - } + readonly specifier: string, + readonly fromFile: string, + readonly meta: Record | undefined, + readonly isVirtual: boolean, + readonly isNotFound: boolean, + private originalState: ExtendedResolveData + ) {} get debugType() { return 'webpack'; } + // Webpack mostly relies on mutation to adjust requests. We could create a + // whole new ResolveData instead, and that would allow defaultResolving to + // happen, but for the output of that process to actually affect the + // downstream code in Webpack we would still need to mutate the original + // ResolveData with the results (primarily the `createData`). So since we + // cannot avoid the mutation anyway, it seems best to do it earlier rather + // than later, so that everything from here forward is "normal". + // + // Technically a NormalModuleLoader `resolve` hook *can* directly return a + // Module, but that is not how the stock one works, and it would force us to + // copy more of Webpack's default behaviors into the inside of our hook. Like, + // we would need to invoke afterResolve, createModule, createModuleClass, etc, + // just like webpack does if we wanted to produce a Module directly. + // + // So the mutation strategy is much less intrusive, even though it means there + // is the risk of state leakage all over the place. + // + // We mitigate that risk by waiting until the last possible moment to apply + // our desired ModuleRequest fields to the ResolveData. This means that as + // 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(): ExtendedResolveData { + this.originalState.request = this.specifier; + this.originalState.context = dirname(this.fromFile); + this.originalState.contextInfo.issuer = this.fromFile; + this.originalState.contextInfo._embroiderMeta = this.meta; + return this.originalState; + } + alias(newSpecifier: string) { - this.state.request = newSpecifier; - return new WebpackModuleRequest(this.babelLoaderPrefix, this.appRoot, this.state) as this; + if (newSpecifier === this.specifier) { + return this; + } + return new WebpackModuleRequest( + this.babelLoaderPrefix, + this.appRoot, + newSpecifier, + this.fromFile, + this.meta, + this.isVirtual, + false, + this.originalState + ) as this; } rehome(newFromFile: string) { if (this.fromFile === newFromFile) { return this; - } else { - this.state.contextInfo.issuer = newFromFile; - this.state.context = dirname(newFromFile); - return new WebpackModuleRequest(this.babelLoaderPrefix, this.appRoot, this.state) as this; } + return new WebpackModuleRequest( + this.babelLoaderPrefix, + this.appRoot, + this.specifier, + newFromFile, + this.meta, + this.isVirtual, + false, + this.originalState + ) as this; } virtualize(filename: string) { let params = new URLSearchParams(); params.set('f', filename); params.set('a', this.appRoot); - let next = this.alias(`${this.babelLoaderPrefix}${virtualLoaderName}?${params.toString()}!`); - next.isVirtual = true; - return next; + return new WebpackModuleRequest( + this.babelLoaderPrefix, + this.appRoot, + `${this.babelLoaderPrefix}${virtualLoaderName}?${params.toString()}!`, + this.fromFile, + this.meta, + true, + false, + this.originalState + ) as this; } withMeta(meta: Record | undefined): this { - this.state.contextInfo._embroiderMeta = meta; - return new WebpackModuleRequest(this.babelLoaderPrefix, this.appRoot, this.state) as this; + return new WebpackModuleRequest( + this.babelLoaderPrefix, + this.appRoot, + this.specifier, + this.fromFile, + meta, + this.isVirtual, + this.isNotFound, + this.originalState + ) as this; + } + notFound(): this { + return new WebpackModuleRequest( + this.babelLoaderPrefix, + this.appRoot, + this.specifier, + this.fromFile, + this.meta, + this.isVirtual, + true, + this.originalState + ) as this; } } diff --git a/test-packages/support/suite-setup-util.ts b/test-packages/support/suite-setup-util.ts index 96f2f5c71..707a62f27 100644 --- a/test-packages/support/suite-setup-util.ts +++ b/test-packages/support/suite-setup-util.ts @@ -27,7 +27,7 @@ async function githubMatrix() { dir, })), ...suites - .filter(s => s.name !== 'node') // TODO: node tests do not work under windows yet + .filter(s => s.name !== 'jest-suites') // TODO: jest tests do not work under windows yet .filter(s => !s.name.includes('watch-mode')) // TODO: watch tests are far too slow on windows right now .map(s => ({ name: `${s.name} windows`, diff --git a/tests/scenarios/compat-resolver-test.ts b/tests/scenarios/compat-resolver-test.ts index d3ed4df4c..44fb2104e 100644 --- a/tests/scenarios/compat-resolver-test.ts +++ b/tests/scenarios/compat-resolver-test.ts @@ -71,7 +71,6 @@ Scenarios.fromProject(() => new Project()) let resolverOptions: CompatResolverOptions = { amdCompatibility: 'cjs', - activeAddons: {}, renameModules: {}, renamePackages: {}, resolvableExtensions: ['.js', '.hbs'], diff --git a/tests/scenarios/core-resolver-test.ts b/tests/scenarios/core-resolver-test.ts index b3df736d1..4ae36a477 100644 --- a/tests/scenarios/core-resolver-test.ts +++ b/tests/scenarios/core-resolver-test.ts @@ -72,6 +72,9 @@ Scenarios.fromProject(() => new Project()) name, keywords: ['ember-addon'], 'ember-addon': meta, + dependencies: { + 'ember-auto-import': '^2.0.0', + }, }; })(), null, @@ -91,7 +94,6 @@ Scenarios.fromProject(() => new Project()) configure = async function (opts?: ConfigureOpts) { let resolverOptions: CompatResolverOptions = { amdCompatibility: 'cjs', - activeAddons: {}, renameModules: {}, renamePackages: opts?.renamePackages ?? {}, resolvableExtensions: ['.js', '.hbs'], @@ -790,6 +792,7 @@ Scenarios.fromProject(() => new Project()) extraResolutions: {}, }; givenFiles({ + 'node_modules/my-addon/node_modules/inner-dep/package.json': '{ "name": "inner-dep" }', 'node_modules/my-addon/node_modules/inner-dep/index.js': '', 'node_modules/.embroider/rewritten-packages/index.json': JSON.stringify(index), 'node_modules/.embroider/rewritten-packages/my-addon.1234/node_modules/my-addon/hello-world.js': `import "inner-dep"`, @@ -809,16 +812,17 @@ Scenarios.fromProject(() => new Project()) test('implicit modules in moved dependencies', async function () { let index: RewrittenPackageIndex = { packages: { - [resolve(app.dir, 'node_modules/a-v1-addon')]: 'a-v1-addon.1234', + [resolve(app.dir, 'node_modules/a-v1-addon')]: 'a-v1-addon.1234/node_modules/a-v1-addon', }, extraResolutions: {}, }; givenFiles({ 'node_modules/.embroider/rewritten-packages/index.json': JSON.stringify(index), - 'node_modules/.embroider/rewritten-packages/a-v1-addon.1234/_app_/components/i-am-implicit.js': ``, - 'node_modules/.embroider/rewritten-packages/a-v1-addon.1234/package.json': addonPackageJSON('a-v1-addon', { - 'implicit-modules': ['./_app_/components/i-am-implicit.js'], - }), + 'node_modules/.embroider/rewritten-packages/a-v1-addon.1234/node_modules/a-v1-addon/_app_/components/i-am-implicit.js': ``, + 'node_modules/.embroider/rewritten-packages/a-v1-addon.1234/node_modules/a-v1-addon/package.json': + addonPackageJSON('a-v1-addon', { + 'implicit-modules': ['./_app_/components/i-am-implicit.js'], + }), 'app.js': `import "./-embroider-implicit-modules.js"`, }); @@ -828,7 +832,10 @@ Scenarios.fromProject(() => new Project()) .module('./app.js') .resolves('./-embroider-implicit-modules.js') .toModule() - .resolves('a-v1-addon/-embroider-implicit-modules.js'); + .resolves('a-v1-addon/_app_/components/i-am-implicit.js') + .to( + './node_modules/.embroider/rewritten-packages/a-v1-addon.1234/node_modules/a-v1-addon/_app_/components/i-am-implicit.js' + ); }); }); }); diff --git a/tests/scenarios/jest-suites-test.ts b/tests/scenarios/jest-suites-test.ts index 1da49b969..d5b0fc6a5 100644 --- a/tests/scenarios/jest-suites-test.ts +++ b/tests/scenarios/jest-suites-test.ts @@ -7,7 +7,7 @@ const { module: Qmodule, test } = QUnit; // this is the bridge between our older Jest-based node tests and our newer // scenario-tester powered tests Scenarios.fromProject(() => new Project('node-tests')) - .map('node', () => {}) + .map('jest-suites', () => {}) .forEachScenario(scenario => { Qmodule(scenario.name, function () { test('run node tests', async function (assert) {