search

stackblitz / webcontainer-core

Dev environments. In your web app.
https://webcontainers.io
MIT License
3.94k stars 173 forks source link

Support `packageManager` field in package.json and provide the requested version automatically #871

Open dominikg opened 2 years ago

dominikg commented 2 years ago

Is your feature request related to a problem? Please describe: After updating pnpm and the engine restriction in a repository, it may no longer be compatible with webcontainers version of pnpm see https://github.com/stackblitz/webcontainer-core/issues/861

Describe the solution you'd like: Support the packageManager field in package.json similarly to corepack.

Describe alternatives you've considered: Use a more relaxed engine constraint and live with the fact that webcontainers use a different version of pnpm

Additional context: https://nodejs.org/api/packages.html#packagemanager https://nodejs.org/api/corepack.html

Running corepack inside webcontainer might not be feasible, but you can do it without by simply parsing packageManager and fetching the version from npm registry as well: https://github.com/sveltejs/vite-plugin-svelte/blob/74fd16db25d0be6c624e3d7e77b170ff7afb34ed/.github/workflows/ci.yml#L46-L48

request extracted from this comment: https://github.com/stackblitz/webcontainer-core/issues/861#issuecomment-1285158343

d3lm commented 1 year ago

Thanks for the report!

Yea this is currently a limitations as we only support one version of pnpm right now. I'd be ideal if we supported the packageManager field indeed. I ll take this with me and discuss this with the team.

d3lm commented 1 year ago

My suggestion for the time being would be to not use strict engine checks.

dominikg commented 1 year ago

vite-plugin-svelte stopped bumping the engine constraint with renovate, so for that repo it isn't time sensitive right now. However with the upcomping pnpm 8 and it's breaking changes, it would be great if webcontainers detected and supported it to avoid inconsistencies between locally used pnpm@8 and pnpm@7 inside webcontainers.

at the very least you would have to bump pnpm@7 to 7.24 or later so that it can understand the new lockfile format v6, otherwise projects that switched to the new format might be incompatible.

dominikg commented 1 year ago

fyi, pnpm@8 has been released today https://github.com/pnpm/pnpm/releases/tag/v8.0.0

projects using pnpm8 are going to use lockfile format v6 by default which is not supported by the current pnpm version deployed on stackblitz webcontainers 

~/projects/vitejs-vite-njsrpa 1m 9s
❯ pnpm -v
7.13.6
d3lm commented 1 year ago

Thanks for putting this on our radar. I have seen the changes in pnpm and we definitely need to update to the latest version so both versions of the lock file are supported and that newer projects don't break.

dominikg commented 1 year ago

If you can't support 8 right away, going up to 7.24 might help to support the new lockfile format at least.

d3lm commented 1 year ago

Yep, I think we may be able to support 8. The problem will still be the exact version from the packageManager field when using strict engine checks.

Shinigami92 commented 1 year ago

@d3lm what is the progress of pnpm v7.24 / v8 bump? Vite is now waiting on this to bump its pnpm to v8. So as soon as Stackblitz's Webcontainer supports it, we will merge the PR in Vite.

d3lm commented 1 year ago

@Shinigami92 Thanks for the info. The plan is to update soon-ish and I will let you know once it's on prod.

d3lm commented 1 year ago

Hey all, just wanted to give an update and say that pnpm 8.2.0 is now rolled out to production. We don't support the packageManager yet but at least pnpm is upgraded and it should now support both lockfile versions.