Skip to content

Commit

Permalink
Use relative paths for self-imports
Browse files Browse the repository at this point in the history 
This restores the original behavior we had on main for self-imports only because those don't need to be visible as dependencies for Vite and are simpler for us to handle
  • Loading branch information
@lolmaus
lolmaus committed Oct 12, 2023
1 parent 6017ffd commit a31e2d6
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 32 deletions.
14 changes: 5 additions & 9 deletions packages/core/src/module-resolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,24 +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.rehome(resolve(pkg.root, '..', 'superFakeTarget.js')).withMeta({
resolvedWithinPackage: pkg.root,
});
// }
}

private preHandleExternal<R extends ModuleRequest>(request: R): R {
Expand Down
51 changes: 28 additions & 23 deletions tests/scenarios/core-resolver-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const { module: Qmodule, test } = QUnit;

Scenarios.fromProject(() => new Project())
.map('core-resolver-test', app => {
let rewrittenApp = app.addDependency('my-app');
let appMeta: AppMeta = {
type: 'app',
version: 2,
Expand All @@ -26,28 +27,28 @@ Scenarios.fromProject(() => new Project())
fileFilter: '_babel_filter.js',
},
};
app.pkg = {
rewrittenApp.pkg = {
name: 'my-app',
keywords: ['ember-addon'],
'ember-addon': appMeta,
};
app.mergeFiles({
rewrittenApp.mergeFiles({
'index.html': '<script src="./app.js" type="module"></script>',
});
app.addDependency('the-apps-dep', {
rewrittenApp.addDependency('the-apps-dep', {
files: {
'index.js': '',
},
});

let v1Addon = baseAddon();
v1Addon.name = 'a-v1-addon';
app.addDependency(v1Addon);
rewrittenApp.addDependency(v1Addon);

// this is just an empty fixture package, it's the presence of a dependency
// named ember-auto-import that tells us that the app was allowed to import
// deps from npm.
app.addDependency('ember-auto-import', { version: '2.0.0' });
rewrittenApp.addDependency('ember-auto-import', { version: '2.0.0' });
})
.forEachScenario(scenario => {
Qmodule(scenario.name, function (hooks) {
Expand All @@ -63,6 +64,7 @@ Scenarios.fromProject(() => new Project())

let configure: (opts?: ConfigureOpts) => Promise<void>;
let app: PreparedApp;
let rewrittenAppDir: string;

function addonPackageJSON(name = 'my-addon', addonMeta?: Partial<AddonMeta>) {
return JSON.stringify(
Expand All @@ -82,10 +84,11 @@ Scenarios.fromProject(() => new Project())
hooks.beforeEach(async assert => {
installAuditAssertions(assert);
app = await scenario.prepare();
rewrittenAppDir = resolve(app.dir, 'node_modules', 'my-app');

givenFiles = function (files: Record<string, string>) {
for (let [filename, contents] of Object.entries(files)) {
outputFileSync(resolve(app.dir, filename), contents, 'utf8');
outputFileSync(resolve(rewrittenAppDir, filename), contents, 'utf8');
}
};
configure = async function (opts?: ConfigureOpts) {
Expand All @@ -95,20 +98,20 @@ Scenarios.fromProject(() => new Project())
renameModules: {},
renamePackages: opts?.renamePackages ?? {},
resolvableExtensions: ['.js', '.hbs'],
appRoot: app.dir,
appRoot: rewrittenAppDir,
engines: [
{
packageName: 'my-app',
root: app.dir,
root: rewrittenAppDir,
fastbootFiles: opts?.fastbootFiles ?? {},
activeAddons: [
{
name: 'my-addon',
root: resolve(app.dir, 'node_modules', 'my-addon'),
root: resolve(rewrittenAppDir, 'node_modules', 'my-addon'),
},
{
name: 'a-v1-addon',
root: resolve(app.dir, 'node_modules', 'a-v1-addon'),
root: resolve(rewrittenAppDir, 'node_modules', 'a-v1-addon'),
},
],
},
Expand All @@ -124,7 +127,7 @@ Scenarios.fromProject(() => new Project())
activePackageRules: [
{
package: 'my-app',
roots: [app.dir],
roots: [rewrittenAppDir],
},
],
};
Expand All @@ -142,7 +145,7 @@ Scenarios.fromProject(() => new Project())
'node_modules/my-addon/package.json': addonPackageJSON('my-addon', opts?.addonMeta),
});

expectAudit = await assert.audit({ app: app.dir, 'reuse-build': true });
expectAudit = await assert.audit({ app: rewrittenAppDir, 'reuse-build': true });
};
});

Expand Down Expand Up @@ -613,9 +616,9 @@ Scenarios.fromProject(() => new Project())
});

test(`known ember-source-provided virtual packages are not externalized when explicitly included in deps`, async function () {
let pkg = readJsonSync(resolve(app.dir, 'package.json'));
let pkg = readJsonSync(resolve(rewrittenAppDir, 'package.json'));
pkg.dependencies['rsvp'] = '*';
writeJSONSync(resolve(app.dir, 'package.json'), pkg);
writeJSONSync(resolve(rewrittenAppDir, 'package.json'), pkg);
givenFiles({
'node_modules/rsvp/index.js': '',
'app.js': `import "rsvp"`,
Expand Down Expand Up @@ -760,14 +763,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(rewrittenAppDir, '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"`,
});

Expand All @@ -776,36 +780,37 @@ 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(rewrittenAppDir, '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');
});

test('implicit modules in moved dependencies', async function () {
let index: RewrittenPackageIndex = {
packages: {
[resolve(app.dir, 'node_modules/a-v1-addon')]: 'a-v1-addon.1234',
[resolve(rewrittenAppDir, 'node_modules/a-v1-addon')]: 'a-v1-addon.1234',
},
extraResolutions: {},
};
Expand Down

0 comments on commit a31e2d6

Please sign in to comment.