From 11dc23b0fb89c5e60f511d5fac36c3a75824d6ec Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 22 Oct 2024 15:19:17 -0400 Subject: [PATCH] handle renaming This is very similar to the existing aliasing feature. Previously, aliasing happened both during our analysis to decide which imports to handle and then again inside webpack. Now we handle it all once during analysis and emit imports for webpack that are already aliased. This means webpack doesn't need to learn about how renamed-modules too, which don't perfectly follow aliasing rules. --- packages/ember-auto-import/ts/auto-import.ts | 73 +++++++++++++++---- packages/ember-auto-import/ts/bundler.ts | 1 - packages/ember-auto-import/ts/package.ts | 37 ++++++---- packages/ember-auto-import/ts/splitter.ts | 15 ++-- .../ts/tests/splitter-test.ts | 44 ++++++++--- packages/ember-auto-import/ts/webpack.ts | 23 +++--- 6 files changed, 133 insertions(+), 60 deletions(-) diff --git a/packages/ember-auto-import/ts/auto-import.ts b/packages/ember-auto-import/ts/auto-import.ts index 809f1ba7..15329b8f 100644 --- a/packages/ember-auto-import/ts/auto-import.ts +++ b/packages/ember-auto-import/ts/auto-import.ts @@ -2,7 +2,7 @@ import Splitter from './splitter'; import { Bundler, debugBundler } from './bundler'; import Analyzer from './analyzer'; import type { TreeType } from './analyzer'; -import Package from './package'; +import Package, { V2AddonResolver } from './package'; import BroccoliDebug from 'broccoli-debug'; import BundleConfig from './bundle-config'; import type { Node } from 'broccoli-node-api'; @@ -12,6 +12,7 @@ import { AppInstance, findTopmostAddon, isDeepAddonInstance, + PackageCache, } from '@embroider/shared-internals'; import WebpackBundler from './webpack'; import { Memoize } from 'typescript-memoize'; @@ -49,6 +50,7 @@ export interface AutoImportSharedAPI { export default class AutoImport implements AutoImportSharedAPI { private packages: Set = new Set(); + private packageCache: PackageCache; private env: 'development' | 'test' | 'production'; private consoleWrite: (msg: string) => void; private analyzers: Map = new Map(); @@ -67,7 +69,13 @@ export default class AutoImport implements AutoImportSharedAPI { constructor(addonInstance: AddonInstance) { let topmostAddon = findTopmostAddon(addonInstance); - this.packages.add(Package.lookupParentOf(topmostAddon)); + this.packageCache = PackageCache.shared( + 'ember-auto-import', + topmostAddon.project.root + ); + this.packages.add( + Package.lookupParentOf(topmostAddon, this.v2AddonResolver) + ); let host = topmostAddon.app; this.installAppFilter(host); @@ -108,7 +116,7 @@ export default class AutoImport implements AutoImportSharedAPI { treeType?: TreeType, supportsFastAnalyzer?: true ) { - let pack = Package.lookupParentOf(addon); + let pack = Package.lookupParentOf(addon, this.v2AddonResolver); this.packages.add(pack); let analyzer = new Analyzer( debugTree(tree, `preprocessor:input-${this.analyzers.size}`), @@ -124,17 +132,55 @@ export default class AutoImport implements AutoImportSharedAPI { this.v2Addons.set(packageName, packageRoot); } - private makeBundler(allAppTree: Node): Bundler { - // this is a concession to compatibility with ember-cli's treeForApp - // merging. Addons are allowed to inject modules into the app, and it's - // extremely common that those modules want to import from the addons - // themselves, even though this jumps arbitrarily many levels in the - // dependency graph. - // - // Since we handle v2 addons, we need to make sure all v2 addons function as - // "dependencies" of the app even though they're not really. - this.rootPackage.magicDeps = this.v2Addons; + get v2AddonResolver(): V2AddonResolver { + return { + hasV2Addon: (name: string): boolean => { + return this.v2Addons.has(name); + }, + + v2AddonRoot: (name: string): string | undefined => { + return this.v2Addons.get(name); + }, + + handleRenaming: (name: string): string => { + let hit = this.renamedModules().get(name); + if (hit) { + return hit; + } + hit = this.renamedModules().get(name + '.js'); + if (hit) { + return hit; + } + hit = this.renamedModules().get(name + '/index.js'); + if (hit) { + return hit; + } + return name; + }, + }; + } + private _renamedModules: Map | undefined; + + private renamedModules(): Map { + if (!this._renamedModules) { + this._renamedModules = new Map(); + for (let packageRoot of this.v2Addons.values()) { + let pkg = this.packageCache.get(packageRoot); + if (pkg.isV2Addon()) { + let renamedModules = pkg.meta['renamed-modules']; + if (renamedModules) { + for (let [from, to] of Object.entries(renamedModules)) { + this._renamedModules.set(from, to); + } + } + } + } + } + return this._renamedModules; + } + + private makeBundler(allAppTree: Node): Bundler { // The Splitter takes the set of imports from the Analyzer and // decides which ones to include in which bundles let splitter = new Splitter({ @@ -166,7 +212,6 @@ export default class AutoImport implements AutoImportSharedAPI { consoleWrite: this.consoleWrite, bundles: this.bundles, webpack, - v2Addons: this.v2Addons, rootPackage: this.rootPackage, }); } diff --git a/packages/ember-auto-import/ts/bundler.ts b/packages/ember-auto-import/ts/bundler.ts index 87ff6303..727311cd 100644 --- a/packages/ember-auto-import/ts/bundler.ts +++ b/packages/ember-auto-import/ts/bundler.ts @@ -15,7 +15,6 @@ export interface BundlerOptions { packages: Set; bundles: BundleConfig; webpack: typeof webpack; - v2Addons: Map; rootPackage: Package; } diff --git a/packages/ember-auto-import/ts/package.ts b/packages/ember-auto-import/ts/package.ts index e5c55b54..8e449dea 100644 --- a/packages/ember-auto-import/ts/package.ts +++ b/packages/ember-auto-import/ts/package.ts @@ -46,9 +46,9 @@ export interface Options { export interface DepResolution { type: 'package'; - path: string; packageName: string; packageRoot: string; + resolvedSpecifier: string; } interface LocalResolution { @@ -71,6 +71,12 @@ type Resolution = | URLResolution | ImpreciseResolution; +export type V2AddonResolver = { + hasV2Addon(name: string): boolean; + v2AddonRoot(name: string): string | undefined; + handleRenaming(name: string): string; +}; + export default class Package { public name: string; public root: string; @@ -87,12 +93,16 @@ export default class Package { private pkgGeneration: number; private pkgCache: any; private macrosConfig: MacrosConfig | undefined; + private extraResolve: V2AddonResolver; - static lookupParentOf(child: AddonInstance): Package { + static lookupParentOf( + child: AddonInstance, + extraResolve: V2AddonResolver + ): Package { if (!parentCache.has(child)) { let pkg = packageCache.get(child.parent); if (!pkg) { - pkg = new this(child); + pkg = new this(child, extraResolve); packageCache.set(child.parent, pkg); } parentCache.set(child, pkg); @@ -100,8 +110,9 @@ export default class Package { return parentCache.get(child)!; } - constructor(child: AddonInstance) { + constructor(child: AddonInstance, extraResolve: V2AddonResolver) { this.name = child.parent.pkg.name; + this.extraResolve = extraResolve; if (isDeepAddonInstance(child)) { this.root = this.pkgRoot = child.parent.root; @@ -233,19 +244,13 @@ export default class Package { return `${this.name}/${this.isAddon ? 'addon' : 'app'}`; } - // extra dependencies that must be treated as if they were really dependencies - // of this package. sigh. - // - // maps from packageName to packageRoot - magicDeps: Map | undefined; - hasDependency(name: string): boolean { let { pkg } = this; return Boolean( pkg.dependencies?.[name] || pkg.devDependencies?.[name] || pkg.peerDependencies?.[name] || - this.magicDeps?.get(name) + this.extraResolve.hasV2Addon(name) ); } @@ -269,7 +274,7 @@ export default class Package { return Boolean( pkg.dependencies?.[name] || pkg.peerDependencies?.[name] || - this.magicDeps?.has(name) + this.extraResolve.hasV2Addon(name) ); } @@ -319,7 +324,7 @@ export default class Package { break; } - let path = this.aliasFor(importedPath); + let path = this.extraResolve.handleRenaming(this.aliasFor(importedPath)); let packageName = getPackageName(path); if (!packageName) { // this can only happen if the user supplied an alias that points at a @@ -344,9 +349,9 @@ export default class Package { ) { return { type: 'package', - path: localPath, packageName: this.name, packageRoot: join(this.root, 'app'), + resolvedSpecifier: path, }; } } @@ -368,7 +373,7 @@ export default class Package { } if (!packageRoot) { - packageRoot = this.magicDeps?.get(packageName); + packageRoot = this.extraResolve.v2AddonRoot(packageName); } if (packageRoot == null) { @@ -384,9 +389,9 @@ export default class Package { this.assertAllowedDependency(packageName, fromPath); return { type: 'package', - path, packageName, packageRoot, + resolvedSpecifier: path, }; } diff --git a/packages/ember-auto-import/ts/splitter.ts b/packages/ember-auto-import/ts/splitter.ts index 344a57fe..58b3ec52 100644 --- a/packages/ember-auto-import/ts/splitter.ts +++ b/packages/ember-auto-import/ts/splitter.ts @@ -10,7 +10,8 @@ import { satisfies } from 'semver'; const debug = makeDebug('ember-auto-import:splitter'); export interface ResolvedImport { - specifier: string; + requestedSpecifier: string; + resolvedSpecifier: string; packageName: string; packageRoot: string; importedBy: LiteralImport[]; @@ -115,7 +116,8 @@ export default class Splitter { seenAlready.importedBy.push(imp); } else { targets.set(imp.specifier, { - specifier: imp.specifier, + requestedSpecifier: imp.specifier, + resolvedSpecifier: target.resolvedSpecifier, packageName: target.packageName, packageRoot: target.packageRoot, importedBy: [imp], @@ -284,9 +286,11 @@ export default class Splitter { } private sortBundle(bundle: BundleDependencies) { - bundle.staticImports.sort((a, b) => a.specifier.localeCompare(b.specifier)); + bundle.staticImports.sort((a, b) => + a.requestedSpecifier.localeCompare(b.requestedSpecifier) + ); bundle.dynamicImports.sort((a, b) => - a.specifier.localeCompare(b.specifier) + a.requestedSpecifier.localeCompare(b.requestedSpecifier) ); bundle.dynamicTemplateImports.sort((a, b) => a.cookedQuasis[0].localeCompare(b.cookedQuasis[0]) @@ -329,7 +333,8 @@ class LazyPrintDeps { private describeResolvedImport(imp: ResolvedImport) { return { - specifier: imp.specifier, + requestedSpecifier: imp.requestedSpecifier, + resolvedSpecifier: imp.resolvedSpecifier, packageRoot: imp.packageRoot, importedBy: imp.importedBy.map(this.describeImport.bind(this)), }; diff --git a/packages/ember-auto-import/ts/tests/splitter-test.ts b/packages/ember-auto-import/ts/tests/splitter-test.ts index 00729170..a2efc6de 100644 --- a/packages/ember-auto-import/ts/tests/splitter-test.ts +++ b/packages/ember-auto-import/ts/tests/splitter-test.ts @@ -69,7 +69,17 @@ Qmodule('splitter', function (hooks) { await project.write(); setup = function (options: Options = {}) { - pack = new Package(stubAddonInstance(project.baseDir, options)); + pack = new Package(stubAddonInstance(project.baseDir, options), { + handleRenaming(name) { + return name; + }, + hasV2Addon() { + return false; + }, + v2AddonRoot() { + return undefined; + }, + }); let transpiled = broccoliBabel(new UnwatchedDir(project.baseDir), { plugins: [ require.resolve('../../js/analyzer-plugin'), @@ -222,7 +232,10 @@ Qmodule('splitter', function (hooks) { assert.deepEqual(deps.get('app')?.dynamicTemplateImports, []); let dynamicImports = deps.get('app')?.dynamicImports; assert.equal(dynamicImports?.length, 1); - assert.equal(dynamicImports?.[0].specifier, example[1].specifier); + assert.equal( + dynamicImports?.[0].requestedSpecifier, + example[1].specifier + ); assert.equal(dynamicImports?.[0].packageName, example[1].packageName); assert.equal( dynamicImports?.[0].packageRoot, @@ -268,7 +281,10 @@ Qmodule('splitter', function (hooks) { assert.deepEqual(deps.get('app')?.staticTemplateImports, []); let staticImports = deps.get('app')?.staticImports; assert.equal(staticImports?.length, 1); - assert.equal(staticImports?.[0].specifier, example[1].specifier); + assert.equal( + staticImports?.[0].requestedSpecifier, + example[1].specifier + ); assert.equal(staticImports?.[0].packageName, example[1].packageName); assert.equal( staticImports?.[0].packageRoot, @@ -370,7 +386,8 @@ Qmodule('splitter', function (hooks) { deps.get('app')?.staticImports.map((i) => ({ packageName: i.packageName, packageRoot: i.packageRoot, - specifier: i.specifier, + requestedSpecifier: i.requestedSpecifier, + resolvedSpecifier: i.resolvedSpecifier, })), [ { @@ -380,7 +397,8 @@ Qmodule('splitter', function (hooks) { 'node_modules', 'aliasing-example' ), - specifier: 'my-aliased-package', + requestedSpecifier: 'my-aliased-package', + resolvedSpecifier: 'aliasing-example/dist/index.js', }, ] ); @@ -401,7 +419,8 @@ Qmodule('splitter', function (hooks) { deps.get('app')?.staticImports.map((i) => ({ packageName: i.packageName, packageRoot: i.packageRoot, - specifier: i.specifier, + requestedSpecifier: i.requestedSpecifier, + resolvedSpecifier: i.resolvedSpecifier, })), [ { @@ -411,7 +430,8 @@ Qmodule('splitter', function (hooks) { 'node_modules', 'aliasing-example' ), - specifier: 'my-aliased-package/inside', + requestedSpecifier: 'my-aliased-package/inside', + resolvedSpecifier: 'aliasing-example/dist/inside', }, ] ); @@ -432,7 +452,8 @@ Qmodule('splitter', function (hooks) { deps.get('app')?.staticImports.map((i) => ({ packageName: i.packageName, packageRoot: i.packageRoot, - specifier: i.specifier, + requestedSpecifier: i.requestedSpecifier, + resolvedSpecifier: i.resolvedSpecifier, })), [ { @@ -442,7 +463,8 @@ Qmodule('splitter', function (hooks) { 'node_modules', 'aliasing-example' ), - specifier: 'aliasing-example', + requestedSpecifier: 'aliasing-example', + resolvedSpecifier: 'aliasing-example/dist', }, ] ); @@ -462,7 +484,7 @@ Qmodule('splitter', function (hooks) { assert.deepEqual( deps.get('app')?.staticImports.map((i) => ({ - specifier: i.specifier, + specifier: i.requestedSpecifier, })), [ { @@ -490,7 +512,7 @@ Qmodule('splitter', function (hooks) { let deps = await splitter.deps(); assert.deepEqual( deps.get('app')?.staticImports.map((i) => ({ - specifier: i.specifier, + specifier: i.requestedSpecifier, })), [ { diff --git a/packages/ember-auto-import/ts/webpack.ts b/packages/ember-auto-import/ts/webpack.ts index 1b49480b..e9213d4b 100644 --- a/packages/ember-auto-import/ts/webpack.ts +++ b/packages/ember-auto-import/ts/webpack.ts @@ -79,10 +79,10 @@ module.exports = (function(){ return m && m.__esModule ? m : Object.assign({ default: m }, m); } {{#each staticImports as |module|}} - d('{{js-string-escape module.specifier}}', EAI_DISCOVERED_EXTERNALS('{{module-to-id module.specifier}}'), function() { return esc(require('{{js-string-escape module.specifier}}')); }); + d('{{js-string-escape module.requestedSpecifier}}', EAI_DISCOVERED_EXTERNALS('{{module-to-id module.requestedSpecifier}}'), function() { return esc(require('{{js-string-escape module.resolvedSpecifier}}')); }); {{/each}} {{#each dynamicImports as |module|}} - d('_eai_dyn_{{js-string-escape module.specifier}}', [], function() { return import('{{js-string-escape module.specifier}}'); }); + d('_eai_dyn_{{js-string-escape module.requestedSpecifier}}', [], function() { return import('{{js-string-escape module.resolvedSpecifier}}'); }); {{/each}} {{#each staticTemplateImports as |module|}} d('_eai_sync_{{js-string-escape module.key}}', [], function() { @@ -105,8 +105,8 @@ module.exports = (function(){ `, { noEscape: true } ) as (args: { - staticImports: { specifier: string }[]; - dynamicImports: { specifier: string }[]; + staticImports: { requestedSpecifier: string; resolvedSpecifier: string }[]; + dynamicImports: { requestedSpecifier: string; resolvedSpecifier: string }[]; staticTemplateImports: { key: string; args: string; template: string }[]; dynamicTemplateImports: { key: string; args: string; template: string }[]; publicAssetURL: string | undefined; @@ -216,15 +216,12 @@ export default class WebpackBundler extends Plugin implements Bundler { resolve: { extensions: EXTENSIONS, mainFields: ['browser', 'module', 'main'], - alias: Object.assign( - { - // this is because of the allowAppImports feature needs to be able to import things - // like app-name/lib/something from within webpack handled code but that needs to be - // able to resolve to app-root/app/lib/something. - [this.opts.rootPackage.name]: `${this.opts.rootPackage.root}/app`, - }, - ...removeUndefined([...this.opts.packages].map((pkg) => pkg.aliases)) - ), + alias: Object.assign({ + // this is because of the allowAppImports feature needs to be able to import things + // like app-name/lib/something from within webpack handled code but that needs to be + // able to resolve to app-root/app/lib/something. + [this.opts.rootPackage.name]: `${this.opts.rootPackage.root}/app`, + }), }, plugins: removeUndefined([stylePlugin]), module: {