Skip to content

fix: use omit when checking ideal tree engine, centralize shouldOmit logic #8372

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.

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

Merged
merged 1 commit into from
Jun 26, 2025

Conversation

owlstronaut
Copy link
Contributor

@owlstronaut owlstronaut commented Jun 20, 2025

fixes #8369
fixes #6805

Fix: Skip engine validation for omitted dependencies

Problem

When running npm install --omit=dev, npm incorrectly validates engine requirements for devDependencies that won't be installed, causing unnecessary install failures. For example, a project with devDependencies requiring Node >=20.6.0 would fail to install on Node 18.x even when using --omit=dev.

Solution

Modified Arborist's engine validation to skip checking omitted dependencies. Added a centralized shouldOmit() method to the Node class and updated #checkEngineAndPlatform() to respect omit flags. Now npm install --omit=dev succeeds when devDependencies have incompatible engine requirements, while still validating engines for dependencies that will actually be installed.

Changes

  • lib/node.js: Added shouldOmit() method for consistent omit logic
  • lib/arborist/build-ideal-tree.js: Skip engine checks for omitted dependencies
  • lib/arborist/reify.js, lib/audit-report.js: Use centralized omit logic
  • Added comprehensive tests and fixtures for all omit scenarios

@owlstronaut owlstronaut marked this pull request as ready for review June 20, 2025 18:45
@owlstronaut owlstronaut requested a review from a team as a code owner June 20, 2025 18:45
@owlstronaut owlstronaut changed the title fix(arborist): use omit when building ideal tree, centralize shouldOmit logic fix(arborist): use omit when checking ideal tree engine, centralize shouldOmit logic Jun 20, 2025
@owlstronaut owlstronaut force-pushed the owlstronaut/centralize-should-omit branch from 33dc877 to d29f0a7 Compare June 25, 2025 21:47
@owlstronaut owlstronaut force-pushed the owlstronaut/centralize-should-omit branch from d29f0a7 to cfdbb4a Compare June 26, 2025 19:47
@wraithgar wraithgar changed the title fix(arborist): use omit when checking ideal tree engine, centralize shouldOmit logic fix: use omit when checking ideal tree engine, centralize shouldOmit logic Jun 26, 2025
@wraithgar wraithgar merged commit f163d01 into latest Jun 26, 2025
17 checks passed
@wraithgar wraithgar deleted the owlstronaut/centralize-should-omit branch June 26, 2025 20:20
@github-actions github-actions bot mentioned this pull request Jun 26, 2025
reggi pushed a commit that referenced this pull request Jun 27, 2025
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.

[BUG] devDependencies considered for minimum version despite --omit-dev [BUG] Option engine-strict=true is not compatible with --omit=dev
2 participants