Skip to content

chore: add packageManager field to support corepack #46756

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

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

Jack-Works
Copy link
Contributor

Corepack is a new package manager manager that default ships in Node 16.9.0.

It will select the correct NPM version based on the "packageManager" field in the package.json

I added this field and set it to 6.14.15. It's the current latest NPM version that uses NPM lockfile version 1.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Nov 10, 2021
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@sandersn sandersn added the Housekeeping Housekeeping PRs label Dec 9, 2021
@sandersn sandersn requested review from jakebailey and orta December 9, 2021 00:59
Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems correct to me; I was going to ask about forward compatibility (I don't know if anyone's testing npm 6 with future versions of node, or if they will push patches to keep it going), but that doesn't really apply if we currently use the v1 lock file, and 7+ don't do it.

@Jack-Works
Copy link
Contributor Author

Yeah, if anyone runs npm install in TS repo with NPMv7 or higher, they will have an extra step to resolve all dependencies, and the lock file will be upgraded to V2. Actually I suggest to use lockfile v2 since this only affects the developers of TypeScript

@jakebailey
Copy link
Member

IMO it's not a good idea to upgrade to npm 7+ if we're still supporting LTS editions that come with npm 6 (otherwise, CI has to do further upgrades or face the "trying my best with v2" message), but it will be a good while until it's totally unsupported, so that's probably not a great motivator.

I know we have benchmarks and other CI that need to run on older versions of node (I think back to 10), but I think npm 7 can do node 10 (whereas npm 8 is node 12+, the current LTS).

Potentially once the lockfile is bumped to v2, we could drop this corepack entirely to not over-restrict (so long as the v2 format is fixed and agrees between npm 7 and npm 8, and so on).

@orta
Copy link
Contributor

orta commented Dec 9, 2021

Agree, this is probably the right call until we move everything over to npm 7

@orta orta merged commit 9b5abac into microsoft:main Dec 9, 2021
@Jack-Works Jack-Works deleted the corepack branch December 9, 2021 14:55
mprobst pushed a commit to mprobst/TypeScript that referenced this pull request Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug Housekeeping Housekeeping PRs
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants