Skip to content

Commit

Permalink
Fix resolve self priority (#4704)
Browse files Browse the repository at this point in the history
  • Loading branch information
timotheeguerin authored Oct 11, 2024
1 parent 84b3d68 commit e4b0124
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 17 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/compiler"
---

Fix order of resolution from node_modules and parent package
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ developer/

*.vsix
*.zip
vite.config.ts.timestamp-*


# Api extractor
packages/*/etc/
Expand Down
31 changes: 14 additions & 17 deletions packages/compiler/src/module-resolver/module-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
InvalidPackageTargetError,
NoMatchingConditionsError,
} from "./esm/utils.js";
import { parseNodeModuleSpecifier } from "./utils.js";
import { NodeModuleSpecifier, parseNodeModuleSpecifier } from "./utils.js";

// Resolve algorithm of node https://nodejs.org/api/modules.html#modules_all_together

Expand Down Expand Up @@ -126,10 +126,6 @@ export async function resolveModule(
}
}

// Try to resolve package itself.
const self = await resolveSelf(specifier, absoluteStart);
if (self) return self;

// Try to resolve as a node_module package.
const module = await resolveAsNodeModule(specifier, absoluteStart);
if (module) return module;
Expand All @@ -154,19 +150,18 @@ export async function resolveModule(
}

/**
* Equivalent implementation to node LOAD_PACKAGE_SELF
* Resolve if the import is importing the current package.
* Try to import a package with that name in the current directory.
*/
async function resolveSelf(name: string, baseDir: string): Promise<ResolvedModule | undefined> {
for (const dir of listDirHierarchy(baseDir)) {
const pkgFile = resolvePath(dir, "package.json");
if (!(await isFile(host, pkgFile))) continue;
const pkg = await readPackage(host, pkgFile);
// Node Spec says that you shouldn't lookup past the first package.json. However we used to support that so keeping this.
if (pkg.name !== name) continue;
return loadPackage(dir, pkg);
}
return undefined;
async function resolveSelf(
{ packageName, subPath }: NodeModuleSpecifier,
dir: string,
): Promise<ResolvedModule | undefined> {
const pkgFile = resolvePath(dir, "package.json");
if (!(await isFile(host, pkgFile))) return undefined;
const pkg = await readPackage(host, pkgFile);
// Node Spec says that you shouldn't lookup past the first package.json. However we used to support that so keeping this.
if (pkg.name !== packageName) return undefined;
return loadPackage(dir, pkg, subPath);
}

/**
Expand All @@ -182,6 +177,8 @@ export async function resolveModule(
const dirs = listDirHierarchy(baseDir);

for (const dir of dirs) {
const self = await resolveSelf(module, dir);
if (self) return self;
const n = await loadPackageAtPath(
joinPaths(dir, "node_modules", module.packageName),
module.subPath,
Expand Down
27 changes: 27 additions & 0 deletions packages/compiler/test/module-resolver/module-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,4 +399,31 @@ describe("resolve self", () => {
mainFile: "/ws/proj/entry.js",
});
});

it("prioritize local node_modules over self from multiple parent up", async () => {
const { host } = mkFs({
"/ws/proj/package.json": JSON.stringify({ name: "@scope/proj", main: "entry.js" }),
"/ws/proj/entry.js": "",
"/ws/proj/nested/index.js": "",
"/ws/proj/node_modules/test-lib/package.json": JSON.stringify({ main: "entry.js" }),
"/ws/proj/node_modules/test-lib/entry.js": "",
"/ws/proj/node_modules/test-lib/nested/index.js": "",
// @scope/proj installed locally
"/ws/proj/node_modules/test-lib/node_modules/@scope/proj/package.json": JSON.stringify({
name: "@scope/proj",
main: "entry.js",
}),
"/ws/proj/node_modules/test-lib/node_modules/@scope/proj/entry.js": "",
});

const resolved = await resolveModule(host, "@scope/proj", {
baseDir: "/ws/proj/node_modules/test-lib/nested",
});
const path = "/ws/proj/node_modules/test-lib/node_modules/@scope/proj";
expect(resolved).toMatchObject({
type: "module",
path,
mainFile: `${path}/entry.js`,
});
});
});

0 comments on commit e4b0124

Please sign in to comment.