fix: stop optional dependencies from being added to prod inside lockfile

[rahulio96]

Summary

Currently, when updating an optional dependency, it's added to both the optional dependencies array and the production dependencies array inside the lockfile (package-lock.json).

Optional dependencies are getting added to production inside add-rm-pkg-deps.js. However, Issue #7530 suggests that this is NOT intended behavior, even though there are unit tests that ensure this behavior is maintained to support previous npm versions.

EDIT: This comment details changes made to the initial approach mentioned below.

This pull request removes the ability for add-rm-pkg-deps.js to add optional dependencies to the prod list and alters two tests to fit this change.

I'm looking to resolve the issue, but am unsure if these changes are the right way to go about this, and am very open to suggestions.

Testing

• Ran tap tests
• Created a test project and used aliasing to test changes

References

• Fixes [BUG] Wrong lock file modification for optional dependency #7530

[wraithgar]

This would be quite a breaking change if implemented. The correct change would be to not remove optional deps from the lockfile if they end up not being installed.

[rahulio96]

Sorry for the very late reply, but I just wanted to clarify something.

When you said, "remove optional deps from the lockfile" did you mean removing them from the prod array inside the lockfile or just removing them from the lockfile altogether?

If it's the latter, I'm not quite sure how it would help resolve the issue and would like to understand why.

[wraithgar]

I mean removing them from the lockfile altogether. What we're functionally wanting here is no different than if I did npm ci --omit=dev. The dev dependencies would not be reified, but they would also not be removed from the lockfile just because they weren't installed at this time.

[rahulio96]

I'm just struggling to see how this helps resolve the problem of an optional dep being added to the prod array inside the lockfile when directly installing said optional dep (e.g npm i <package> --save-optional). From what I understand, these suggestions don't seem to impact that behavior, unless there's something I'm missing.

[rahulio96]

Hi @wraithgar, I wanted to follow up on this discussion.

I appreciate your suggestions and am open to implementing them, but I'm still having trouble understanding how they address the issue. Also, from what I can tell with current behavior, if an optional dep is already in the lockfile and it fails installation, it will stay inside the lockfile. I want to better understand your perspective so any further clarification would be greatly appreciated.

On another note, I've also considered an alternate solution that removes the optional dep from the lockfile's prod array while saving the ideal tree during reify. It'd still be a breaking change, but it's a little less extreme than removing code from add-rm-pkg-deps.

If you have the time, any feedback would go a long way.

[rahulio96]

I haven't heard back in a while, so I'm undoing my previous commits and cautiously moving forward with the alternate solution I mentioned earlier but with a few modifications.

In this updated approach, if the optional dependency fails installation, it remains in both the prod and optional arrays inside the lockfile, so it can be installed again at a later date.

However, if the optional dependency successfully installs, then it is removed from the prod array inside the lockfile to reduce user confusion.

As always, I’m open to feedback and any necessary changes.