Skip to content

fix(arborist): add missing top property to isolated reifier proxy nodes#9064

Merged
wraithgar merged 1 commit intonpm:release/v10from
manzoorwanijk:fix/omit-dev-linked-strategy
Mar 5, 2026
Merged

fix(arborist): add missing top property to isolated reifier proxy nodes#9064
wraithgar merged 1 commit intonpm:release/v10from
manzoorwanijk:fix/omit-dev-linked-strategy

Conversation

@manzoorwanijk
Copy link
Contributor

@manzoorwanijk manzoorwanijk commented Mar 5, 2026

In continuation of our exploration of using install-strategy=linked in the Gutenberg monorepo, which powers the WordPress Block Editor.

##Summary

Fixes npm install --omit=dev crashing with install-strategy=linked in workspace monorepos.

addOmitsToTrashList iterates inventory nodes and accesses node.top.isProjectRoot, but the isolated reifier's proxy nodes for the root, workspace, and workspace link were missing the top property, causing a TypeError.

  • Add top (self-reference) to root and workspace proxy nodes
  • Add top: root to workspace link proxy nodes
  • Add isWorkspace: true to workspace proxy nodes so their dev deps can be omitted

References

Fixes #9063

@manzoorwanijk manzoorwanijk requested a review from a team as a code owner March 5, 2026 16:13
@wraithgar
Copy link
Member

This is exactly the kind of thing we knew to expect with all of the pseudo node/link/etc objects that the isolated reifier was making. Thanks for picking this up.

Is there a reason this is a PR to v10? Usually we land things in latest first and backport if needed.

@manzoorwanijk
Copy link
Contributor Author

Is there a reason this is a PR to v10? Usually we land things in latest first and backport if needed.

Explained in the linked issue

Note: this is v10-only. npm 11 removed addOmitsToTrashList and moved omit handling into the diff calculation.

@wraithgar
Copy link
Member

Would it be preferable to pull in that npm 11 change instead?

@manzoorwanijk
Copy link
Contributor Author

Would it be preferable to pull in that npm 11 change instead?

I did think about it first but the npm 11 change (f6c868d) touches 5 files (reify.js, diff.js, node.js, snapshot, tests) and restructures how omit is calculated — moving it from addOmitsToTrashList in reify into Diff.calculate. It's a cleaner solution but a bigger backport.

This PR is a minimal fix (3 lines + test) scoped to the crash only. Happy to go either way — I can attempt the backport if you'd prefer that approach, or we can land this as a targeted fix for v10.

Separately, I have a fix for a related issue on latest: --omit=dev is silently ignored with the linked strategy because the isolated reifier never consults the omit option. See #9066

@wraithgar
Copy link
Member

I did think about it first

This is what we really want here. That's good enough for us! Thanks.

@wraithgar wraithgar merged commit 2d83231 into npm:release/v10 Mar 5, 2026
19 checks passed
@manzoorwanijk manzoorwanijk deleted the fix/omit-dev-linked-strategy branch March 5, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants