use package paths instead of relative paths for app tree resolving

embroider-build · Oct 24, 2023 · 89f7384 · 89f7384
1 parent a1343c1
commit 89f7384
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 43 deletions.
diff --git 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<string> = [];
-    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
@@ -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
@@ -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<AddonPackage>;
+  // 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<AddonPackage, string>;
   // 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
@@ -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
diff --git 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'),
                   },
                 ],
               },