Skip to content

Keep pnpm links intact when running Deno - #1262

Merged
dahlia merged 1 commit into
fedify-dev:2.0-maintenancefrom
dahlia:bugfix/pnpm-pack
Oct 6, 2026
Merged

dahlia merged 1 commit into
fedify-dev:2.0-maintenancefrom
dahlia:bugfix/pnpm-pack

Conversation

@dahlia

@dahlia dahlia commented Oct 6, 2026

Copy link
Copy Markdown
Member

Deno 2.9 can replace pnpm's workspace links, leaving pnpm pack unable to resolve workspace: dependencies. Setting nodeModulesDir to "none" in deno.json keeps Deno's npm cache separate from pnpm's installation. Development dependencies are declared explicitly because Deno no longer supplies the links. Fresh loads its Vite config natively so its build continues to work.

An isolated regression test checks that Deno preserves pnpm's links and that pnpm pack still runs prepack and resolves workspace versions. CI also checks that nodeModulesDir remains "none".

Tested packing on Deno 2.7.7/2.9.7, affected package suites on Deno/Node.js/Bun, Fresh/docs builds, and Workers tests. The full Deno suite still reports a docloader timer leak; the parallel Node.js run stalled, though the isolated vocabulary suite passed.

Fixes #1259.

Keep Deno's npm cache separate from pnpm's node_modules so Deno 2.9
cannot remove workspace links before packing. Declare the development
dependencies previously supplied by Deno's root links, and load the
Fresh Vite config natively through an explicit npm entry point.

Add an isolated regression task that uses the root setting and checks
link preservation, prepack execution, and workspace version rewriting.
Check the cache setting in CI without installing fixture dependencies.

Codex implemented and tested the changes; Claude Code reviewed the
plan and the implementation.

Fixes fedify-dev#1259

Changelog: none
Assisted-by: Codex:gpt-6.1-sol
Assisted-by: Claude Code:claude-fable-5-1
@dahlia dahlia self-assigned this Oct 6, 2026
@dahlia dahlia added runtime/deno Deno runtime related component/build Build system and packaging labels Oct 6, 2026
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: da8e2f88-2816-4643-9e66-91812f796daf
📥 Commits

Reviewing files that changed from the base of the PR and between a2414f9 and 664b54a.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (8)
  • deno.json
  • docs/package.json
  • examples/fresh/deno.json
  • mise.toml
  • packages/fastify/package.json
  • packages/sqlite/package.json
  • packages/testing/package.json
  • scripts/test_pack.mjs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The change sets Deno’s nodeModulesDir to none and checks that setting. It updates workspace development dependencies and the Fresh example’s Vite tasks. A new integration test checks installed links and packed package metadata.

Changes

Workspace tooling

Layer / File(s) Summary
Set and check the Deno node_modules mode
deno.json, mise.toml
The root Deno configuration sets nodeModulesDir to none. The check task now fails if the setting differs.
Update workspace tooling declarations and commands
docs/package.json, packages/fastify/package.json, packages/sqlite/package.json, packages/testing/package.json, examples/fresh/deno.json
The package manifests add development dependencies. The Fresh example’s dev and build tasks run Vite through Deno with --configLoader native.
Test packing after Deno checks
mise.toml, scripts/test_pack.mjs
The new test:pack task runs an integration test. The test installs a temporary workspace, checks installed links before and after Deno, packs an adapter, and verifies the prepack side effect and packed peer dependency.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 664b5

The change sets Deno's nodeModulesDir to none to keep pnpm links intact, adds a regression test for it, and declares the workspace dev dependencies. No concrete merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (7 skipped: 7 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive Issue #1259 covers affected branches 2.0–2.3. This PR targets 2.0 and changes nodeModulesDir to "none"; its regression test checks pnpm links and packing. The available evidence does not establish… Provide evidence of the relevant configuration or equivalent protection on branches 2.1–2.3 to determine whether the full issue scope is covered.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: preserving pnpm workspace links when running Deno.
Description check ✅ Passed The description explains the Deno and pnpm link issue, the changes made, and the regression test and validation work.
Out of Scope Changes check ✅ Passed The added package development dependencies and Fresh Vite task changes support operation with Deno no longer managing node_modules. The regression test and CI check verify the linked issue’s fix. Th…
Full details: Linked Issues check

Explanation

Issue #1259 covers affected branches 2.0–2.3. This PR targets 2.0 and changes nodeModulesDir to "none"; its regression test checks pnpm links and packing. The available evidence does not establish whether branches 2.1–2.3 also preserve the links.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (7 skipped: 7 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dahlia
dahlia merged commit fb38d47 into fedify-dev:2.0-maintenance Oct 6, 2026
17 checks passed
@dahlia
dahlia deleted the bugfix/pnpm-pack branch October 6, 2026 13:06
@dahlia dahlia linked an issue Oct 6, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/build Build system and packaging runtime/deno Deno runtime related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pnpm pack fails on 2.0–2.3 maintenance branches after running Deno 2.9

1 participant