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

fix: shrinkwrap setting incorrect lockfileVersion #3978

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

lukekarrys
Copy link
Contributor

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.

@lukekarrys lukekarrys requested a review from a team as a code owner November 3, 2021 00:42
@lukekarrys lukekarrys changed the base branch from latest to release-next November 3, 2021 00:42
@lukekarrys
Copy link
Contributor Author

@wraithgar I realize this conflicts with #3959, so I'm happy to hold off and fix my conflicts after that is merged.

@lukekarrys lukekarrys marked this pull request as draft November 3, 2021 20:26
@lukekarrys
Copy link
Contributor Author

Converting to a draft until #3959 is merged.

@lukekarrys lukekarrys marked this pull request as ready for review November 3, 2021 21: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.

PR-URL: #3978
Credit: @lukekarrys
Close: #3978
Reviewed-by: @wraithgar
@lukekarrys lukekarrys merged commit e5bfdac into release-next Nov 3, 2021
@wraithgar wraithgar deleted the lk/3962-shrinkwrap branch November 4, 2021 16:14
@wraithgar wraithgar mentioned this pull request Nov 4, 2021
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] --lockfile-version is not respected when using shrinkwrap and hidden lockfile
2 participants