fix(arborist): omit failed optional dependencies from installed deps #8184

owlstronaut · 2025-03-26T15:30:57Z

… but keep them 'in the tree'

This PR was authored by @zkat

Fixes: #4828
Fixes: #7961
Replaces: #8127
Replaces: #8077

When optional dependencies fail, we don't want to install them, for whatever reason. The way this "uninstallation" is done right now is by simply removing the dependencies from the ideal tree during reification.

Unfortunately, this means that what gets saved to the "hidden lockfile" is the edited ideal tree as well, and when npm runs next, it'll use that when regenerating the "real" package-lock.

This PR tags failed optional deps such that they're still added to the "trash list" (and thus have their directories cleaned up by the reifier), but prevents Arborist from removing them altogether from the ideal tree (which is usually done by setting their parent to null, making them unreachable, and letting them get GC'd).

PS: It's Friday, this is what I managed to get done together. I'm making this a draft PR for now so folks can look at it, but I want to make sure both reifiers work well, fix some messaging issues, and clean stuff up (I'm pretty sure I'm guarding ideallyInert in more places than I need to, because I was trying to find the right spot). Still, feel free to talk about the approach. I'll get back to this on Monday.

PPS: also hi

workspaces/arborist/lib/shrinkwrap.js

workspaces/arborist/lib/arborist/reify.js

owlstronaut · 2025-03-28T01:07:08Z

@zkat I did what I could here. Had a couple of comments/lack of my own clarity in things I changed - see above.

wraithgar · 2025-03-31T15:27:54Z

Other than the comment we really really should re-use the exceptional description from #8177, either by copying it into this issue now or when we merge. Probably easier to do now. I'll do it now.

zkat · 2025-03-31T17:13:42Z

workspaces/arborist/lib/arborist/build-ideal-tree.js

      const set = optionalSet(node)
      for (const node of set) {
-        node.parent = null
+        node.ideallyInert = true


I've been thinking about this, and maybe it's better to just call it inert instead of ideallyInert. I think putting the ideally in there is probably a bit too much clever inside baseball and it's more verbose to boot. :)

Some other name would be good, too, as long as it communicates that this is in the tree, but we don't physically install it.

paulrutter · 2025-03-31T19:23:54Z

Thanks for pulling this work forward, much appreciated @zkat @owlstronaut @wraithgar.
Happy to see this is almost ready to get merged.

… but keep them 'in the tree' Fixes: #4828 Fixes: #7961 Replaces: #8127 Replaces: #8077 When optional dependencies fail, we don't want to install them, for whatever reason. The way this "uninstallation" is done right now is by simply removing the dependencies from the ideal tree during reification. Unfortunately, this means that what gets saved to the "hidden lockfile" is the edited ideal tree as well, and when NPM runs next, it'll use that when regenerating the "real" package-lock. This PR tags failed optional deps such that they're still added to the "trash list" (and thus have their directories cleaned up by the reifier), but prevents Arborist from removing them altogether from the ideal tree (which is usually done by setting their parent to `null`, making them unreachable, and letting them get GC'd).

jakebailey · 2025-04-03T15:53:44Z

Amazing, I'm so excited for this to be fixed. Thank you @zkat @owlstronaut @wraithgar 🎉

JounQin · 2025-04-20T03:29:30Z

@jakebailey Can this be back ported into old versions?

wraithgar · 2025-04-20T14:27:37Z

This is not going to be backported.

npm version 11.3.0 includes a fix (see: npm/cli#8184) that was pointed out in tailwindlabs/tailwindcss#15806 (comment) where it should resolve properly. See this thread for my own investigation parcel-bundler/lightningcss#956 (comment)

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 6cc703f

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7151 Task: 0 X-original-commit: 6cc703f Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 724f762

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7153 Task: 0 X-original-commit: 724f762 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: b1dc674

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7157 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7156 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7155 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7154 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Discovered while investigating #8535 (comment) Similar to #8566, relates to #8184 Moves `inert` (uninstallable optional) calculation into `buildIdealTree` so it can be used in diff. Also removes most of #8184 in favor of a [simpler fix](https://github.com/npm/cli/pull/8602/files#diff-02626074e1a4a170693607e4a3a69dfc08ee52067734717833b22cf162923e07R354), see PR comments for the journey. Improvements: * we don't see uninstallable packages as "installed" in CLI output * `createSparseTree` no longer creates dirs that will only be cleaned later For the example in the linked issue, it changes output from `added 156 packages` to `added 12 packages` and combined with #8537 it changes to `added 6 packages`, the expected result.

The `--no-package-lock` workaround was added due to npm bug #4828, where npm < 11.3.0 generates incomplete lockfiles for packages with optional platform dependencies (esbuild, rollup). Optional cross-platform dependencies were restored to `package-lock.json` in 358f276, so npm will be able to install from the lock file in the GitHub Actions. Also, fixed in npm 11.3.0 (Apr 2025), but Node v22 ships npm v10 and will remain affected out-of-the-box. Investigation notes follow. What happened? -------------- 1. Switch from yarn to npm: `package-lock.json` added, `yarn.lock` removed - modelcontextprotocol@702f827 Presumably: - run `npm install` to generate a `package-lock.json` from the yarn-managed `node_modules`, on macos - bug #4828: npm omitted optional cross-platform dependencies from the lock file 2. Pull 47, tries `npm ci`, and reverts, on 11 Nov 2024 modelcontextprotocol@3789ef9 - "Try restoring npm ci" --> testing the new node release for the bug? - ran against `setup-node`, `node-version: 18`, likely: 18.20.5 (released nov 11, 2024; ~same day) - git show 3789ef9:.github/workflows/main.yml - Failed action, and logs have expired - https://github.com/modelcontextprotocol/inspector/actions/runs/11782443393/job/32817472448 - https://nodejs.org/en/download/archive/v18.20.5 - uses npm 10.8.2 - Re-tried in inspector fork - workflow run at 3789ef9 - change `node-version: 18` to `18.20.5` (exact node / npm on commit date) - Fails due to missing `linux-x64-gnu` platform dep (rollup, would similarly affect esbuild) 3. Cross-platform dependencies restored to lockfile on 1 May 2025 modelcontextprotocol#372 - modelcontextprotocol@358f276 - worked because `package-lock.json` and `node_modules` were both removed - i.e., not the bug conditions -> even npm < 11.3.0 generates correct lockfile - At that point, `--no-package-lock` could've been removed from CI, Dockerfile, etc. NPM --- npm (aborist) fixed #4828 npm/cli#8184 - npm/cli#4828 --> frequent http 500, due to many comments - npm/cli@a96d8f6 - will not be backported Released in 11.3.0 on 8 Apr npm/cli#8150 - https://github.com/npm/cli/releases/tag/v11.3.0 - arborist 9.0.2 - https://github.com/npm/cli/releases/tag/arborist-v9.0.2 - npm v11.3.0 ships with node v24.2.0, on 6 May 2025 - https://nodejs.org/en/download/archive/v24 Node v22 ships npm v10 - https://nodejs.org/en/download/archive/v22 - will always be affected, no backport coming

The `--no-package-lock` workaround was added due to npm bug #4828, where npm < 11.3.0 generates incomplete lockfiles for packages with optional platform dependencies (esbuild, rollup). Optional cross-platform dependencies were restored to `package-lock.json` in 358f276, so npm will be able to install from the lock file in the GitHub Actions. Also, fixed in npm 11.3.0 (Apr 2025), but Node v22 ships npm v10 and will remain affected out-of-the-box. Investigation notes follow. What happened? -------------- 1. Switch from yarn to npm: `package-lock.json` added, `yarn.lock` removed - modelcontextprotocol@702f827 Presumably: - run `npm install` to generate a `package-lock.json` from the yarn-managed `node_modules`, on macos - bug #4828: npm omitted optional cross-platform dependencies from the lock file npm can't easily fix this. `npm install [email protected]` could write a resolved dependency, but at the exact version so `foo` would be pinned, and semver operator twiddling would be required to restore it as-was. The fix would be to "manually" re-add the missing metadata. 2. Pull 47, tries `npm ci`, and reverts, on 11 Nov 2024 modelcontextprotocol@3789ef9 - "Try restoring npm ci" --> testing the new node release for the bug? - ran against `setup-node`, `node-version: 18`, likely: 18.20.5 (released nov 11, 2024; ~same day) - git show 3789ef9:.github/workflows/main.yml - Failed action, and logs have expired - https://github.com/modelcontextprotocol/inspector/actions/runs/11782443393/job/32817472448 - https://nodejs.org/en/download/archive/v18.20.5 - uses npm 10.8.2 - Re-tried in inspector fork - workflow run at 3789ef9 - change `node-version: 18` to `18.20.5` (exact node / npm on commit date) - Fails due to missing `linux-x64-gnu` platform dep (rollup, would similarly affect esbuild) 3. Cross-platform dependencies restored to lockfile on 1 May 2025 modelcontextprotocol#372 - modelcontextprotocol@358f276 - worked because `package-lock.json` and `node_modules` were both removed - i.e., not the bug conditions -> even npm < 11.3.0 generates correct lockfile - At that point, `--no-package-lock` could've been removed from CI, Dockerfile, etc. NPM --- npm (aborist) fixed #4828 npm/cli#8184 - npm/cli#4828 --> frequent http 500, due to many comments - npm/cli@a96d8f6 - will not be backported Released in 11.3.0 on 8 Apr npm/cli#8150 - https://github.com/npm/cli/releases/tag/v11.3.0 - arborist 9.0.2 - https://github.com/npm/cli/releases/tag/arborist-v9.0.2 - npm v11.3.0 ships with node v24.2.0, on 6 May 2025 - https://nodejs.org/en/download/archive/v24 Node v22 ships npm v10 - https://nodejs.org/en/download/archive/v22 - will always be affected, no backport coming

owlstronaut changed the title ~~fix(arborist): omit failed optional dependencies from installed deps, but keep them 'in the tree'~~ fix(arborist): omit failed optional dependencies from installed deps Mar 27, 2025

owlstronaut force-pushed the latest branch from 85ec0c9 to 26b6454 Compare March 27, 2025 18:03

owlstronaut commented Mar 28, 2025

View reviewed changes

workspaces/arborist/lib/shrinkwrap.js Show resolved Hide resolved

owlstronaut commented Mar 28, 2025

View reviewed changes

workspaces/arborist/lib/arborist/reify.js Outdated Show resolved Hide resolved

owlstronaut commented Mar 28, 2025

View reviewed changes

workspaces/arborist/lib/arborist/reify.js Outdated Show resolved Hide resolved

owlstronaut marked this pull request as ready for review March 31, 2025 14:45

owlstronaut requested a review from a team as a code owner March 31, 2025 14:45

zkat reviewed Mar 31, 2025

View reviewed changes

zkat mentioned this pull request Mar 31, 2025

fix(arborist): omit failed optional dependencies from installed deps,… #8177

Closed

owlstronaut force-pushed the owlstronaut/optionals-in-tree branch from f635c97 to 68a8d05 Compare April 1, 2025 21:17

wraithgar approved these changes Apr 3, 2025

View reviewed changes

owlstronaut merged commit a96d8f6 into latest Apr 3, 2025
16 checks passed

owlstronaut deleted the owlstronaut/optionals-in-tree branch April 3, 2025 15:45

github-actions bot mentioned this pull request Apr 3, 2025

chore: release 11.3.0 #8150

Merged

owlstronaut mentioned this pull request Apr 8, 2025

Don't prune optional dependencies #8077

Closed

jakebailey mentioned this pull request Apr 9, 2025

[BUG] Platform-specific optional dependencies not being included in package-lock.json when reinstalling with node_modules present #4828

Closed

2 tasks

thdk mentioned this pull request Apr 18, 2025

Nx release version skipLockFileUpdate no longer working since Nx 20.8 nrwl/nx#30793

Closed

Andrew-Chen-Wang mentioned this pull request Apr 22, 2025

Upgrade npm to latest in CI Andrew-Chen-Wang/template-nextjs#2

Merged

Marv51 mentioned this pull request May 5, 2025

NPM 11.3 update causes file not found error for platform specific dependencies TobyAndToby/generate-license-file#626

Closed

fw-bot mentioned this pull request Sep 17, 2025

[FIX] package: add swc binaries to optional dependencies odoo/o-spreadsheet#7151

Closed

14 tasks

fw-bot mentioned this pull request Sep 17, 2025

[FIX] package: add swc binaries to optional dependencies odoo/o-spreadsheet#7153

Closed

14 tasks

fw-bot mentioned this pull request Sep 17, 2025

[FIX] package: add swc binaries to optional dependencies odoo/o-spreadsheet#7154

Closed

14 tasks

fw-bot mentioned this pull request Sep 17, 2025

[FIX] package: add swc binaries to optional dependencies odoo/o-spreadsheet#7155

Closed

14 tasks

fw-bot mentioned this pull request Sep 17, 2025

[FIX] package: add swc binaries to optional dependencies odoo/o-spreadsheet#7156

Closed

14 tasks

fw-bot mentioned this pull request Sep 17, 2025

[FIX] package: add swc binaries to optional dependencies odoo/o-spreadsheet#7157

Closed

14 tasks

p2edwards mentioned this pull request Sep 23, 2025

chore(deps): bump storybook to 9.1.13 DEV-888 kobotoolbox/kpi#6080

Draft

9 tasks

liamcmitchell mentioned this pull request Sep 28, 2025

fix: refactor node.ideallyInert to node.inert #8602

Merged

eljonny mentioned this pull request Oct 15, 2025

Fix node22 lightningcss workaround colanode/colanode#231

Merged

uta-bogle mentioned this pull request Oct 24, 2025

🐛 Error: Cannot find module '@biomejs/cli-linux-x64/biome' on x64 linux host biomejs/biome#4467

Closed

1 task

caugner mentioned this pull request Dec 1, 2025

Bump Node.js in all repos from v22 to v24 mdn/mdn#772

Open

21 tasks

Fuzzy3 mentioned this pull request Dec 8, 2025

Release v11.4.2 kirbydesign/designsystem#4189

Merged

liamcmitchell mentioned this pull request Dec 8, 2025

[BUG] npm install produces different results on first and second run #8535

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(arborist): omit failed optional dependencies from installed deps #8184

fix(arborist): omit failed optional dependencies from installed deps #8184

Uh oh!

owlstronaut commented Mar 26, 2025 •

edited by wraithgar

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

owlstronaut commented Mar 28, 2025

Uh oh!

wraithgar commented Mar 31, 2025

Uh oh!

zkat Mar 31, 2025

Uh oh!

paulrutter commented Mar 31, 2025

Uh oh!

Uh oh!

jakebailey commented Apr 3, 2025

Uh oh!

JounQin commented Apr 20, 2025

Uh oh!

wraithgar commented Apr 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fix(arborist): omit failed optional dependencies from installed deps #8184

fix(arborist): omit failed optional dependencies from installed deps #8184

Uh oh!

Conversation

owlstronaut commented Mar 26, 2025 • edited by wraithgar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

owlstronaut commented Mar 28, 2025

Uh oh!

wraithgar commented Mar 31, 2025

Uh oh!

zkat Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

paulrutter commented Mar 31, 2025

Uh oh!

Uh oh!

jakebailey commented Apr 3, 2025

Uh oh!

JounQin commented Apr 20, 2025

Uh oh!

wraithgar commented Apr 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

owlstronaut commented Mar 26, 2025 •

edited by wraithgar

Loading