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 modifies yarn.lock in incorrect ways #5126

Open
2 tasks done
victorb opened this issue Jul 6, 2022 · 17 comments
Open
2 tasks done

[BUG] npm install modifies yarn.lock in incorrect ways #5126

victorb opened this issue Jul 6, 2022 · 17 comments
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 8.x work is associated with a specific npm 8 release

Comments

@victorb
Copy link

victorb commented Jul 6, 2022

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

If you run npm install in a project that has a yarn.lock file, npm changes both the syntax, data and order of yarn.lock file.

Changes I've spotted that shouldn't happen:

  • npm adds double-quotes around everything (is-number@^7.0.0: becomes "is-number@^7.0.0":, version becomes "version" and so on)
  • Registry URLs get overwritten ("https://registry.yarnpkg.com/is-number/-/is-number-7.0.0.tgz#7535345b896734d5f80c4d06c50955527a14f12b" becomes "https://registry.npmjs.org/is-number/-/is-number-7.0.0.tgz")
  • Order of keys becomes shuffled (yarn.lock has version, resolved, integrity in that order, after npm install, the order becomes integrity, resolved, version)

Expected Behavior

No commands run with npm should modify files npm doesn't have anything to do with, namely yarn.lock which is managed by a different program than npm.

Steps To Reproduce

  1. cd $(mktemp -d) Create new temporary directory for a test project
  2. npm init --yes Create new package.json
  3. npm install --save is-number Add a dependency
  4. yarn install Install dependencies via yarn, creating the yarn.lock file
  5. cp yarn.lock yarn.lock.original Save a copy of the original yarn.lock file
  6. npm install Run npm install again which modifies the yarn.lock file unexpectedly
  7. diff yarn.lock yarn.lock.original show the difference between the npm-modified yarn.lock file with the original one that yarn itself produces

Environment

  • npm: 8.13.2
  • Node.js: v18.4.0
  • OS Name: Arch Linux
  • System Model Name: Desktop
  • npm config:
; "user" config from /home/user/.npmrc

//registry.npmjs.org/:_authToken = (protected)

; node bin location = /home/user/.nvm/versions/node/v18.4.0/bin/node
; node version = v18.4.0
; npm local prefix = /tmp/tmp.YKcr2lMqCS
; npm version = 8.13.2
; cwd = /tmp/tmp.YKcr2lMqCS
; HOME = /home/user
; Run `npm config ls -l` to show all defaults.
@victorb victorb added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Jul 6, 2022
@victorb
Copy link
Author

victorb commented Jul 12, 2022

Is this something the npm cli team would want to have fixed? I might attempt submitting a patch for it if so, but some confirmation that the team sees it as an issue (and it's not on purpose) first would be nice so I could avoid wasting time on a patch, if it has no chance of being accepted. CC @wraithgar @ljharb

@nlf
Copy link
Contributor

nlf commented Jul 13, 2022

i think there's two separate issues here.

  1. npm install allows changes to happen to your lock file, this differs from yarn install and can be a source of confusion. i suspect that npm ci (an alias for npm clean-install) will give you the behavior you're looking for. this behavior won't change for a while yet, but we do hope to make the ci style "strictly obey the lock file" behavior the default in the future.
  2. npm changes both the syntax, data and order of yarn.lock file. THIS is a bug for sure. our goal was to play nicely with the yarn.lock, but if we're not doing that then we should fix it.

i'm curious, is this something that's surfacing now because yarn changed the format of their lock file? it's very probable that our compatibility layer there is old and out of date, but if we change it we need to make sure it plays nicely with older formats as well.

Is this something the npm cli team would want to have fixed? I might attempt submitting a patch for it if so, but some confirmation that the team sees it as an issue (and it's not on purpose) first would be nice so I could avoid wasting time on a patch, if it has no chance of being accepted

we would absolutely love to have your help getting this fixed! to give you a bit of a head start, the relevant code is found here

reading the comments there, it looks like we explicitly chose to not use the canonical @yarnpkg/lockfile package due to it being very large. that being the case it would definitely be preferable to not bring in that dependency and fix our own in house implementation instead, unless there's another parser or the official one has changed to reduce its size. either way, you are always more than welcome to open a pull request to fix this or any other bug and we'd be happy to help answer questions so we can support you in this effort!

@nlf nlf added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Jul 13, 2022
@nlf nlf changed the title [BUG] npm install should not modify yarn.lock [BUG] npm install modifies yarn.lock in incorrect ways Jul 13, 2022
@lengerrong
Copy link

notice the same issue, any update on the thread?

@wraithgar
Copy link
Member

npm ci is also affected, it looks like the yarn.lock is modified even then.

cf #5317

@derekpiasecki
Copy link

Since #5317 has been combined with this ticket, please make sure that npm ci does not open yarn.lock with permission for writing (not modifying the file is not sufficient).

@alamothe
Copy link

Is there a command line option to prevent this obnoxious behavior?

@Blackclaws
Copy link

@nlf This issue is even worse when using a yarn v2 lockfile that has a completely different format. There should be a simple option for npm to ignore yarn.lock completely. Both for restore and for writing. This issue comes up if you build a workspace with both yarn and npm managed packages. This happens if you combine multiple packages for local development in an environment where different teams use different package managers.

@cachius
Copy link

cachius commented Oct 14, 2022

Workaround for key order
Run yarn install after npm install/update/whatever

This way you don't completely mess up the history of your yarn.lock file

@alamothe
Copy link

alamothe commented Oct 16, 2022

I mean, the workaround is just to restore the file with git restore yarn.lock after npm is run, but it gets boring.

@cachius
Copy link

cachius commented Oct 24, 2022

@alamothe But then you lose any legit changes made by npm, too, like npm install/update/... . If not many you can carry them over manually.

@wraithgar @shalvah Thanks for taking care of this in PR#5724.

@cachius
Copy link

cachius commented Nov 2, 2022

@victorb Fixed in #5751 in v9.0.1 by @wraithgar
@alamothe Can be closed.

@wraithgar
Copy link
Member

According to #5659 this doesn't address all of the issues, only some.

@alamothe
Copy link

alamothe commented Nov 7, 2022

@cachius what is the use case for using npm to manage yarn.lock? Why not just use yarn?

@boatilus
Copy link

boatilus commented Nov 8, 2022

One use case is installing listed peer dependencies via npm in a Yarn project with the --no-save option (to allow, for example, running unit tests with peer dependencies available), as the Yarn team has refused to implement a similar option. Even with the --no-save option, npm still incorrectly modifies yarn.lock.

@ramblingenzyme
Copy link

ramblingenzyme commented Jan 16, 2023

Why on earth is NPM even changing the lockfile. We're using npm patch in CI to update a package.json and push a tag for the new version, and it not only does an install (which might be messing up our node_modules directory) it's doing this rewrite to yarn.lock. (Of course the Yarn 3 version command works differently, so we haven't moved to using that at this point)

The big issue however, is that it's a Yarn 3 project, with a v6 lockfile, and then NPM overwrites it fully with a v1 lockfile which broke the rest of our pipeline.

@brycetham
Copy link

I find that this behavior breaks our CI pipeline, since we run npm install yarn to install yarn locally, and then run yarn install, which ends up upgrading a bunch of dependencies instead of using the version specified in yarn.lock.

stianjensen added a commit to stianjensen/sheplays-browser that referenced this issue Jul 29, 2023
It was incorrectly modified by npm install:
npm/cli#5126
@Santoshraj2 Santoshraj2 self-assigned this Oct 2, 2023
@Santoshraj2 Santoshraj2 removed their assignment Dec 1, 2023
@zcrkey
Copy link

zcrkey commented Jun 7, 2024

what is the use case for using npm to manage yarn.lock? Why not just use yarn?
It's quite confusing, as normally yarn.lock should be managed by Yarn, not by npm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

No branches or pull requests