Skip to content

Commit

Permalink
native v2 addons can always import from NPM
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ef4 committed Jan 13, 2022
1 parent 5e59f5d commit b6c243a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 6 deletions.
16 changes: 12 additions & 4 deletions packages/core/src/babel-plugin-adjust-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/macros/src/babel/dependency-satisfies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export default function dependencySatisfies(path: NodePath<t.CallExpression>, 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;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/macros/src/glimmer/dependency-satisfies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
26 changes: 26 additions & 0 deletions tests/scenarios/v2-addon-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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');
});
});
`,
},
},
});
})
Expand Down

0 comments on commit b6c243a

Please sign in to comment.