Merge pull request #1627 from embroider-build/resolver-refactor

Resolver refactor

package.json

@@ -21,8 +21,7 @@
   },
   "jest": {
     "projects": [
-      "<rootDir>/packages/*",
-      "<rootDir>/test-packages/"
+      "<rootDir>/packages/*"
     ],
     "testEnvironment": "node",
     "testMatch": [

packages/compat/src/standalone-addon-build.ts

@@ -44,6 +44,10 @@ ${summarizePeerDepViolations(violations)}`
     return new Funnel(interior, { destDir: index.packages[pkg.root] });
   });
 
+  let fakeTargets = Object.values(index.packages).map(dir => {
+    return writeFile(join(dir, '..', 'moved-package-target.js'), '');
+  });
+
   return broccoliMergeTrees([
     ...exteriorTrees,
     new Funnel(compatApp.synthesizeStylesPackage(interiorTrees), {
@@ -53,6 +57,7 @@ ${summarizePeerDepViolations(violations)}`
       destDir: '@embroider/synthesized-vendor',
     }),
     writeFile('index.json', JSON.stringify(index, null, 2)),
+    ...fakeTargets,
   ]);
 }
 
@@ -62,7 +67,7 @@ function buildAddonIndex(compatApp: CompatApp, appPackage: Package, packages: Se
     extraResolutions: {},
   };
   for (let oldPkg of packages) {
-    let newRoot = `${oldPkg.name}.${hashed(oldPkg.root)}`;
+    let newRoot = `${oldPkg.name}.${hashed(oldPkg.root)}/node_modules/${oldPkg.name}`;
     content.packages[oldPkg.root] = newRoot;
     let nonResolvableDeps = oldPkg.nonResolvableDeps;
     if (nonResolvableDeps) {

packages/compat/tests/audit.test.ts

@@ -318,7 +318,7 @@ describe('audit', function () {
     });
     let result = await audit();
     expect(result.findings).toEqual([]);
-    expect(Object.keys(result.modules).length).toBe(3);
+    expect(Object.keys(result.modules).length).toBe(2);
   });
 
   test('finds missing component in standalone hbs', async function () {

packages/core/src/module-resolver.ts

@@ -754,7 +754,11 @@ export class Resolver {
       // was moved. RewrittenPackageCache.resolve already took care of finding
       // the right target, and we redirect the request so it will look inside
       // that target.
-      return logTransition('request targets a moved package', request, this.resolveWithinPackage(request, targetPkg));
+      return logTransition(
+        'request targets a moved package',
+        request,
+        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.
@@ -815,22 +819,20 @@ 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.
-      return logTransition(`v1 self-import`, request, this.resolveWithinPackage(request, pkg));
+      return logTransition(
+        `v1 self-import`,
+        request,
+        request.alias(request.specifier.replace(pkg.name, '.')).rehome(resolve(pkg.root, 'package.json'))
+      );
     }
 
     return request;
   }
 
-  private resolveWithinPackage<R extends ModuleRequest>(request: R, pkg: Package): R {
-    if ('exports' in pkg.packageJSON) {
-      // this is the easy case -- a package that uses exports can safely resolve
-      // its own name, so it's enough to let it resolve the (self-targeting)
-      // specifier from its own package root.
-      return request.rehome(resolve(pkg.root, 'package.json'));
-    } else {
-      // otherwise we need to just assume that internal naming is simple
-      return request.alias(request.specifier.replace(pkg.name, '.')).rehome(resolve(pkg.root, 'package.json'));
-    }
+  private resolveWithinMovedPackage<R extends ModuleRequest>(request: R, pkg: Package): R {
+    return request.rehome(resolve(pkg.root, '..', 'moved-package-target.js')).withMeta({
+      resolvedWithinPackage: pkg.root,
+    });
   }
 
   private preHandleExternal<R extends ModuleRequest>(request: R): R {
@@ -954,7 +956,7 @@ export class Resolver {
         throw new Error(
           `A module tried to resolve "${request.specifier}" and didn't find it (${label}).
 
- - Maybe a dependency declaration is missing? 
+ - Maybe a dependency declaration is missing?
  - Remember that v1 addons can only import non-Ember-addon NPM dependencies if they include ember-auto-import in their dependencies.
  - If this dependency is available in the AMD loader (because someone manually called "define()" for it), you can configure a shim like:
 
@@ -977,6 +979,14 @@ export class Resolver {
   }
 
   fallbackResolve<R extends ModuleRequest>(request: R): R {
+    if (request.specifier === '@embroider/macros') {
+      // the macros package is always handled directly within babel (not
+      // necessarily as a real resolvable package), so we should not mess with it.
+      // It might not get compiled away until *after* our plugin has run, which is
+      // why we need to know about it.
+      return logTransition('fallback early exit', request);
+    }
+
     let { specifier, fromFile } = request;
 
     if (compatPattern.test(specifier)) {
@@ -996,6 +1006,13 @@ 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);
     if (!pkg) {
       return logTransition('no identifiable owningPackage', request);
@@ -1043,14 +1060,11 @@ export class Resolver {
     }
 
     // auto-upgraded packages can fall back to the set of known active addons
-    //
-    // v2 packages can fall back to the set of known active addons only to find
-    // themselves (which is needed due to app tree merging)
-    if ((pkg.meta['auto-upgraded'] || packageName === pkg.name) && this.options.activeAddons[packageName]) {
+    if (pkg.meta['auto-upgraded'] && this.options.activeAddons[packageName]) {
       return logTransition(
         `activeAddons`,
         request,
-        this.resolveWithinPackage(request, this.packageCache.get(this.options.activeAddons[packageName]))
+        this.resolveWithinMovedPackage(request, this.packageCache.get(this.options.activeAddons[packageName]))
       );
     }
 

tests/scenarios/core-resolver-test.ts

@@ -760,14 +760,15 @@ Scenarios.fromProject(() => new Project())
         test('app can resolve file in rewritten addon', async function () {
           let index: RewrittenPackageIndex = {
             packages: {
-              [resolve(app.dir, 'node_modules/my-addon')]: 'my-addon.1234',
+              [resolve(app.dir, 'node_modules/my-addon')]: 'my-addon.1234/node_modules/my-addon',
             },
             extraResolutions: {},
           };
           givenFiles({
             'node_modules/.embroider/rewritten-packages/index.json': JSON.stringify(index),
-            'node_modules/.embroider/rewritten-packages/my-addon.1234/hello-world.js': ``,
-            'node_modules/.embroider/rewritten-packages/my-addon.1234/package.json': addonPackageJSON(),
+            'node_modules/.embroider/rewritten-packages/my-addon.1234/node_modules/my-addon/hello-world.js': ``,
+            'node_modules/.embroider/rewritten-packages/my-addon.1234/node_modules/my-addon/package.json':
+              addonPackageJSON(),
             'app.js': `import "my-addon/hello-world"`,
           });
 
@@ -776,28 +777,29 @@ Scenarios.fromProject(() => new Project())
           expectAudit
             .module('./app.js')
             .resolves('my-addon/hello-world')
-            .to('./node_modules/.embroider/rewritten-packages/my-addon.1234/hello-world.js');
+            .to('./node_modules/.embroider/rewritten-packages/my-addon.1234/node_modules/my-addon/hello-world.js');
         });
 
         test('moved addon resolves dependencies from its original location', async function () {
           let index: RewrittenPackageIndex = {
             packages: {
-              [resolve(app.dir, 'node_modules/my-addon')]: 'my-addon.1234',
+              [resolve(app.dir, 'node_modules/my-addon')]: 'my-addon.1234/node_modules/my-addon',
             },
             extraResolutions: {},
           };
           givenFiles({
             '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/hello-world.js': `import "inner-dep"`,
-            'node_modules/.embroider/rewritten-packages/my-addon.1234/package.json': addonPackageJSON(),
+            'node_modules/.embroider/rewritten-packages/my-addon.1234/node_modules/my-addon/hello-world.js': `import "inner-dep"`,
+            'node_modules/.embroider/rewritten-packages/my-addon.1234/node_modules/my-addon/package.json':
+              addonPackageJSON(),
             'app.js': `import "my-addon/hello-world"`,
           });
 
           await configure({});
 
           expectAudit
-            .module('./node_modules/.embroider/rewritten-packages/my-addon.1234/hello-world.js')
+            .module('./node_modules/.embroider/rewritten-packages/my-addon.1234/node_modules/my-addon/hello-world.js')
             .resolves('inner-dep')
             .to('./node_modules/my-addon/node_modules/inner-dep/index.js');
         });