From b6c243a7eccad95b191172befc302826d33e031d Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 13 Jan 2022 02:03:12 -0500 Subject: [PATCH] native v2 addons can always import from NPM This check was intended to prevent v1 addons from accidentally gaining new semantics under embroider, but it needs to be relaxed for v2 addons which *are* supposed to be able to import from NPM without any special dependency on ember-auto-import. --- .../core/src/babel-plugin-adjust-imports.ts | 16 +++++++++--- .../macros/src/babel/dependency-satisfies.ts | 2 +- .../src/glimmer/dependency-satisfies.ts | 2 +- tests/scenarios/v2-addon-test.ts | 26 +++++++++++++++++++ 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/packages/core/src/babel-plugin-adjust-imports.ts b/packages/core/src/babel-plugin-adjust-imports.ts index e2c2f25ce..dda9f18b1 100644 --- a/packages/core/src/babel-plugin-adjust-imports.ts +++ b/packages/core/src/babel-plugin-adjust-imports.ts @@ -159,10 +159,14 @@ function isExplicitlyExternal(specifier: string, fromPkg: V2Package): boolean { return Boolean(fromPkg.isV2Addon() && fromPkg.meta['externals'] && fromPkg.meta['externals'].includes(specifier)); } -function isResolvable(packageName: string, fromPkg: Package, appRoot: string): false | Package { +function isResolvable(packageName: string, fromPkg: V2Package, appRoot: string): false | Package { try { let dep = PackageCache.shared('embroider-stage3', appRoot).resolve(packageName, fromPkg); - if (!dep.isEmberPackage() && !fromPkg.hasDependency('ember-auto-import')) { + if (!dep.isEmberPackage() && fromPkg.meta['auto-upgraded'] && !fromPkg.hasDependency('ember-auto-import')) { + // classic ember addons can only import non-ember dependencies if they + // have ember-auto-import. + // + // whereas native v2 packages can always import any dependency return false; } return dep; @@ -499,9 +503,13 @@ class AdjustFile { } @Memoize() - relocatedIntoPackage(): Package | undefined { + relocatedIntoPackage(): V2Package | undefined { if (this.isRelocated) { - return this.packageCache.ownerOfFile(this.name); + let owning = this.packageCache.ownerOfFile(this.name); + if (owning && !owning.isV2Ember()) { + throw new Error(`bug: it should only be possible to get relocated into a v2 ember package here`); + } + return owning; } } } diff --git a/packages/macros/src/babel/dependency-satisfies.ts b/packages/macros/src/babel/dependency-satisfies.ts index fc465686e..62a8109a9 100644 --- a/packages/macros/src/babel/dependency-satisfies.ts +++ b/packages/macros/src/babel/dependency-satisfies.ts @@ -25,7 +25,7 @@ export default function dependencySatisfies(path: NodePath, st let sourceFileName = sourceFile(path, state); try { let us = state.packageCache.ownerOfFile(sourceFileName); - if (!us || us.dependencies.every(dep => dep.name !== packageName.value)) { + if (!us?.hasDependency(packageName.value)) { return false; } diff --git a/packages/macros/src/glimmer/dependency-satisfies.ts b/packages/macros/src/glimmer/dependency-satisfies.ts index a01e301cb..9202181a5 100644 --- a/packages/macros/src/glimmer/dependency-satisfies.ts +++ b/packages/macros/src/glimmer/dependency-satisfies.ts @@ -23,7 +23,7 @@ export default function dependencySatisfies( let range = node.params[1].value; let us = packageCache.ownerOfFile(baseDir || moduleName); - if (!us || us.dependencies.every(dep => dep.name !== packageName)) { + if (!us?.hasDependency(packageName)) { return false; } diff --git a/tests/scenarios/v2-addon-test.ts b/tests/scenarios/v2-addon-test.ts index 87e7a0fdc..77ae7df5c 100644 --- a/tests/scenarios/v2-addon-test.ts +++ b/tests/scenarios/v2-addon-test.ts @@ -46,9 +46,24 @@ appScenarios setComponentTemplate(TEMPLATE, ExampleComponent); `, }, + 'import-from-npm.js': ` + export default async function() { + let { message } = await import('third-party'); + return message() + } + `, }, }); addon.linkDependency('@embroider/addon-shim', { baseDir: __dirname }); + addon.addDependency('third-party', { + files: { + 'index.js': ` + export function message() { + return 'content from third-party'; + } + `, + }, + }); project.addDevDependency(addon); @@ -78,6 +93,17 @@ appScenarios }); `, }, + unit: { + 'import-test.js': ` + import { module, test } from 'qunit'; + import example from 'v2-addon/import-from-npm'; + module('Unit | import', function(hooks) { + test('v2 addons can import() from NPM', async function(assert) { + assert.equal(await example(), 'content from third-party'); + }); + }); + `, + }, }, }); })