chore: upgrade pnpm to version 9

[paoloricciuti]

Svelte 5 rewrite

Feel free to close immediately if it's not desired but the upgrade should be seamless. I don't know if we should also specify the version in CI but let's see.

Also does this require a coordinated effort with sveltekit?

Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (main).

If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is svelte-4 and not main.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3be636a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[PuruVJ]

Ur a savior!

[Conduitry]

As with sveltejs/kit#12129. I expect that this is blocked until Vercel supports pnpm v9.

[paoloricciuti]

As with sveltejs/kit#12129. I expect that this is blocked until Vercel supports pnpm v9.

Can you approve the deployment to check if it's working? @PuruVJ said that he's currently deploying with pnpm 9

[Rich-Harris]

I'll check in with the team, see if we can get an ETA on this

[PuruVJ]

Error is happening cuz of strictness of packageManager field, I am using pnpm 9 locally with lockfile v9, vercel has pnpm 8 but that supports lockfile 9 as well, so it works

[Rich-Harris]

How do we fix it?

[PuruVJ]

Either wait for vercel to release v9, or remove packageManager field for now. Generate lockfile v9 from pnpm 9 locally, push it, and vercel's pnpm v8 will then use that v9 lockfile(it support that)

[Rich-Harris]

Apparently Vercel already shipped pnpm 9, so maybe the problem is on our end?

[paoloricciuti]

Apparently Vercel already shipped pnpm 9, so maybe the problem is on our end?

I think the problem is the stricness of the packageManager field? unfortunately you can't set it to something like pnpm@^9

[Rich-Harris]

It seems there's a bug with the version detection — not really following the conversation internally but it could be what's causing these headaches

[paoloricciuti]

It seems there's a bug with the version detection — not really following the conversation internally but it could be what's causing these headaches

Keep us posted 🤞🏼

[Rich-Harris]

marking as draft for now to reduce noise

[paoloricciuti]

marking as draft for now to reduce noise

Sure no prob 🤟🏻

[Rich-Harris]

Looks like 9.0.4 (which I think is the latest available version) is good

[PuruVJ]

I believe 9.1 is the latest 🤔

…

On Fri, 17 May 2024 at 7:01 AM, Rich Harris ***@***.***> wrote: Looks like 9.0.4 (which I think is the latest available version) is good — Reply to this email directly, view it on GitHub <#11637 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALMH4F6KAHVWV7462FA6DZTZCVMWFAVCNFSM6AAAAABHYD4HS2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJWGQ3DOOBWGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[paoloricciuti]

Looks like 9.0.4 (which I think is the latest available version) is good

9.1 is the latest but i think even if the packageManager is super strict once you have it locally you can use pnpm 9 even with higher version (at least it was like this with pnpm 8)

[Rich-Harris]

Sorry, I meant the latest available version in the build environment. Though apparently this is due to some interaction with corepack — investigation ongoing

[benmccann]

We'd also need to add an .npmrc file with package-manager-strict=false or else the project can't be run locally. Unless that default in pnpm 9 is reverted (pnpm/pnpm#8087)

[Rich-Harris]

Update from @EndangeredMassa (thank you!) here: vercel/vercel#11607 (comment)

The relevant bit for us:

you can work around this problem by doing any of the following:

• enable corepack (vercel docs)
  • set env var ENABLE_EXPERIMENTAL_COREPACK to 1
  • make sure you have a matching package.json#packageManager value
• remove your package.json#engines.pnpm value (pnpm docs)

[benmccann]

Awesome! Good to see the CI passing now

My comment above about needing the .npmrc will still need to be addressed or else people can't run this project locally without corepack anymore

[paoloricciuti]

Awesome! Good to see the CI passing now

My comment above about needing the .npmrc will still need to be addressed or else people can't run this project locally without corepack anymore

If we remove engines we would get both the deployment on vercel and the ability to run locally right?

[benmccann]

If we remove engines we would get both the deployment on vercel and the ability to run locally right?

No. That's how prior versions of pnpm worked, but pnpm 9 changed the behavior. I personally consider it to be a regression, but jury is still out on that: pnpm/pnpm#8087

[benmccann]

It turns out that the packageManager field is no longer required by Vercel. I removed it so that local builds work. Can you fix the merge conflicts in the lock file? Then I think we can merge this

[Conduitry]

Without the packageManager field, though, aren't people with corepack not going to get the automatic version switching anymore? I don't think corepack works off engines field ranges. People would have to manually switch between versions of pnpm when different projects use different ones.

[benmccann]

Yes, that's true, but we didn't put that there for contributors. It was there only for purposes of deploying to Vercel. It'll also stop the endless renovate PRs to bump that field whenever there's a new version of pnpm, which is another win from removing it

[paoloricciuti]

It turns out that the packageManager field is no longer required by Vercel. I removed it so that local builds work. Can you fix the merge conflicts in the lock file? Then I think we can merge this

Fixing now...in favor of removing it

[benmccann]

Hmm. Having to put with: version: 9 4 different places is kind of annoying though. Maybe we should wait a couple more days and see how the poll shakes out. Removing the package-manager-strict enforcement is only behind by half a dozen votes. I wonder if just setting that to false might be the better backup plan vs removing the packageManager field

[paoloricciuti]

Hmm. Having to put with: version: 9 4 different places is kind of annoying though. Maybe we should wait a couple more days and see how the poll shakes out. Removing the package-manager-strict enforcement is only behind by half a dozen votes. I wonder if just setting that to false might be the better backup plan vs removing the packageManager field

It is quite annoying but also it only changes when a major of pnpm is released so it's not that often. Tbf the strictness of the packageManager field is kinda annoying on local too if you have a different version.

But up to you for the final decision 😄

[benmccann]

I'll merge this for now. If pnpm/action-setup#126 gets merged then we can remove all the version: 9 stuff in a follow up