Skip to content

Updates package manager versions - #106

Merged
arcanis merged 2 commits into
mainfrom
mael/package-manager-updates
May 28, 2022
Merged

arcanis merged 2 commits into
mainfrom
mael/package-manager-updates

Conversation

@arcanis

@arcanis arcanis commented May 15, 2022

Copy link
Copy Markdown
Contributor

Upgrade package manager versions to their latest releases.

Comment thread config.json Outdated
},
"pnpm": {
"default": "6.29.1",
"default": "7.1.0",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ping @zkochan @aduh95, I'm not sure whether we can/should/must upgrade the default pnpm from 6 to 7, what do you think (I'm worried it would make it a breaking change for Node)?

Also cc @mcollina: with your proposal (automatically retrieving the latest versions), in a situation like this one, do you think Corepack should just upgrade from 6 to 7?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should aim for the latest, Corepack is still experimental in Node.js so breaking changes are still expected (and the upside is worth the potential breakage imo).

I think Matteo proposal was we should use a semver-range as the default (so 7.x in this instance), so on a fresh install of Node.js you're less likely to get a version that contains known security issues.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't know, whatever the Node.js team thinks is appropriate.
But do update pnpm to v7 in Node.js 18.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should use ^ by default, meaning that updating a major should be considered a breaking change. Exceptions should also be possible, like for npm 8, where the only breaking change wrt 7 was dropping node 10.

@arcanis arcanis mentioned this pull request May 24, 2022
@arcanis
arcanis requested a review from aduh95 May 24, 2022 15:41

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@arcanis arcanis mentioned this pull request May 24, 2022
Comment thread config.json
"definitions": {
"npm": {
"default": "8.4.0",
"default": "8.10.0",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Forgive my drive-by comment here, but I thought npm wasn't supported with corepack?

@merceyz merceyz May 24, 2022 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It has always been supported but not "enabled by default" when you run corepack enable, you can explicitly enable it with corepack enable npm
Ref #24

Note that the npm shims will not be installed unless explicitly requested, as npm is currently distributed with Node.js through other means.
https://github.com/nodejs/corepack/blob/648bd95e947d0a6be0beba4c594de127d94da7d9/README.md#corepack-enable--name

@arcanis
arcanis merged commit 2b7180e into main May 28, 2022
@arcanis
arcanis deleted the mael/package-manager-updates branch May 28, 2022 09:54
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.

6 participants