Skip to content

fix(externals): force-trace named traceDeps to fix pnpm nested deps #4391

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.

Sign up for GitHub

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

Open
pi0x wants to merge 16 commits into
base: main
Choose a base branch
from
+28 −0
Open

fix(externals): force-trace named traceDeps to fix pnpm nested deps #4391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
741ed34
fix(externals): force-trace named traceDeps to fix pnpm nested deps
pi0 Jun 28, 2026
8d20efa
Merge branch 'main' into fix/trace-include
pi0 Jun 29, 2026
848ff04
Merge branch 'main' into fix/trace-include
pi0 Jun 29, 2026
f33f359
Merge branch 'main' into fix/trace-include
pi0 Jun 29, 2026
edb0473
Merge branch 'main' into fix/trace-include
pi0 Jun 30, 2026
00aa8ca
chore: update deps
pi0 Jun 30, 2026
8f2df5b
chore: remove unused devDependencies (#4302)
ulrichstark Jun 30, 2026
c87fc68
docs: update zerops provider docs (#4380)
Vanshika-Rana Jun 30, 2026
7427e3b
fix(vite): respect bun/deno export conditions in dev server (#4397)
pi0x Jun 30, 2026
7317d0f
fix(route-meta): add order:pre to route-meta plugin hooks (#4316)
kyjus25 Jun 30, 2026
8885036
test(public-assets): cover node reader path resolution and traversal …
pi0 Jun 30, 2026
f6034ac
refactor(deno): use default node handler for serveStatic (#4398)
pi0x Jun 30, 2026
3a661ba
presets(vercel): use reflinks for custom function dirs (#4373)
RihanArfan Jun 30, 2026
9ea0402
docs: add defineNitroPlugin to migration guide (#4400)
RihanArfan Jul 1, 2026
c94e3a1
chore: update deps
pi0 Jul 2, 2026
8ed9f73
Merge branch 'main' into fix/trace-include
pi0 Jul 2, 2026
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
8 changes: 8 additions & 0 deletions src/build/plugins/externals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export function externals(opts: ExternalsOptions): Plugin {
await traceNodeModules([...tracedPaths], {
...traceOpts,
fullTraceInclude: resolved?.fullTraceInclude,
traceInclude: resolved?.traceInclude,
conditions: opts.conditions,
rootDir: opts.rootDir,
writePackageJson: true, // deno compat
Expand Down Expand Up @@ -227,11 +228,18 @@ export function resolveTraceDeps(
const fullTraceInclude = [...new Set([...builtinFullTrace, ...userFullTrace])].filter(
(d) => !negated.has(d)
);
// Named (non-RegExp) deps to force-trace by name. nft cannot statically detect
// packages that are loaded dynamically (e.g. native bindings), so they are
// resolved and traced explicitly by `traceNodeModules`. This also makes them
// work under pnpm, where a nested dependency is not hoisted and only resolves
// from the dependent package's real `.pnpm` location.
const traceInclude = resolved.filter((d): d is string => typeof d === "string");
return {
includePattern: tracePattern
? new RegExp(`(?:^|[/\\\\]node_modules[/\\\\])(?:${tracePattern})(?:[/\\\\]|$)`)
: undefined,
fullTraceInclude: fullTraceInclude.length > 0 ? fullTraceInclude : undefined,
traceInclude: traceInclude.length > 0 ? traceInclude : undefined,
};
}

Expand Down
20 changes: 20 additions & 0 deletions test/unit/trace-deps.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@ describe("resolveTraceDeps", () => {
expect(result.fullTraceInclude).toContain("prisma");
});

it("returns named deps as traceInclude (builtins + user, RegExp excluded)", () => {
const result = resolveTraceDeps(["my-pkg", /my-.*-pkg/], defaults);
expect(result.traceInclude).toContain("sharp");
expect(result.traceInclude).toContain("canvas");
expect(result.traceInclude).toContain("my-pkg");
// RegExp entries cannot be resolved by name and must be excluded
expect(result.traceInclude!.every((d) => typeof d === "string")).toBe(true);
});
Comment on lines +43 to +50

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the exact traceInclude membership.

This test doesn't really prove the RegExp selector is excluded. An implementation that accidentally stringifies the regex would still pass typeof d === "string". Please assert the final members/count instead.

Suggested test tightening 
   it("returns named deps as traceInclude (builtins + user, RegExp excluded)", () => {
     const result = resolveTraceDeps(["my-pkg", /my-.*-pkg/], defaults);
-    expect(result.traceInclude).toContain("sharp");
-    expect(result.traceInclude).toContain("canvas");
-    expect(result.traceInclude).toContain("my-pkg");
-    // RegExp entries cannot be resolved by name and must be excluded
-    expect(result.traceInclude!.every((d) => typeof d === "string")).toBe(true);
+    expect(result.traceInclude).toEqual(
+      expect.arrayContaining(["sharp", "canvas", "my-pkg"])
+    );
+    expect(result.traceInclude).toHaveLength(3);
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("returns named deps as traceInclude (builtins + user, RegExp excluded)", () => {
const result = resolveTraceDeps(["my-pkg", /my-.*-pkg/], defaults);
expect(result.traceInclude).toContain("sharp");
expect(result.traceInclude).toContain("canvas");
expect(result.traceInclude).toContain("my-pkg");
// RegExp entries cannot be resolved by name and must be excluded
expect(result.traceInclude!.every((d) => typeof d === "string")).toBe(true);
});
it("returns named deps as traceInclude (builtins + user, RegExp excluded)", () => {
const result = resolveTraceDeps(["my-pkg", /my-.*-pkg/], defaults);
expect(result.traceInclude).toEqual(
expect.arrayContaining(["sharp", "canvas", "my-pkg"])
);
expect(result.traceInclude).toHaveLength(3);
});
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/unit/trace-deps.test.ts` around lines 43 - 50, The trace deps test is
too loose because it only checks that traceInclude entries are strings, which
would still pass if the RegExp selector were incorrectly stringified. Tighten
the assertion in trace-deps.test.ts around resolveTraceDeps to verify the exact
traceInclude membership and count for the ["my-pkg", /my-.*-pkg/] case, using
the existing defaults fixture and the resolveTraceDeps result to ensure only the
expected builtins plus "my-pkg" are present.


it("excludes negated packages from traceInclude", () => {
const result = resolveTraceDeps(["!sharp"], defaults);
expect(result.traceInclude).not.toContain("sharp");
expect(result.traceInclude).toContain("canvas");
});

it("returns undefined traceInclude when all deps are negated", () => {
const result = resolveTraceDeps(["!sharp", "!canvas"], defaults);
expect(result.traceInclude).toBeUndefined();
});

it("throws on bare ! selector", () => {
expect(() => resolveTraceDeps(["!"], defaults)).toThrow('Invalid traceDeps selector: "!"');
});
Expand Down
Loading