Skip to content

Conversation

YodaDaCoda
Copy link

Fixes #7622

The linked issue describes the problem in detail, I won't replicate that here.

@YodaDaCoda YodaDaCoda requested a review from a team as a code owner May 20, 2025 11:07
@owlstronaut
Copy link
Contributor

@YodaDaCoda Are you able to add a test that would fail before the fix?

@YodaDaCoda YodaDaCoda changed the title fix(arborist): don't checkEnging/checkPlatform for extraneous deps fix(arborist): don't checkEngine/checkPlatform for extraneous deps May 21, 2025
@YodaDaCoda
Copy link
Author

YodaDaCoda commented May 21, 2025

I have made an attempt... but it doesn't work how I expect it to. The test passes whether or not the fix is in place (i.e. it doesn't throw). I need help from someone more familiar with the codebase to figure out how to properly trigger the condition.

@wraithgar
Copy link
Member

Strict mode throws. You can pass in the values and set strict to true. A good example is here

@YodaDaCoda
Copy link
Author

I worded that very poorly. It probably would throw, if the problematic dep wasn't being pruned. Not clear to me why it's pruned in the test, but not in Real Life. The package.json / package-lock.json probably need to be tweaked somehow.

It's the checkPlatform() method that I've experienced causing the error, not checkEngine(), so the engineStrict option doesn't help.

That example you've shared does help me clean up the test somewhat though, thanks.

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] npm ci erroneously installs optional OS-constrained transitive dependency through direct shrinkwrap dependency
3 participants