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

Upgrade pnpm to v8 #1051

Closed
wenche opened this issue May 2, 2022 · 19 comments
Closed

Upgrade pnpm to v8 #1051

wenche opened this issue May 2, 2022 · 19 comments
Assignees
Labels
dependencies Pull requests that update a dependency file 🚀 ready to deploy Use this if issue is ready to be deployed 🛠 technical Technical stuffs like reducing debt, refactor or improve code base

Comments

@wenche
Copy link
Contributor

wenche commented May 2, 2022

pnpm v7 is released, and we should look into what it takes to upgrade

@wenche wenche added the 🛠 technical Technical stuffs like reducing debt, refactor or improve code base label May 2, 2022
@wenche wenche mentioned this issue May 2, 2022
10 tasks
@fernandolucchesi
Copy link
Contributor

dependent on #1040

@SvSven SvSven self-assigned this May 23, 2022
SvSven added a commit that referenced this issue May 31, 2022
pnpm v7 has this set to true by default now
SvSven added a commit that referenced this issue May 31, 2022
The strict-peer-dependencies flag is due to lack of npmrc file
SvSven added a commit that referenced this issue May 31, 2022
We should be getting these from the
testing library package, but for some
reason we are not with pnpm v7
wenche added a commit that referenced this issue May 31, 2022
wenche added a commit that referenced this issue May 31, 2022
Since it is not actually used and it caused lot of issues with peer dependencies with pnpm v7
@SvSven
Copy link
Contributor

SvSven commented May 31, 2022

We now have support for pnpm v7 in the code and GitHub Actions - however we need to resolve how we handle the peer dependencies. What is different in v7 is that the option strict-peer-dependencies is now set to true by default; we can override this by manually setting it to false in .npmrc or by passing it as a flag in the terminal command (which we need to do for GH Actions since we lack a config file there). This works, but is not ideal - better would be to just go over the dependencies and make sure we include what we need (and remove what we don't)

@wenche
Copy link
Contributor Author

wenche commented Jun 1, 2022

@storybook/addon-docs uses v1.x.x of @mdx-js/react and that version does not support React 18 (version 2 does) See also storybookjs/storybook#18383 for some related discussions. I don't think this is of great importance for us since we are not using mdx

@wenche
Copy link
Contributor Author

wenche commented Jun 1, 2022

@storybook/addon-essentials -> @storybook/addon-actions -> react-inspector does not yet support React 18

@wenche
Copy link
Contributor Author

wenche commented Jun 1, 2022

 @storybook/react
│ └─┬ react-element-to-jsx-string
│   ├── ✕ unmet peer react@"^0.14.8 || ^15.0.1 || ^16.0.0 || ^17.0.1": found 18.1.0
│   └── ✕ unmet peer react-dom@"^0.14.8 || ^15.0.1 || ^16.0.0 || ^17.0.1": found 18.1.0

storybookjs/storybook#18349

@wenche
Copy link
Contributor Author

wenche commented Jun 1, 2022

┬ @equinor/eds-core-react
│ └─┬ @equinor/eds-utils
│   └─┬ react-popper
│     └── ✕ unmet peer react@"^16.8.0 || ^17": found 18.1.0

react-popper v2.30 has fixed this, but the EDS team are holding back the upgrade for React 18 a bit. We don't use any components from EDS that relies on react-popper, so this is not a big deal for us

@wenche
Copy link
Contributor Author

wenche commented Jun 16, 2022

Forgot to add the one for Reach

No progress on any of the issues yet.

@nilsml
Copy link
Contributor

nilsml commented Jun 17, 2022

Why isn't this closed? pnpm is using latest version already

@wenche
Copy link
Contributor Author

wenche commented Jun 17, 2022

Because we are waiting on updates for all the library mentioned in the comments to get rid of peerDeps warnings

@SvSven
Copy link
Contributor

SvSven commented Sep 6, 2022

No official React 18 support for reach-ui yet

@SvSven SvSven added 🕑 waiting for someone else dependencies Pull requests that update a dependency file labels Sep 6, 2022
@millianapia
Copy link
Contributor

as we are phasing out reach-ui #1378, we can start looking at this again

@fernandolucchesi
Copy link
Contributor

Ready to be worked on after #1532

@nilsml
Copy link
Contributor

nilsml commented May 2, 2023

pnpm 8 is the latest, so we should upgrade to that version

@millianapia millianapia assigned millianapia and unassigned wenche and SvSven May 10, 2023
@fernandolucchesi fernandolucchesi changed the title Upgrade pnpm to v7 Upgrade pnpm to v8 May 15, 2023
millianapia added a commit that referenced this issue May 16, 2023
@SvSven
Copy link
Contributor

SvSven commented Jun 2, 2023

According to the migration guide for v8, we need to regenerate the pnpm lockfile as they have upgraded to lockfile version 6, that's missing in the PR still. Should be as simple as making sure your local pnpm version is 8.5.1 similar to the ones pinned in the PR, and then running pnpm setup-project - it'll autoregenerate the lockfiles then - it did for me at least.

Haven't fully tested the web and studio with the new lockfile/pnpm v8 yet though

Also looks like there isn't a Dockerfile in the v3 studio yet, but we'll need that to deploy the satellite studios as well. We can take that in another issue

@SvSven
Copy link
Contributor

SvSven commented Jun 6, 2023

Have pushed updated lock files, we should make sure everything still works as expected 😬

@millianapia
Copy link
Contributor

i have checked that the web and studio still works as expected, all of the regular pnpm commands still works. Other things to have in mind before i merge @fernandolucchesi @padms @SvSven ?

@fernandolucchesi
Copy link
Contributor

i have checked that the web and studio still works as expected, all of the regular pnpm commands still works. Other things to have in mind before i merge @fernandolucchesi @padms @SvSven ?

we should test if docker images are also working as expected

@fernandolucchesi
Copy link
Contributor

and dockerfile of sanityv3 should also be updated

@fernandolucchesi
Copy link
Contributor

reviewed sanity v3 docker and works fine! ✅

millianapia added a commit that referenced this issue Jun 13, 2023
@millianapia millianapia added the 🚀 ready to deploy Use this if issue is ready to be deployed label Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file 🚀 ready to deploy Use this if issue is ready to be deployed 🛠 technical Technical stuffs like reducing debt, refactor or improve code base
Projects
None yet
Development

No branches or pull requests

5 participants