fix(nix): pin pnpm_10 through fetchPnpmDeps and pnpmConfigHook#3990
Conversation
The flake's pnpm_10 override built a 10.33.2 binary, but only added
it to nativeBuildInputs. Both fetchPnpmDeps (the deps-fetch phase)
and pnpmConfigHook (the install phase) were silently falling back to
pkgs.pnpm, which has moved to 11.x on nixpkgs-unstable. That
triggers ERR_PNPM_UNSUPPORTED_ENGINE since package.json pins
>=10.33.2 <11.
Thread pnpm_10 into both:
- pnpmConfigHook.overrideAttrs replaces its hardcoded
propagatedBuildInputs = [ pnpm ] with [ pnpm_10 ].
- fetchPnpmDeps now receives pnpm = pnpm_10 explicitly
(its default is pkgs.pnpm).
Verified by rebuilding .#daemon.pnpmDeps and .#web.pnpmDeps
against current unstable, both succeed.
lefarcen
added
size/S
lefarcen
left a comment
lefarcen
left a comment
There was a problem hiding this comment.
Hey @GodTamIt! 👋
Tracking down why fetchPnpmDeps and pnpmConfigHook both silently fall back to pkgs.pnpm — even while the flake's pnpm_10 override was already in nativeBuildInputs — is a subtle build bug. The overrideAttrs approach on pnpmConfigHook (appending pnpm_10 to propagatedBuildInputs rather than replacing the list, so the upstream's writable-tmpdir-as-home-hook survives) and the explicit pnpm = pnpm_10 argument on fetchPnpmDeps are the right Nix idioms. Both derivations (package-daemon.nix and package-web.nix) handled consistently.
"Validate Nix flake" CI passed ✅ — that's the authoritative signal for a change like this.
Flagging for our reviewer queue — nothing blocking from my read.
lefarcen
added
needs-validation
|
@GodTamIt friendly reminder: this PR has been waiting on an author response for more than 3 days after reviewer or maintainer feedback. When you have a chance, please reply here or push an update. To keep the queue manageable, PRs with no author activity for more than 5 days after feedback may be closed automatically, but they can be reopened when work resumes. |
|
Still waiting for a real review |
|
Hey @GodTamIt — fair call. The earlier bot pass here was triage, not the human/pool review you're asking for. I've requested @nettee for a real code review on this PR. One thing that will help that handoff move faster: since this is labeled |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
I verified the new fetchPnpmDeps pin against upstream nixpkgs and left one non-blocking comment about the paired pnpmConfigHook override. The pnpm = pnpm_10 part looks like the real fix; the hook override appears to add brittle coupling to nixpkgs internals without changing the install-phase pnpm selection on current nixpkgs.
|
Hey @GodTamIt — the real review is in now from @nettee. The main actionable point is narrow and non-blocking: the explicit If you want to tighten this PR up, I'd start by replying on that thread or pushing the smaller version of the fix @nettee suggested. And please still fill in the Bug fix verification section in the PR body so the next pass has the repro/validation seam spelled out. |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@GodTamIt I re-checked the current head against the changed Nix ranges and verified the important part of the fix is now the explicit pnpm = pnpm_10 pin on both fetchPnpmDeps call sites. I also spot-checked current nixpkgs: fetchPnpmDeps still defaults its own pnpm argument, while pnpmConfigHook now looks up pnpm from PATH, so keeping pnpm_10 in nativeBuildInputs and narrowing the code change to the dep-fetch pin is the right shape here. Nice cleanup on the follow-up revision.
🎉 📡 You just leveled up to Giotto
🙌 Your contributions are sending a clear signal across the network: you care about making Open Design better. Keep transmitting. 💛 Thanks for helping Open Design move forward. Keep building in the open. 🚀 📊 Rank #98 among 100+ contributors |
|
Hi @GodTamIt! Your first Open Design PR has been merged! Huge thanks for jumping in and improving the project! You contributed: Merged PR: #3990 fix(nix): pin pnpm_10 through fetchPnpmDeps and pnpmConfigHook This PR improved project setup reliability, which is valuable because small environment fixes remove friction for everyone else building or testing locally. That kind of systems context usually transfers well into adjacent configuration issues. For your next contribution, we picked two issues that look like a good follow-up:
It is another infrastructure-facing issue with a clear settings and filesystem angle, so it fits the same practical problem space.
It also sits near CLI and local-environment behavior, which makes it a reasonable next step after your setup-focused fix. If one of these looks interesting, feel free to comment /claim on the issue and we will help you get started! Once your second PR gets merged, you will move into our Continuous Contributor tier. We are also starting to highlight repeat contributors more actively in the community, so this is a great time to keep going! Thanks again for the first PR, and welcome to the Open Design contributor community! The Open Design team P.S. We hang out in Discord — come say hi: https://discord.gg/3C6EWXbdQQ |
5 participants
Why
The flake's pnpm_10 override built a 10.33.2 binary, but only added it to nativeBuildInputs. Both fetchPnpmDeps (the deps-fetch phase) and pnpmConfigHook (the install phase) were silently falling back to pkgs.pnpm, which has moved to 11.x on nixpkgs-unstable. That triggers ERR_PNPM_UNSUPPORTED_ENGINE since package.json pins
What users will see
Users with nix flake setups for their system that have later nixpkgs versions will now be able to build their flake with
open-design.Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-pack/tools-prflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolTRANSLATIONS.mdfor the locale workflow)package.json(dependenciesordevDependencies); workspace-packagepackage.jsonfiles are out of scope. Include a paragraph on what we get vs. what bytes we ship (seeCONTRIBUTING.md→ Code style)Bug verification
Use this in a Nix flake with a nixpkgs hash pin on the latest
unstable. You can see thatpnpmversion is now 11+. Without this change, you'll encounter pnpm errors.Validation