From 7d2f862f4fd6b00a97763e08a31ac9c73332b09a Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 15 Jul 2023 17:07:45 -0400 Subject: [PATCH 1/5] Generate per-package implicit-modules imports Previously, all implicit-modules got imported by their containing engine (normally the app). This makes some sense, because they are getting AMD-defined into one flat namespace anyway for compatibility reasons. If there are multiple versions of a package in the transitive deps, only one can be present in the AMD loader. However, this assumes there will be resolvability from the app all the way to the transitive deps, which is not reliably true. And it also means that if there is a mismatch between the list of implicit-modules provided by different copies of a package you could run into failures where you try to find them in the wrong copy. So instead, this PR moves the importing of implicit modules into per-package virtual modules. Each package imports its own direct dependencies' implicit modules, as well as the virtual implicit-module entrypoints for those dependencies, so the system recurses. --- .../compat/src/babel-plugin-adjust-imports.ts | 2 +- packages/compat/src/compat-app-builder.ts | 76 ++-------- packages/compat/src/dependency-rules.ts | 2 +- packages/core/src/index.ts | 1 + packages/core/src/module-resolver.ts | 56 +++++-- packages/core/src/resolver-loader.ts | 32 ++++ packages/core/src/virtual-content.ts | 139 +++++++++++++++++- packages/shared-internals/src/package.ts | 18 +++ .../src/rewritten-package-cache.ts | 4 + packages/webpack/src/virtual-loader.ts | 21 ++- .../webpack/src/webpack-resolver-plugin.ts | 22 ++- 11 files changed, 278 insertions(+), 95 deletions(-) create mode 100644 packages/core/src/resolver-loader.ts diff --git a/packages/compat/src/babel-plugin-adjust-imports.ts b/packages/compat/src/babel-plugin-adjust-imports.ts index 1c527d4d3..172f50196 100644 --- a/packages/compat/src/babel-plugin-adjust-imports.ts +++ b/packages/compat/src/babel-plugin-adjust-imports.ts @@ -182,7 +182,7 @@ function lazyPackageLookup(config: InternalConfig, filename: string) { return { get owningPackage() { if (!owningPackage) { - owningPackage = { result: config.resolver.owningPackage(filename) }; + owningPackage = { result: config.resolver.packageCache.ownerOfFile(filename) }; } return owningPackage.result; }, diff --git a/packages/compat/src/compat-app-builder.ts b/packages/compat/src/compat-app-builder.ts index 995131580..4b13b2056 100644 --- a/packages/compat/src/compat-app-builder.ts +++ b/packages/compat/src/compat-app-builder.ts @@ -43,7 +43,7 @@ import { AppFiles, RouteFiles } from '@embroider/core/src/app-files'; import { PortableHint, maybeNodeModuleVersion } from '@embroider/core/src/portable'; import assertNever from 'assert-never'; import { Memoize } from 'typescript-memoize'; -import { join, dirname, sep } from 'path'; +import { join, dirname } from 'path'; import resolve from 'resolve'; import { V1Config } from './v1-config'; import { AddonMeta, Package, PackageInfo } from '@embroider/core'; @@ -140,9 +140,9 @@ export class CompatAppBuilder { let result = (pkg.dependencies.filter(this.isActiveAddon) as AddonPackage[]).filter( // When looking for child addons, we want to ignore 'peerDependencies' of // a given package, to align with how ember-cli resolves addons. So here - // we only include dependencies that definitely appear in one of the other - // sections. - addon => pkg.packageJSON.dependencies?.[addon.name] || pkg.packageJSON.devDependencies?.[addon.name] + // we only include dependencies that are definitely active due to one of + // the other sections. + addon => pkg.categorizeDependency(addon.name) !== 'peerDependencies' ); if (pkg === this.appPackageWithMovedDeps) { let extras = [this.synthVendor, this.synthStyles].filter(this.isActiveAddon) as AddonPackage[]; @@ -703,10 +703,10 @@ export class CompatAppBuilder { private engines: { engine: Engine; appSync: SyncDir; fastbootSync: SyncDir | undefined }[] | undefined; - private updateAppJS(inputPaths: OutputPaths): AppFiles[] { + private updateAppJS(appJSPath: string): AppFiles[] { if (!this.engines) { - this.engines = this.partitionEngines(inputPaths.appJS).map(engine => { - if (engine.sourcePath === inputPaths.appJS) { + this.engines = this.partitionEngines(appJSPath).map(engine => { + if (engine.sourcePath === appJSPath) { // this is the app. We have more to do for the app than for other // engines. let fastbootSync: SyncDir | undefined; @@ -718,7 +718,7 @@ export class CompatAppBuilder { } return { engine, - appSync: new SyncDir(inputPaths.appJS, this.root), + appSync: new SyncDir(appJSPath, this.root), fastbootSync, }; } else { @@ -938,7 +938,7 @@ export class CompatAppBuilder { this.firstBuild = false; } - let appFiles = this.updateAppJS(inputPaths); + let appFiles = this.updateAppJS(inputPaths.appJS); let emberENV = this.configTree.readConfig().EmberENV; let assets = this.gatherAssets(inputPaths); @@ -1192,7 +1192,7 @@ export class CompatAppBuilder { return cached; } - let eagerModules = []; + let eagerModules: string[] = []; let requiredAppFiles = [this.requiredOtherFiles(appFiles)]; if (!this.options.staticComponents) { @@ -1272,7 +1272,7 @@ export class CompatAppBuilder { // this is a backward-compatibility feature: addons can force inclusion of // modules. - this.gatherImplicitModules('implicit-modules', appFiles, amdModules); + eagerModules.push('./#embroider-implicit-modules'); let params = { amdModules, fastbootOnlyAmdModules, lazyRoutes, lazyEngines, eagerModules, styles }; if (entryParams) { @@ -1337,7 +1337,7 @@ export class CompatAppBuilder { let amdModules: { runtime: string; buildtime: string }[] = []; // this is a backward-compatibility feature: addons can force inclusion of // test support modules. - this.gatherImplicitModules('implicit-test-modules', engine, amdModules); + eagerModules.push('./#embroider-implicit-test-modules'); for (let relativePath of engine.tests) { amdModules.push(this.importPaths(engine, relativePath)); @@ -1357,44 +1357,6 @@ export class CompatAppBuilder { prepared.set(asset.relativePath, asset); return asset; } - - private gatherImplicitModules( - section: 'implicit-modules' | 'implicit-test-modules', - { engine }: AppFiles, - lazyModules: { runtime: string; buildtime: string }[] - ) { - for (let addon of engine.addons) { - let implicitModules = addon.meta[section]; - if (implicitModules) { - let renamedModules = inverseRenamedModules(addon.meta, this.resolvableExtensionsPattern); - for (let name of implicitModules) { - let packageName = addon.name; - - if (addon.isV2Addon()) { - let renamedMeta = addon.meta['renamed-packages']; - if (renamedMeta) { - Object.entries(renamedMeta).forEach(([key, value]) => { - if (value === addon!.name) { - packageName = key; - } - }); - } - } - - let runtime = join(packageName, name).replace(this.resolvableExtensionsPattern, ''); - let runtimeRenameLookup = runtime.split('\\').join('/'); - if (renamedModules && renamedModules[runtimeRenameLookup]) { - runtime = renamedModules[runtimeRenameLookup]; - } - runtime = runtime.split(sep).join('/'); - lazyModules.push({ - runtime, - buildtime: posix.join(packageName, name), - }); - } - } - } - } } function maybeReplace(dom: JSDOM, element: Element | undefined): Node | undefined { @@ -1541,20 +1503,6 @@ const { babelFilter } = require(${JSON.stringify(require.resolve('@embroider/cor module.exports = babelFilter({{json-stringify skipBabel}}, "{{js-string-escape appRoot}}"); `) as (params: { skipBabel: Options['skipBabel']; appRoot: string }) => string; -// meta['renamed-modules'] has mapping from classic filename to real filename. -// This takes that and converts it to the inverst mapping from real import path -// to classic import path. -function inverseRenamedModules(meta: AddonPackage['meta'], extensions: RegExp) { - let renamed = meta['renamed-modules']; - if (renamed) { - let inverted = {} as { [name: string]: string }; - for (let [classic, real] of Object.entries(renamed)) { - inverted[real.replace(extensions, '')] = classic.replace(extensions, ''); - } - return inverted; - } -} - function combinePackageJSON(...layers: object[]) { function custom(objValue: any, srcValue: any, key: string, _object: any, _source: any, stack: { size: number }) { if (key === 'keywords' && stack.size === 0) { diff --git a/packages/compat/src/dependency-rules.ts b/packages/compat/src/dependency-rules.ts index 9346dff52..27225552c 100644 --- a/packages/compat/src/dependency-rules.ts +++ b/packages/compat/src/dependency-rules.ts @@ -230,7 +230,7 @@ export function activePackageRules( } export function appTreeRulesDir(root: string, resolver: Resolver) { - let pkg = resolver.owningPackage(root); + let pkg = resolver.packageCache.ownerOfFile(root); if (pkg?.isV2Addon()) { // in general v2 addons can keep their app tree stuff in other places than // "_app_" and we would need to check their package.json to see. But this code diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 58deed289..e22229fe1 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -22,6 +22,7 @@ export { ResolverFunction, SyncResolverFunction, } from './module-resolver'; +export { ResolverLoader } from './resolver-loader'; export { virtualContent } from './virtual-content'; export type { Engine } from './app-files'; diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 4a2b8938a..2ab947db4 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -16,6 +16,7 @@ import { virtualContent, fastbootSwitch, decodeFastbootSwitch, + decodeImplicitModules, } from './virtual-content'; import { Memoize } from 'typescript-memoize'; import { describeExports } from './describe-exports'; @@ -144,7 +145,7 @@ export type SyncResolverFunction Res; export class Resolver { - constructor(private options: Options) {} + constructor(readonly options: Options) {} beforeResolve(request: R): R { if (request.specifier === '@embroider/macros') { @@ -157,6 +158,7 @@ export class Resolver { request = this.handleFastbootSwitch(request); request = this.handleGlobalsCompat(request); + request = this.handleImplicitModules(request); request = this.handleRenaming(request); // we expect the specifier to be app relative at this point - must be after handleRenaming request = this.generateFastbootSwitch(request); @@ -251,7 +253,7 @@ export class Resolver { type: 'found', result: { type: 'virtual' as 'virtual', - content: virtualContent(request.specifier), + content: virtualContent(request.specifier, this), filename: request.specifier, }, }; @@ -279,14 +281,10 @@ export class Resolver { } } - private get packageCache() { + get packageCache() { return RewrittenPackageCache.shared('embroider', this.options.appRoot); } - owningPackage(fromFile: string): Package | undefined { - return this.packageCache.ownerOfFile(fromFile); - } - private logicalPackage(owningPackage: V2Package, file: string): V2Package { let logicalLocation = this.reverseSearchAppTree(owningPackage, file); if (logicalLocation) { @@ -300,7 +298,7 @@ export class Resolver { } private generateFastbootSwitch(request: R): R { - let pkg = this.owningPackage(request.fromFile); + let pkg = this.packageCache.ownerOfFile(request.fromFile); if (!pkg) { return request; @@ -358,7 +356,7 @@ export class Resolver { return logTransition('non-special import in fastboot switch', request); } - let pkg = this.owningPackage(match.filename); + let pkg = this.packageCache.ownerOfFile(match.filename); if (pkg) { let rel = explicitRelative(pkg.root, match.filename); @@ -397,13 +395,41 @@ export class Resolver { return logTransition('failed to match in fastboot switch', request); } + private handleImplicitModules(request: R): R { + let im = decodeImplicitModules(request.specifier); + if (!im) { + return request; + } + + let pkg = this.packageCache.ownerOfFile(request.fromFile); + if (!pkg?.isV2Ember()) { + throw new Error(`bug: found implicit modules import in non-ember package at ${request.fromFile}`); + } + + let packageName = getPackageName(im.fromFile); + if (packageName) { + let dep = this.packageCache.resolve(packageName, pkg); + return logTransition( + `dep's implicit modules`, + request, + request.virtualize(resolve(dep.root, `#embroider-${im.type}`)) + ); + } else { + return logTransition( + `own implicit modules`, + request, + request.virtualize(resolve(pkg.root, `#embroider-${im.type}`)) + ); + } + } + private handleGlobalsCompat(request: R): R { let match = compatPattern.exec(request.specifier); if (!match) { return request; } let { type, rest } = match.groups!; - let fromPkg = this.owningPackage(request.fromFile); + let fromPkg = this.packageCache.ownerOfFile(request.fromFile); if (!fromPkg?.isV2Ember()) { return request; } @@ -677,7 +703,7 @@ export class Resolver { } private handleRewrittenPackages(request: R): R { - let requestingPkg = this.owningPackage(request.fromFile); + let requestingPkg = this.packageCache.ownerOfFile(request.fromFile); if (!requestingPkg) { return request; } @@ -734,7 +760,7 @@ export class Resolver { return request; } - let pkg = this.owningPackage(request.fromFile); + let pkg = this.packageCache.ownerOfFile(request.fromFile); if (!pkg || !pkg.isV2Ember()) { return request; } @@ -795,7 +821,7 @@ export class Resolver { return request; } let { specifier, fromFile } = request; - let pkg = this.owningPackage(fromFile); + let pkg = this.packageCache.ownerOfFile(fromFile); if (!pkg || !pkg.isV2Ember()) { return request; } @@ -916,7 +942,7 @@ export class Resolver { return request; } - let pkg = this.owningPackage(fromFile); + let pkg = this.packageCache.ownerOfFile(fromFile); if (!pkg) { return logTransition('no identifiable owningPackage', request); } @@ -1116,7 +1142,7 @@ export class Resolver { // check if this file is resolvable as a global component, and if so return // its dasherized name reverseComponentLookup(filename: string): string | undefined { - const owningPackage = this.owningPackage(filename); + const owningPackage = this.packageCache.ownerOfFile(filename); if (!owningPackage?.isV2Ember()) { return; } diff --git a/packages/core/src/resolver-loader.ts b/packages/core/src/resolver-loader.ts new file mode 100644 index 000000000..137873bfc --- /dev/null +++ b/packages/core/src/resolver-loader.ts @@ -0,0 +1,32 @@ +import { readJSONSync } from 'fs-extra'; +import { Resolver, Options } from './module-resolver'; +import { locateEmbroiderWorkingDir } from '@embroider/shared-internals'; +import { join } from 'path'; +import { watch as fsWatch, FSWatcher } from 'fs'; + +export class ResolverLoader { + #resolver: Resolver | undefined; + #configFile: string; + #watcher: FSWatcher | undefined; + + constructor(readonly appRoot: string, watch = false) { + this.#configFile = join(locateEmbroiderWorkingDir(this.appRoot), 'resolver.json'); + if (watch) { + this.#watcher = fsWatch(this.#configFile, { persistent: false }, () => { + this.#resolver = undefined; + }); + } + } + + close() { + this.#watcher?.close(); + } + + get resolver(): Resolver { + if (!this.#resolver) { + let config: Options = readJSONSync(join(locateEmbroiderWorkingDir(this.appRoot), 'resolver.json')); + this.#resolver = new Resolver(config); + } + return this.#resolver; + } +} diff --git a/packages/core/src/virtual-content.ts b/packages/core/src/virtual-content.ts index e4f0979ec..66e6ff420 100644 --- a/packages/core/src/virtual-content.ts +++ b/packages/core/src/virtual-content.ts @@ -1,5 +1,5 @@ -import { dirname, basename, resolve } from 'path'; -import { explicitRelative } from '.'; +import { dirname, basename, resolve, posix, sep, join } from 'path'; +import { Resolver, explicitRelative, extensionsPattern, AddonPackage, Package } from '.'; import { compile } from './js-handlebars'; const externalPrefix = '/@embroider/external/'; @@ -8,7 +8,7 @@ const externalPrefix = '/@embroider/external/'; // this produces the corresponding contents. It's a static, stateless function // because we recognize that that process that did resolution might not be the // same one that loads the content. -export function virtualContent(filename: string): string { +export function virtualContent(filename: string, resolver: Resolver): string { if (filename.startsWith(externalPrefix)) { return externalShim({ moduleName: filename.slice(externalPrefix.length) }); } @@ -22,6 +22,11 @@ export function virtualContent(filename: string): string { return fastbootSwitchTemplate(fb); } + let im = decodeImplicitModules(filename); + if (im) { + return renderImplicitModules(im, resolver); + } + throw new Error(`not an @embroider/core virtual file: ${filename}`); } @@ -133,3 +138,131 @@ export default mod.default; export const {{name}} = mod.{{name}}; {{/each}} `) as (params: { names: string[]; hasDefaultExport: boolean }) => string; + +export function decodeImplicitModules( + filename: string +): { type: 'implicit-modules' | 'implicit-test-modules'; fromFile: string } | undefined { + if (filename.endsWith('/#embroider-implicit-modules')) { + return { + type: 'implicit-modules', + fromFile: filename.slice(0, -1 * '/#embroider-implicit-modules'.length), + }; + } + if (filename.endsWith('/#embroider-implicit-test-modules')) { + return { + type: 'implicit-test-modules', + fromFile: filename.slice(0, -1 * '/#embroider-implicit-test-modules'.length), + }; + } +} + +function renderImplicitModules( + { + type, + fromFile, + }: { + type: 'implicit-modules' | 'implicit-test-modules'; + fromFile: string; + }, + resolver: Resolver +): string { + let resolvableExtensionsPattern = extensionsPattern(resolver.options.resolvableExtensions); + + const pkg = resolver.packageCache.ownerOfFile(fromFile); + if (!pkg?.isV2Ember()) { + throw new Error(`bug: saw special implicit modules import in non-ember package at ${fromFile}`); + } + + let lazyModules: { runtime: string; buildtime: string }[] = []; + let eagerModules: string[] = []; + + let deps = pkg.dependencies.sort(orderAddons); + + for (let dep of deps) { + // anything that isn't a v2 ember package by this point is not an active + // addon. + if (!dep.isV2Addon()) { + continue; + } + + // we ignore peerDependencies here because classic ember-cli ignores + // peerDependencies here, and we're implementing the implicit-modules + // backward-comptibility feature. + if (pkg.categorizeDependency(dep.name) === 'peerDependencies') { + continue; + } + + let implicitModules = dep.meta[type]; + if (implicitModules) { + let renamedModules = inverseRenamedModules(dep.meta, resolvableExtensionsPattern); + for (let name of implicitModules) { + let packageName = dep.name; + + let renamedMeta = dep.meta['renamed-packages']; + if (renamedMeta) { + Object.entries(renamedMeta).forEach(([key, value]) => { + if (value === dep.name) { + packageName = key; + } + }); + } + + let runtime = join(packageName, name).replace(resolvableExtensionsPattern, ''); + let runtimeRenameLookup = runtime.split('\\').join('/'); + if (renamedModules && renamedModules[runtimeRenameLookup]) { + runtime = renamedModules[runtimeRenameLookup]; + } + runtime = runtime.split(sep).join('/'); + lazyModules.push({ + runtime, + buildtime: posix.join(packageName, name), + }); + } + } + // we don't recurse across an engine boundary. Engines import their own + // implicit-modules. + if (!dep.isEngine()) { + eagerModules.push(posix.join(dep.name, `#embroider-${type}`)); + } + } + return implicitModulesTemplate({ lazyModules, eagerModules }); +} + +const implicitModulesTemplate = compile(` +import { importSync as i } from '@embroider/macros'; +let d = window.define; +{{#each lazyModules as |module|}} +d("{{js-string-escape module.runtime}}", function(){ return i("{{js-string-escape module.buildtime}}");}); +{{/each}} +{{#each eagerModules as |module|}} +import "{{js-string-escape module}}"; +{{/each}} +`) as (params: { eagerModules: string[]; lazyModules: { runtime: string; buildtime: string }[] }) => string; + +// meta['renamed-modules'] has mapping from classic filename to real filename. +// This takes that and converts it to the inverst mapping from real import path +// to classic import path. +function inverseRenamedModules(meta: AddonPackage['meta'], extensions: RegExp) { + let renamed = meta['renamed-modules']; + if (renamed) { + let inverted = {} as { [name: string]: string }; + for (let [classic, real] of Object.entries(renamed)) { + inverted[real.replace(extensions, '')] = classic.replace(extensions, ''); + } + return inverted; + } +} + +function orderAddons(depA: Package, depB: Package): number { + let depAIdx = 0; + let depBIdx = 0; + + if (depA && depA.meta && depA.isV2Addon()) { + depAIdx = depA.meta['order-index'] || 0; + } + if (depB && depB.meta && depB.isV2Addon()) { + depBIdx = depB.meta['order-index'] || 0; + } + + return depAIdx - depBIdx; +} diff --git a/packages/shared-internals/src/package.ts b/packages/shared-internals/src/package.ts index ed463a6dc..be3cd5109 100644 --- a/packages/shared-internals/src/package.ts +++ b/packages/shared-internals/src/package.ts @@ -6,6 +6,7 @@ import { AddonMeta, AppMeta, PackageInfo } from './metadata'; import PackageCache from './package-cache'; import flatMap from 'lodash/flatMap'; export default class Package { + // order here matters because we rely on it in categorizeDependency private dependencyKeys: ('dependencies' | 'devDependencies' | 'peerDependencies')[]; constructor(readonly root: string, protected packageCache: PackageCache, private isApp: boolean) { @@ -166,6 +167,23 @@ export default class Package { return flatMap(this.dependencyKeys, key => Object.keys(this.packageJSON[key] || {})); } + // this answers the question, "why is this thing in `this.dependencies`?". The + // order of the dependency keys is important, because "dependencies" always + // dominates (if it's your dependency, it's always your dependency). Next + // comes devDependencies, so that if you're the topmost package, your + // devDependencies are active dependenceis. Last comes peerDependencies. It's + // common for a peerDep to also appear in devDependencies, but unless your + // pacakge is the topmost your devDependencies won't be included in + // this.dependencyKeys, so we'll see that you are now getting the package via + // peerDeps. + categorizeDependency(name: string) { + for (let key of this.dependencyKeys) { + if (this.packageJSON[key]?.[name]) { + return key; + } + } + } + @Memoize() get dependencies(): Package[] { return this.dependencyNames diff --git a/packages/shared-internals/src/rewritten-package-cache.ts b/packages/shared-internals/src/rewritten-package-cache.ts index d2a4971e2..0f72f61e3 100644 --- a/packages/shared-internals/src/rewritten-package-cache.ts +++ b/packages/shared-internals/src/rewritten-package-cache.ts @@ -324,4 +324,8 @@ class WrappedPackage implements PackageTheGoodParts { // package.json.ß return this.plainPkg.hasDependency(name); } + + categorizeDependency(name: string): 'dependencies' | 'devDependencies' | 'peerDependencies' | undefined { + return this.plainPkg.categorizeDependency(name); + } } diff --git a/packages/webpack/src/virtual-loader.ts b/packages/webpack/src/virtual-loader.ts index d42ea7149..8b55478ec 100644 --- a/packages/webpack/src/virtual-loader.ts +++ b/packages/webpack/src/virtual-loader.ts @@ -1,11 +1,26 @@ -import { virtualContent } from '@embroider/core'; +import { ResolverLoader, virtualContent } from '@embroider/core'; import { LoaderContext } from 'webpack'; +let resolverLoader: ResolverLoader | undefined; + +function setup(appRoot: string): ResolverLoader { + if (resolverLoader?.appRoot !== appRoot) { + resolverLoader = new ResolverLoader(appRoot); + } + return resolverLoader; +} + export default function virtualLoader(this: LoaderContext) { if (typeof this.query === 'string' && this.query[0] === '?') { - let filename = this.query.slice(1); + let params = new URLSearchParams(this.query); + let filename = params.get('f'); + let appRoot = params.get('a'); + if (!filename || !appRoot) { + throw new Error(`bug in @embroider/webpack virtual loader, cannot locate params in ${this.query}`); + } + let { resolver } = setup(appRoot); this.resourcePath = filename; - return virtualContent(filename); + return virtualContent(filename, resolver); } throw new Error(`@embroider/webpack/src/virtual-loader received unexpected request: ${this.query}`); } diff --git a/packages/webpack/src/webpack-resolver-plugin.ts b/packages/webpack/src/webpack-resolver-plugin.ts index 20db3e276..369ec99e5 100644 --- a/packages/webpack/src/webpack-resolver-plugin.ts +++ b/packages/webpack/src/webpack-resolver-plugin.ts @@ -14,15 +14,17 @@ export { EmbroiderResolverOptions as Options }; const virtualLoaderName = '@embroider/webpack/src/virtual-loader'; const virtualLoaderPath = resolve(__dirname, './virtual-loader.js'); -const virtualRequestPattern = new RegExp(`${escapeRegExp(virtualLoaderPath)}\\?(?.+)!`); +const virtualRequestPattern = new RegExp(`${escapeRegExp(virtualLoaderPath)}\\?(?.+)!`); export class EmbroiderPlugin { #resolver: EmbroiderResolver; #babelLoaderPrefix: string; + #appRoot: string; constructor(opts: EmbroiderResolverOptions, babelLoaderPrefix: string) { this.#resolver = new EmbroiderResolver(opts); this.#babelLoaderPrefix = babelLoaderPrefix; + this.#appRoot = opts.appRoot; } #addLoaderAlias(compiler: Compiler, name: string, alias: string) { @@ -46,7 +48,7 @@ export class EmbroiderPlugin { let adaptedResolve = getAdaptedResolve(defaultResolve); nmf.hooks.resolve.tapAsync({ name: '@embroider/webpack', stage: 50 }, (state: unknown, callback: CB) => { - let request = WebpackModuleRequest.from(state, this.#babelLoaderPrefix); + let request = WebpackModuleRequest.from(state, this.#babelLoaderPrefix, this.#appRoot); if (!request) { defaultResolve(state, callback); return; @@ -109,7 +111,7 @@ class WebpackModuleRequest implements ModuleRequest { specifier: string; fromFile: string; - static from(state: any, babelLoaderPrefix: string): WebpackModuleRequest | 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 @@ -118,7 +120,7 @@ class WebpackModuleRequest implements ModuleRequest { for (let dep of state.dependencies) { let match = virtualRequestPattern.exec(dep._parentModule?.userRequest); if (match) { - state.contextInfo.issuer = match.groups!.filename; + state.contextInfo.issuer = new URLSearchParams(match.groups!.query).get('f'); state.context = dirname(state.contextInfo.issuer); } } @@ -132,12 +134,13 @@ class WebpackModuleRequest implements ModuleRequest { !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, state); + return new WebpackModuleRequest(babelLoaderPrefix, appRoot, state); } } constructor( private babelLoaderPrefix: string, + private appRoot: string, public state: { request: string; context: string; @@ -158,7 +161,7 @@ class WebpackModuleRequest implements ModuleRequest { alias(newSpecifier: string) { this.state.request = newSpecifier; - return new WebpackModuleRequest(this.babelLoaderPrefix, this.state) as this; + return new WebpackModuleRequest(this.babelLoaderPrefix, this.appRoot, this.state) as this; } rehome(newFromFile: string) { if (this.fromFile === newFromFile) { @@ -166,11 +169,14 @@ class WebpackModuleRequest implements ModuleRequest { } else { this.state.contextInfo.issuer = newFromFile; this.state.context = dirname(newFromFile); - return new WebpackModuleRequest(this.babelLoaderPrefix, this.state) as this; + return new WebpackModuleRequest(this.babelLoaderPrefix, this.appRoot, this.state) as this; } } virtualize(filename: string) { - let next = this.alias(`${this.babelLoaderPrefix}${virtualLoaderName}?${filename}!`); + 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; } From 28315be5b9f396145c99e4bfadbddeb2bce96fbb Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 15 Jul 2023 17:57:08 -0400 Subject: [PATCH 2/5] attempting to fix matching on windows --- packages/core/src/virtual-content.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/core/src/virtual-content.ts b/packages/core/src/virtual-content.ts index 66e6ff420..4ed1b2b6b 100644 --- a/packages/core/src/virtual-content.ts +++ b/packages/core/src/virtual-content.ts @@ -139,19 +139,16 @@ export const {{name}} = mod.{{name}}; {{/each}} `) as (params: { names: string[]; hasDefaultExport: boolean }) => string; +const implicitModulesPattern = /(?.*)[\\/]#embroider-implicit-(?test-)?modules$/; + export function decodeImplicitModules( filename: string ): { type: 'implicit-modules' | 'implicit-test-modules'; fromFile: string } | undefined { - if (filename.endsWith('/#embroider-implicit-modules')) { - return { - type: 'implicit-modules', - fromFile: filename.slice(0, -1 * '/#embroider-implicit-modules'.length), - }; - } - if (filename.endsWith('/#embroider-implicit-test-modules')) { + let m = implicitModulesPattern.exec(filename); + if (m) { return { - type: 'implicit-test-modules', - fromFile: filename.slice(0, -1 * '/#embroider-implicit-test-modules'.length), + type: m.groups!.test ? 'implicit-test-modules' : 'implicit-modules', + fromFile: m.groups!.filename, }; } } From 8c94dcb54dd43984606315cd3b59c4a0f483f090 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 15 Jul 2023 18:30:44 -0400 Subject: [PATCH 3/5] updating tests --- test-packages/support/audit-assertions.ts | 8 +++++++ tests/scenarios/compat-namespaced-app-test.ts | 22 +++++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/test-packages/support/audit-assertions.ts b/test-packages/support/audit-assertions.ts index cbf5c1a5d..4317895f3 100644 --- a/test-packages/support/audit-assertions.ts +++ b/test-packages/support/audit-assertions.ts @@ -116,6 +116,14 @@ export class ExpectModule { this.expectAudit.assert.codeEqual(this.module.content, expectedSource); } + codeContains(expectedSource: string) { + if (!this.module) { + this.emitMissingModule(); + return; + } + this.expectAudit.assert.codeContains(this.module.content, expectedSource); + } + resolves(specifier: string): PublicAPI { if (!this.module) { this.emitMissingModule(); diff --git a/tests/scenarios/compat-namespaced-app-test.ts b/tests/scenarios/compat-namespaced-app-test.ts index 12e986797..24e05ef94 100644 --- a/tests/scenarios/compat-namespaced-app-test.ts +++ b/tests/scenarios/compat-namespaced-app-test.ts @@ -6,6 +6,7 @@ import QUnit from 'qunit'; const { module: Qmodule, test } = QUnit; import { ExpectFile, expectFilesAt } from '@embroider/test-support/file-assertions/qunit'; import { throwOnWarnings } from '@embroider/core'; +import { setupAuditTest } from '@embroider/test-support/audit-assertions'; appScenarios .map('compat-namespaced-app', app => { @@ -35,6 +36,8 @@ appScenarios assert.equal(result.exitCode, 0, result.output); }); + let expectAudit = setupAuditTest(hooks, () => ({ app: app.dir })); + hooks.beforeEach(assert => { expectFile = expectFilesAt(readFileSync(join(app.dir, 'dist/.stage2-output'), 'utf8'), { qunit: assert }); }); @@ -43,15 +46,16 @@ appScenarios }); test(`imports within app js`, function () { - let assertFile = expectFile('assets/@ef4/namespaced-app.js'); - assertFile.matches( - /d\(["'"]my-addon\/my-implicit-module["'], function\(\)\{ return i\(["']my-addon\/my-implicit-module\.js["']\);/, - 'implicit-modules have correct paths' - ); - assertFile.matches( - /d\(["']@ef4\/namespaced-app\/app['"], function\(\)\{ return i\(['"]@ef4\/namespaced-app\/app\.js"\);\}\);/, - `app's own modules are correct` - ); + expectAudit + .module('assets/@ef4/namespaced-app.js') + .resolves('./#embroider-implicit-modules') + .toModule() + .resolves('my-addon/my-implicit-module.js') + .to('./node_modules/my-addon/my-implicit-module.js'); + + expectAudit.module('assets/@ef4/namespaced-app.js').codeContains(` + d('@ef4/namespaced-app/app', function(){ return i('@ef4/namespaced-app/app.js');}); + `); }); test(`app css location`, function () { From cad4542af4fb17c555610c2439e205a2d8e91e7c Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 15 Jul 2023 18:42:34 -0400 Subject: [PATCH 4/5] fixing type error --- test-packages/support/audit-assertions.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test-packages/support/audit-assertions.ts b/test-packages/support/audit-assertions.ts index 4317895f3..bae25c125 100644 --- a/test-packages/support/audit-assertions.ts +++ b/test-packages/support/audit-assertions.ts @@ -210,6 +210,8 @@ type PublicAPI = { [K in keyof T]: T[K] }; class EmptyExpectModule implements PublicAPI { doesNotExist() {} codeEquals() {} + codeContains() {} + resolves(): PublicAPI { return new EmptyExpectResolution() as PublicAPI; } From 5811386002beb57dc18c4ef754d63c8ac709e587 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Sat, 15 Jul 2023 19:09:52 -0400 Subject: [PATCH 5/5] updating another test --- test-packages/support/index.ts | 9 --------- tests/scenarios/compat-renaming-test.ts | 13 +++++-------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/test-packages/support/index.ts b/test-packages/support/index.ts index 79f88e44e..1710ad8d5 100644 --- a/test-packages/support/index.ts +++ b/test-packages/support/index.ts @@ -1,7 +1,6 @@ import { join } from 'path'; import 'jest'; import { transform as transform7, TransformOptions as Options7 } from '@babel/core'; -import escapeRegExp from 'lodash/escapeRegExp'; import { createContext, Script } from 'vm'; interface RunDefaultOptions { @@ -102,14 +101,6 @@ export function emberTemplateCompiler() { }; } -export function definesPattern(runtimeName: string, buildTimeName: string): RegExp { - runtimeName = escapeRegExp(runtimeName); - buildTimeName = escapeRegExp(buildTimeName); - return new RegExp( - `d\\(['"]${runtimeName}['"], *function *\\(\\) *\\{[\\s\\n]*return i\\(['"]${buildTimeName}['"]\\);?[\\s\\n]*\\}\\)` - ); -} - export { ExpectFile } from './file-assertions'; export { Rebuilder } from './rebuilder'; export { Transpiler } from './transpiler'; diff --git a/tests/scenarios/compat-renaming-test.ts b/tests/scenarios/compat-renaming-test.ts index 4f04fd644..7dec1dbe3 100644 --- a/tests/scenarios/compat-renaming-test.ts +++ b/tests/scenarios/compat-renaming-test.ts @@ -4,7 +4,6 @@ import QUnit from 'qunit'; import { resolve, sep } from 'path'; const { module: Qmodule, test } = QUnit; -import { definesPattern } from '@embroider/test-support'; import { ExpectFile, expectRewrittenFilesAt } from '@embroider/test-support/file-assertions/qunit'; import { throwOnWarnings } from '@embroider/core'; @@ -225,13 +224,11 @@ appScenarios .to('./node_modules/emits-multiple-packages/somebody-elses-package/utils/index.js'); }); test('renamed modules keep their classic runtime name when used as implicit-modules', function () { - let assertFile = expectFile('assets/app-template.js'); - assertFile.matches( - definesPattern( - 'somebody-elses-package/environment', - 'emits-multiple-packages/somebody-elses-package/environment' - ) - ); + expectAudit.module('assets/app-template.js').resolves('./#embroider-implicit-modules').toModule().codeContains(` + d('somebody-elses-package/environment', function() { + return i('emits-multiple-packages/somebody-elses-package/environment') + }); + `); }); test('rewriting one module does not capture entire package namespace', function () { expectAudit