Skip to content
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

[BUG] npm install --package-lock-only does not imply --package-lock #2747

Closed
ljharb opened this issue Feb 21, 2021 · 6 comments · Fixed by #3684
Closed

[BUG] npm install --package-lock-only does not imply --package-lock #2747

ljharb opened this issue Feb 21, 2021 · 6 comments · Fixed by #3684
Assignees
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion Release 7.x work is associated with a specific npm 7 release

Comments

@ljharb
Copy link
Collaborator

ljharb commented Feb 21, 2021

Current Behavior:

put package-lock=false in .npmrc.

Run npm install --package-lock-only.

Expected Behavior:

a package-lock.json is generated.

Steps To Reproduce:

See above.

Environment:

  • npm: 7.5.4

This is a regression from #146 / v6.9.0.

(possibly related to #2358)

@ljharb ljharb added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Feb 21, 2021
@ljharb
Copy link
Collaborator Author

ljharb commented Mar 23, 2021

cc @wraithgar @darcyclarke; can this be triaged?

@wraithgar
Copy link
Member

Asking to install from the package-lock-only when you have disabled the package-lock seems like a paradox.

The description for package-lock config:

If set to false, then ignore package-lock.json files when installing. This will also prevent writing package-lock.json if save is true.

The description for package-lock-only config:

If set to true, it will update only the package-lock.json, instead of checking node_modules and downloading dependencies.

When we try this we get this warning on the cli

npm WARN old lockfile
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile

Which is also technically incorrect. I don't know what the right behavior is here but I feel like we should explicitly bail here when being asked to resolve a logical paradox.

Looking through the original issue it seems this was all fixing a symptom, not a problem, and that was that npm audit couldn't work in environments where you had disabled package-locks. We should fix the problem itself.

@wraithgar wraithgar added Needs Discussion is pending a discussion and removed Needs Triage needs review for next steps labels Mar 23, 2021
@wraithgar
Copy link
Member

wraithgar commented Mar 23, 2021

Even if we want to resolve the two requests, the package-lock directive is the more explicit. Having package-lock-only implicitly change the explicit package-lock value if it has been set is very counterintuitive.

@ljharb
Copy link
Collaborator Author

ljharb commented Mar 23, 2021

@wraithgar the right behavior is what npm 6 eventually did - package-lock-only implies package-lock. This was an unintentional breaking change in v7.

Even if npm audit worked (as it should always have) without a lockfile, I’d still have this use case - all my packages have the lockfile disabled, and i use package-lock-only to generate a “before” and “after” lockfile to triage bugs caused by dependency updates.

@Schweinepriester
Copy link

Having this issue as well using node 15.14.0/npm 7.7.6.

I've got

package-lock=false
save=false

in my (global) .npmrc.
Previously using node 14/npm 6 I've explicitly used npm i --package-lock-only to get a package-lock.json if I wanted one.

save=false alone (without package-lock=false) also appears to block package-lock.json from being generated, so this issue could be broadened to

  • put package-lock=false or save=false in .npmrc

or should that aspect be its own issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@wraithgar @ljharb @nlf @darcyclarke @Schweinepriester and others