-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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] --lockfile-version is not respected when using shrinkwrap and hidden lockfile #3962
Comments
(removed - this didn't actually work as I described it) |
@dominykas I can't reproduce this. Here's what I'm seeing: ❯ cat package.json
{
"name": "3962",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC",
"volta": {
"node": "14.18.1",
"npm": "8.1.2"
}
}
❯ npm --version
node 8.1.2
❯ node --version
v14.18.1
❯ ls -lha
total 4.0K
drwxr-xr-x 3 lukekarrys staff 96 Oct 29 11:16 .
drwxr-xr-x 45 lukekarrys staff 1.5K Oct 29 11:13 ..
-rw-r--r-- 1 lukekarrys staff 278 Oct 29 11:13 package.json
❯ npm shrinkwrap --lockfile-version=2
npm notice created a lockfile as npm-shrinkwrap.json
❯ cat npm-shrinkwrap.json| json .lockfileVersion
2
❯ ls -lha
total 8.0K
drwxr-xr-x 4 lukekarrys staff 128 Oct 29 11:14 .
drwxr-xr-x 45 lukekarrys staff 1.5K Oct 29 11:13 ..
-rw-r--r-- 1 lukekarrys staff 195 Oct 29 11:14 npm-shrinkwrap.json
-rw-r--r-- 1 lukekarrys staff 278 Oct 29 11:13 package.json |
@lukekarrys off topic, but what is |
It's this package from the registry: https://www.npmjs.com/package/json. It does less than |
Gotcha; that seems unwise since most people already have jq available and know what it is :-) (that package is not highly used) maybe if the examples used npx it’d be clearer? |
I agree that it should be made clearer and thanks for the reminder since this already come up in our docs: npm/documentation#45 I'm still leaning towards |
Are you sure you had no lockfile to start with? |
Yes, that's why I showed the output from the initial |
Oh, yeah, sorry, I missed the I experimented a little more. I was able to see the behavior you're seeing on an empty folder, however when I ran
(that last one I find a bit interesting) |
Do you have Here's my full results (without ❯ ls -al
total 4
drwxr-xr-x 3 lukekarrys staff 96 Nov 1 13:04 .
drwxr-xr-x 45 lukekarrys staff 1440 Oct 29 11:13 ..
-rw-r--r-- 1 lukekarrys staff 218 Nov 1 13:04 package.json
❯ cat package.json
{
"name": "3962",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC"
}
❯ npm i lodash
added 1 package, and audited 2 packages in 1s
found 0 vulnerabilities
❯ npm shrinkwrap --lockfile-version=2
npm notice package-lock.json has been renamed to npm-shrinkwrap.json
❯ cat npm-shrinkwrap.json | npx json .lockfileVersion
2
❯ cat node_modules/.package-lock.json | npx json .lockfileVersion
2 And if I do ❯ npm i lodash --package-lock=false
added 1 package, and audited 2 packages in 1s
found 0 vulnerabilities
❯ npm shrinkwrap --lockfile-version=2
npm notice created a lockfile as npm-shrinkwrap.json
❯ cat npm-shrinkwrap.json | npx json .lockfileVersion
3
❯ cat node_modules/.package-lock.json | npx json .lockfileVersion
2 |
Yes, I do have it set globally. |
Thanks! I updated the issue title to more accurately reflect what's happening and can confirm this is a bug that we should fix. |
I also found that when update a shrinkwrap file, we sometimes show a message with the incorrect lockfile version, even if the file did get updated correctly. ❯ ls -lha
total 12K
drwxr-xr-x 5 lukekarrys staff 160 Nov 2 07:47 .
drwxr-xr-x 46 lukekarrys staff 1.5K Nov 2 01:37 ..
-rw-r--r-- 1 lukekarrys staff 648 Nov 2 07:47 package-lock.json
-rw-r--r-- 1 lukekarrys staff 266 Nov 2 07:45 package.json
❯ cat package-lock.json | npx json .lockfileVersion
2
❯ npm shrinkwrap --lockfile-version=3
npm notice package-lock.json has been renamed to npm-shrinkwrap.json
❯ npm shrinkwrap --lockfile-version=3
npm notice npm-shrinkwrap.json updated to version 2
❯ cat npm-shrinkwrap.json | npx json .lockfileVersion
3 |
Fix: #3962 When created from a hidden lockfile, shrinkwrap was always setting the lockfileVersion to 3. The shrinkwrap command also needed to be updated to log the correct message based on the lockfileVersion config instead of the static lockfileVersion. With these fixes, it was possible to log a better message whenever we are changing the lockfileVersion, either from a config or any existing lockfile. Lastly, the tests for shrinkwrap were completely refactored to use the real npm and test all conceivable scenarios, as well as removing usage of `util.promisify` in favor of `fs.promises` now that we've dropped support for Node 10.
Fix: #3962 When created from a hidden lockfile, shrinkwrap was always setting the lockfileVersion to 3. The shrinkwrap command also needed to be updated to log the correct message based on the lockfileVersion config instead of the static lockfileVersion. With these fixes, it was possible to log a better message whenever we are changing the lockfileVersion, either from a config or any existing lockfile. Lastly, the tests for shrinkwrap were completely refactored to use the real npm and test all conceivable scenarios, as well as removing usage of `util.promisify` in favor of `fs.promises` now that we've dropped support for Node 10.
Fix: #3962 When created from a hidden lockfile, shrinkwrap was always setting the lockfileVersion to 3. The shrinkwrap command also needed to be updated to log the correct message based on the lockfileVersion config instead of the static lockfileVersion. With these fixes, it was possible to log a better message whenever we are changing the lockfileVersion, either from a config or any existing lockfile. Lastly, the tests for shrinkwrap were completely refactored to use the real npm and test all conceivable scenarios, as well as removing usage of `util.promisify` in favor of `fs.promises` now that we've dropped support for Node 10.
🙇 |
Is there an existing issue for this?
Current Behavior
$ npm shrinkwrap --lockfile-version=2
produces a v3 lockfile.v8.1.1
used to print a warning, but #3949 changed that.v8.1.2
just produces a v3 lockfile without any warnings.Expected Behavior
lockfile-version
param/config should be respected.Steps To Reproduce
$ rm -rf npm-shrinkwrap.json
(if the shrinkwrap exists - it seems to work OK)$ npm shrinkwrap --lockfile-version=2
$ cat npm-shrinkwrap.json | jq .lockfileVersion
Actual: 3
Expected: 2
Environment
The text was updated successfully, but these errors were encountered: