Merge pull request #1278 from embroider-build/nested-v2-resolution-fix

allow v2 addons to use app tree fallback resolution
embroider-build · Nov 16, 2022 · be37be5 · be37be5
2 parents f966721 + 1f259e6
commit be37be5
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 10 deletions.
diff --git a/packages/core/src/babel-plugin-adjust-imports.ts b/packages/core/src/babel-plugin-adjust-imports.ts
@@ -258,14 +258,10 @@ function handleExternal(specifier: string, sourceFile: AdjustFile, opts: Options
   if (relocatedPkg) {
     // this file has been moved into another package (presumably the app).
 
-    // self-imports are legal in the app tree, even for v2 packages
-    if (packageName === pkg.name) {
-      return specifier;
-    }
-
     // first try to resolve from the destination package
     if (isResolvable(packageName, relocatedPkg, opts.appRoot)) {
-      if (!pkg.meta['auto-upgraded']) {
+      // self-imports are legal in the app tree, even for v2 packages.
+      if (!pkg.meta['auto-upgraded'] && packageName !== pkg.name) {
         throw new Error(
           `${pkg.name} is trying to import ${packageName} from within its app tree. This is unsafe, because ${pkg.name} can't control which dependencies are resolvable from the app`
         );
@@ -275,7 +271,8 @@ function handleExternal(specifier: string, sourceFile: AdjustFile, opts: Options
       // second try to resolve from the source package
       let targetPkg = isResolvable(packageName, pkg, opts.appRoot);
       if (targetPkg) {
-        if (!pkg.meta['auto-upgraded']) {
+        // self-imports are legal in the app tree, even for v2 packages.
+        if (!pkg.meta['auto-upgraded'] && packageName !== pkg.name) {
           throw new Error(
             `${pkg.name} is trying to import ${packageName} from within its app tree. This is unsafe, because ${pkg.name} can't control which dependencies are resolvable from the app`
           );
@@ -297,7 +294,10 @@ function handleExternal(specifier: string, sourceFile: AdjustFile, opts: Options
   }
 
   // auto-upgraded packages can fall back to the set of known active addons
-  if (pkg.meta['auto-upgraded'] && opts.activeAddons[packageName]) {
+  //
+  // 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) && opts.activeAddons[packageName]) {
     return explicitRelative(dirname(sourceFile.name), specifier.replace(packageName, opts.activeAddons[packageName]));
   }
 

diff --git a/tests/scenarios/v2-addon-test.ts b/tests/scenarios/v2-addon-test.ts
@@ -1,12 +1,12 @@
-import { appScenarios, baseV2Addon } from './scenarios';
+import { appScenarios, baseAddon, baseV2Addon } from './scenarios';
 import { PreparedApp } from 'scenario-tester';
 import QUnit from 'qunit';
 import merge from 'lodash/merge';
 
 const { module: Qmodule, test } = QUnit;
 
 appScenarios
-  .map('v2-addon', project => {
+  .map('v2-addon-basics', project => {
     let addon = baseV2Addon();
     addon.pkg.name = 'v2-addon';
     (addon.pkg as any)['ember-addon']['app-js']['./components/example-component.js'] =
@@ -49,6 +49,47 @@ appScenarios
 
     project.addDevDependency(addon);
 
+    // a v1 addon, which will have a v2 addon as a dep
+    let intermediate = baseAddon();
+    intermediate.pkg.name = 'intermediate';
+    intermediate.linkDependency('ember-auto-import', { baseDir: __dirname });
+    merge(intermediate.files, {
+      app: {
+        components: {
+          'hello.js': 'export { default } from "intermediate/components/hello"',
+        },
+      },
+      addon: {
+        components: {
+          'hello.hbs': '<div class="intermediate-hello"><Inner /></div>',
+        },
+      },
+    });
+    project.addDevDependency(intermediate);
+
+    // the inner v2 addon, which gets consumed by `intermediate`
+    let inner = baseV2Addon();
+    inner.pkg.name = 'inner';
+    (inner.pkg as any)['ember-addon']['app-js']['./components/inner.js'] = 'app/components/inner.js';
+    merge(inner.files, {
+      app: {
+        components: {
+          'inner.js': `export { default } from 'inner/components/inner';`,
+        },
+      },
+      components: {
+        'inner.js': `
+          import Component from '@glimmer/component';
+          import { hbs } from 'ember-cli-htmlbars';
+          import { setComponentTemplate } from '@ember/component';
+          const TEMPLATE = hbs("<div class='inner'>it works</div>")
+          export default class ExampleComponent extends Component {}
+          setComponentTemplate(TEMPLATE, ExampleComponent);
+        `,
+      },
+    });
+    intermediate.addDependency(inner);
+
     merge(project.files, {
       app: {
         templates: {
@@ -75,6 +116,23 @@ appScenarios
             });
           `,
         },
+        intergration: {
+          'intermediate-test.js': `
+            import { module, test } from 'qunit';
+            import { setupRenderingTest } from 'ember-qunit';
+            import { render } from '@ember/test-helpers';
+            import hbs from 'htmlbars-inline-precompile';
+
+            module('Integration | intermediate', function(hooks) {
+              setupRenderingTest(hooks);
+
+              test('v1 addon can invoke v2 addon through the app tree', async function(assert) {
+                await render(hbs('<Hello />'));
+                assert.dom('.intermediate-hello .inner').containsText('it works');
+              });
+            });
+          `,
+        },
         unit: {
           'import-test.js': `
            import { module, test } from 'qunit';