From 89f73840b8fcf3d813ae3f113a5153a38151a44a Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Thu, 19 Oct 2023 14:11:46 +0100 Subject: [PATCH] use package paths instead of relative paths for app tree resolving --- packages/compat/src/compat-app-builder.ts | 21 +++++----- packages/core/package.json | 1 + packages/core/src/app-files.ts | 6 +-- packages/core/src/module-resolver.ts | 51 ++++++++++------------- pnpm-lock.yaml | 9 +++- tests/scenarios/core-resolver-test.ts | 2 + 6 files changed, 47 insertions(+), 43 deletions(-) diff --git a/packages/compat/src/compat-app-builder.ts b/packages/compat/src/compat-app-builder.ts index e2f0364b2b..15f5c2cb1a 100644 --- a/packages/compat/src/compat-app-builder.ts +++ b/packages/compat/src/compat-app-builder.ts @@ -289,9 +289,10 @@ export class CompatAppBuilder { root: realpathSync(index === 0 ? this.root : appFiles.engine.package.root), fastbootFiles: appFiles.fastbootFiles, activeAddons: [...appFiles.engine.addons] - .map(a => ({ - name: a.name, - root: a.root, + .map(([addon, canResolveFromFile]) => ({ + name: addon.name, + root: addon.root, + canResolveFromFile, })) // the traditional order is the order in which addons will run, such // that the last one wins. Our resolver's order is the order to @@ -396,7 +397,7 @@ export class CompatAppBuilder { private impliedAddonAssets(type: keyof ImplicitAssetPaths, { engine }: AppFiles): string[] { let result: Array = []; - for (let addon of sortBy(Array.from(engine.addons), this.scriptPriority.bind(this))) { + for (let addon of sortBy(Array.from(engine.addons.keys()), this.scriptPriority.bind(this))) { let implicitScripts = addon.meta[type]; if (implicitScripts) { let styles = []; @@ -640,12 +641,12 @@ export class CompatAppBuilder { if (!child.isEngine()) { this.findActiveAddons(child, engine, true); } - engine.addons.add(child); + engine.addons.set(child, resolvePath(pkg.root, 'package.json')); } // ensure addons are applied in the correct order, if set (via @embroider/compat/v1-addon) if (!isChild) { - engine.addons = new Set( - [...engine.addons].sort((a, b) => { + engine.addons = new Map( + [...engine.addons].sort(([a], [b]) => { return (a.meta['order-index'] || 0) - (b.meta['order-index'] || 0); }) ); @@ -656,7 +657,7 @@ export class CompatAppBuilder { let queue: Engine[] = [ { package: this.appPackageWithMovedDeps, - addons: new Set(), + addons: new Map(), parent: undefined, sourcePath: appJSPath, modulePrefix: this.modulePrefix(), @@ -671,12 +672,12 @@ export class CompatAppBuilder { break; } this.findActiveAddons(current.package, current); - for (let addon of current.addons) { + for (let addon of current.addons.keys()) { if (addon.isEngine() && !seenEngines.has(addon)) { seenEngines.add(addon); queue.push({ package: addon, - addons: new Set(), + addons: new Map(), parent: current, sourcePath: addon.root, modulePrefix: addon.name, diff --git a/packages/core/package.json b/packages/core/package.json index 97b3c363d5..e43bab3251 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -42,6 +42,7 @@ "lodash": "^4.17.21", "resolve": "^1.20.0", "resolve-package-path": "^4.0.1", + "reverse-exports": "workspace:*", "typescript-memoize": "^1.0.1", "walk-sync": "^3.0.0" }, diff --git a/packages/core/src/app-files.ts b/packages/core/src/app-files.ts index 3294e282f2..4bd825ed21 100644 --- a/packages/core/src/app-files.ts +++ b/packages/core/src/app-files.ts @@ -44,7 +44,7 @@ export class AppFiles { combinedFiles.add(f); } - for (let addon of engine.addons) { + for (let addon of engine.addons.keys()) { let appJS = addon.meta['app-js']; if (appJS) { for (let filename of Object.keys(appJS)) { @@ -202,8 +202,8 @@ export class AppFiles { export interface Engine { // the engine's own package package: Package; - // the set of active addons in the engine - addons: Set; + // the set of active addons in the engine. For each one we keep track of a file that can resolve the addon, because we'll need that later. + addons: Map; // the parent engine, if any parent: Engine | undefined; // where the engine's own V2 code comes from diff --git a/packages/core/src/module-resolver.ts b/packages/core/src/module-resolver.ts index 654abe79ad..86849680e5 100644 --- a/packages/core/src/module-resolver.ts +++ b/packages/core/src/module-resolver.ts @@ -11,6 +11,8 @@ import { explicitRelative, RewrittenPackageCache } from '@embroider/shared-inter import makeDebug from 'debug'; import assertNever from 'assert-never'; import resolveModule from 'resolve'; +import reversePackageExports from 'reverse-exports'; + import { virtualExternalESModule, virtualExternalCJSModule, @@ -70,7 +72,7 @@ export interface Options { interface EngineConfig { packageName: string; - activeAddons: { name: string; root: string }[]; + activeAddons: { name: string; root: string; canResolveFromFile: string }[]; fastbootFiles: { [appName: string]: { localFilename: string; shadowedFilename: string | undefined } }; root: string; } @@ -79,29 +81,29 @@ type MergeEntry = | { type: 'app-only'; 'app-js': { - localPath: string; - packageRoot: string; + specifier: string; + fromFile: string; fromPackageName: string; }; } | { type: 'fastboot-only'; 'fastboot-js': { - localPath: string; - packageRoot: string; + specifier: string; + fromFile: string; fromPackageName: string; }; } | { type: 'both'; 'app-js': { - localPath: string; - packageRoot: string; + specifier: string; + fromFile: string; fromPackageName: string; }; 'fastboot-js': { - localPath: string; - packageRoot: string; + specifier: string; + fromFile: string; fromPackageName: string; }; }; @@ -401,7 +403,7 @@ export class Resolver { return logTransition( 'matched addon entry', request, - request.alias(entry[section].localPath).rehome(resolve(entry[section].packageRoot, 'package.json')) + request.alias(entry[section].specifier).rehome(entry[section].fromFile) ); } } @@ -630,8 +632,8 @@ export class Resolver { engineModules.set(inEngineName, { type: 'app-only', 'app-js': { - localPath: inAddonName, - packageRoot: addon.root, + specifier: reversePackageExports(addon.packageJSON, inAddonName), + fromFile: addonConfig.canResolveFromFile, fromPackageName: addon.name, }, }); @@ -644,8 +646,8 @@ export class Resolver { engineModules.set(inEngineName, { type: 'both', 'app-js': { - localPath: inAddonName, - packageRoot: addon.root, + specifier: reversePackageExports(addon.packageJSON, inAddonName), + fromFile: addonConfig.canResolveFromFile, fromPackageName: addon.name, }, 'fastboot-js': prevEntry['fastboot-js'], @@ -674,8 +676,8 @@ export class Resolver { engineModules.set(inEngineName, { type: 'fastboot-only', 'fastboot-js': { - localPath: inAddonName, - packageRoot: addon.root, + specifier: reversePackageExports(addon.packageJSON, inAddonName), + fromFile: addonConfig.canResolveFromFile, fromPackageName: addon.name, }, }); @@ -688,8 +690,8 @@ export class Resolver { engineModules.set(inEngineName, { type: 'both', 'fastboot-js': { - localPath: inAddonName, - packageRoot: addon.root, + specifier: reversePackageExports(addon.packageJSON, inAddonName), + fromFile: addonConfig.canResolveFromFile, fromPackageName: addon.name, }, 'app-js': prevEntry['app-js'], @@ -1143,18 +1145,11 @@ export class Resolver { case undefined: return undefined; case 'app-only': - return request - .alias(matched.entry['app-js'].localPath) - .rehome(resolve(matched.entry['app-js'].packageRoot, 'package.json')); + return request.alias(matched.entry['app-js'].specifier).rehome(matched.entry['app-js'].fromFile); case 'fastboot-only': - return request - .alias(matched.entry['fastboot-js'].localPath) - .rehome(resolve(matched.entry['fastboot-js'].packageRoot, 'package.json')); + return request.alias(matched.entry['fastboot-js'].specifier).rehome(matched.entry['fastboot-js'].fromFile); case 'both': - let foundAppJS = this.nodeResolve( - matched.entry['app-js'].localPath, - resolve(matched.entry['app-js'].packageRoot, 'package.json') - ); + let foundAppJS = this.nodeResolve(matched.entry['app-js'].specifier, matched.entry['app-js'].fromFile); if (foundAppJS.type !== 'real') { throw new Error( `${matched.entry['app-js'].fromPackageName} declared ${inEngineSpecifier} in packageJSON.ember-addon.app-js, but that module does not exist` diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6fc51fd8cf..a377ec508d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -410,6 +410,9 @@ importers: resolve-package-path: specifier: ^4.0.1 version: 4.0.1 + reverse-exports: + specifier: workspace:* + version: link:../reverse-exports typescript-memoize: specifier: ^1.0.1 version: 1.0.1 @@ -566,6 +569,8 @@ importers: specifier: ^5.1.6 version: 5.1.6 + packages/reverse-exports: {} + packages/router: dependencies: '@ember/test-waiters': @@ -17146,7 +17151,7 @@ packages: dev: true /eslint-plugin-es@4.1.0(eslint@8.42.0): - resolution: {integrity: sha512-GILhQTnjYE2WorX5Jyi5i4dz5ALWxBIdQECVQavL6s7cI76IZTDWleTHkxz/QT3kvcs2QlGHvKLYsSlPOlPXnQ==, tarball: https://registry.npmjs.org/eslint-plugin-es/-/eslint-plugin-es-4.1.0.tgz} + resolution: {integrity: sha512-GILhQTnjYE2WorX5Jyi5i4dz5ALWxBIdQECVQavL6s7cI76IZTDWleTHkxz/QT3kvcs2QlGHvKLYsSlPOlPXnQ==} engines: {node: '>=8.10.0'} peerDependencies: eslint: '>=4.19.1' @@ -24739,7 +24744,7 @@ packages: resolution: {integrity: sha512-dCwEFf0/oT85M1fHBg4F0jtLwJrutGoHSQXCh7u4o2t1drG+c0a9Flnqww6XUKSfQMPpJBRjU8d4RXB09qtvaA==} hasBin: true peerDependencies: - browserslist: '>= 4.21.0' + browserslist: ^4.14.0 dependencies: browserslist: 4.21.10 escalade: 3.1.1 diff --git a/tests/scenarios/core-resolver-test.ts b/tests/scenarios/core-resolver-test.ts index c9d276d652..b3df736d18 100644 --- a/tests/scenarios/core-resolver-test.ts +++ b/tests/scenarios/core-resolver-test.ts @@ -105,10 +105,12 @@ Scenarios.fromProject(() => new Project()) { name: 'my-addon', root: resolve(app.dir, 'node_modules', 'my-addon'), + canResolveFromFile: resolve(app.dir, 'package.json'), }, { name: 'a-v1-addon', root: resolve(app.dir, 'node_modules', 'a-v1-addon'), + canResolveFromFile: resolve(app.dir, 'package.json'), }, ], },