Skip to content

Commit a984e2d

Browse files
divybotlittledivy
andauthored
chore: test node_modules symlinks in npm workspace with cyclic deps (#34759)
Adds a regression spec test for the npm-workspace symlink scenario reported in #19726. ## Background The issue reported that an npm workspace where member packages are symlinked into `node_modules` (as `npm install` does) caused Rust to panic during package.json module resolution. The reproduction (verikono/deno-npm-symlinks) has three workspace packages where `pkg-a` and `pkg-b` import each other (a cyclic dependency). This was fixed somewhere along the way — the maintainer noted on the issue that it already works in v2.1.2, and I confirmed it works on current `main` across every resolution path I tried: - import-map resolution to the local workspace sources - pure `node_modules` + package.json `exports` resolution (`nodeModulesDir: manual`) - absolute symlinks pointing outside the project root - cyclic deps via `deno run`, `deno test` (type-check + run), and `deno info` (which correctly canonicalizes the symlink to the real path and shows the cycle) ## This PR Since there was no code left to fix, this adds a regression spec test at `tests/specs/npm/workspace_symlink_cyclic/` so the scenario doesn't regress. The test: - sets up an npm workspace with `main-project`, `pkg-a`, `pkg-b` (with `pkg-a` ⇄ `pkg-b` cyclic deps), mirroring the original reproduction - recreates the `node_modules` symlinks `npm install` produces, via a `setup.js` step using `Deno.symlinkSync` (no symlinks committed to git) - runs `deno test`, which resolves the symlinked workspace packages through `node_modules` and exercises the cyclic dependency Gated to `unix` since creating directory symlinks on Windows requires elevated privileges. Closes #19726 Closes denoland/divybot#440 --------- Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent c4e2dcd commit a984e2d

12 files changed

Lines changed: 121 additions & 0 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Regression test for https://github.com/denoland/deno/issues/19726
2+
//
3+
// An npm workspace where each member package is symlinked into the root
4+
// `node_modules` (as `npm install` does). `pkg-a` and `pkg-b` import each other
5+
// (a cyclic dependency). Resolving these symlinked workspace packages through
6+
// `node_modules` used to panic during package.json module resolution.
7+
{
8+
"tempDir": true,
9+
"if": "unix",
10+
"steps": [
11+
{
12+
"args": "run -A setup.js",
13+
"output": "[WILDCARD]"
14+
},
15+
{
16+
"args": "test -A",
17+
"output": "test.out"
18+
}
19+
]
20+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"imports": {
3+
"main-project": "./packages/main-project/src/index.ts",
4+
"pkg-a": "./packages/pkg-a/src/index.ts",
5+
"pkg-b": "./packages/pkg-b/src/index.ts"
6+
},
7+
"test": {
8+
"include": ["**/src/**/*.spec.ts"]
9+
}
10+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"name": "basic-npm-monorepo",
3+
"version": "1.0.0",
4+
"type": "module",
5+
"workspaces": ["packages/*"],
6+
"dependencies": {
7+
"main-project": "*",
8+
"pkg-a": "*",
9+
"pkg-b": "*"
10+
}
11+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "main-project",
3+
"version": "1.0.0",
4+
"type": "module",
5+
"dependencies": { "pkg-a": "*", "pkg-b": "*" },
6+
"exports": { ".": "./src/index.ts" }
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { packageA } from "pkg-a";
2+
import { packageB } from "pkg-b";
3+
4+
export function test() {
5+
packageA();
6+
packageB();
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "pkg-a",
3+
"version": "1.0.0",
4+
"type": "module",
5+
"dependencies": { "pkg-b": "*" },
6+
"exports": { ".": "./src/index.ts" }
7+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { packageAFromPackageB } from "pkg-b";
2+
3+
export function packageA() {
4+
console.info("invoked pkg-a.packageA");
5+
packageAFromPackageB();
6+
}
7+
8+
export function packageBFromPackageA() {
9+
console.info("invoked packageBFromPackageA");
10+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { packageA } from "pkg-a";
2+
import { packageB } from "pkg-b";
3+
import { test } from "main-project";
4+
5+
Deno.test("cyclic workspace packages resolve through node_modules symlinks", () => {
6+
packageA();
7+
packageB();
8+
test();
9+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "pkg-b",
3+
"version": "1.0.0",
4+
"type": "module",
5+
"dependencies": { "pkg-a": "*" },
6+
"exports": { ".": "./src/index.ts" }
7+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { packageBFromPackageA } from "pkg-a";
2+
3+
export function packageB() {
4+
console.info("invoked pkg-b.packageB");
5+
packageBFromPackageA();
6+
}
7+
8+
export function packageAFromPackageB() {
9+
console.info("invoked packageAFromPackageB");
10+
}

0 commit comments

Comments
 (0)