Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolver refactor #1627

Merged
merged 6 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
},
"jest": {
"projects": [
"<rootDir>/packages/*",
"<rootDir>/test-packages/"
"<rootDir>/packages/*"
],
"testEnvironment": "node",
"testMatch": [
Expand Down
7 changes: 6 additions & 1 deletion packages/compat/src/standalone-addon-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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), {
Expand All @@ -53,6 +57,7 @@ ${summarizePeerDepViolations(violations)}`
destDir: '@embroider/synthesized-vendor',
}),
writeFile('index.json', JSON.stringify(index, null, 2)),
...fakeTargets,
]);
}

Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/compat/tests/audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
50 changes: 32 additions & 18 deletions packages/core/src/module-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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:

Expand All @@ -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)) {
Expand All @@ -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);
Expand Down Expand Up @@ -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]))
);
}

Expand Down
18 changes: 10 additions & 8 deletions tests/scenarios/core-resolver-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"`,
});

Expand All @@ -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');
});
Expand Down
Loading