chore: migrate to pnpm #2560
chore: migrate to pnpm #2560
Conversation
🦋 Changeset detectedLatest commit: 4fd2bd0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 95 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| import TextInput from '@commercetools-uikit/text-input'; | ||
| // eslint-disable-next-line import/no-unresolved | ||
| import TextInput from '../../../packages/components/inputs/text-input'; |
There was a problem hiding this comment.
At the moment of writing this comment storybook builds are not consistent.
But in order to mitigate the effect of yarn berry "ghost-dependencies" yet not adding any circular dependencies to the monorepo, I have refactored most of the stories to load the packages based on the relative path.
This was a convention in some of the stories created earlier, e.g. here or here that I think we should go back to (instead of falling back on the hoisted packages).
Additionally, in this particular case the eslint-disable comment was added because this whole package is copied to presets and then the path is wrong. (As a side note, the story is loaded from here, not from presets)
| echo "Patching packages" | ||
| yarn patch-package |
There was a problem hiding this comment.
This is now done by pnpm itself during install (https://pnpm.io/cli/patch)
| "devDependencies": { | ||
| "react": "17.0.2" | ||
| "@types/react": "17.0.59", | ||
| "react": "17.0.2", | ||
| "react-select": "5.7.3" |
There was a problem hiding this comment.
react-select added to devDependencies only due to some missing types. Same thing was done in other packages
| "@typescript-eslint/eslint-plugin": "5.62.0", | ||
| "@typescript-eslint/parser": "5.62.0", | ||
| "core-js-compat": "^3.23.4", | ||
| "nwsapi": "2.2.5" |
There was a problem hiding this comment.
|
For the time being the migration is blocked by failing storybook builds. The issue (as I see it) is the overall amount of modules (installed with pnpm) to process with storybook’s internal instance of webpack (v4). I managed to get it started locally only a handful of times. The builds typically end up with Increasing I have tried:
In a fork of uikit with minimal amount of packages and storybook works fine there in combination with pnpm. Even in the uikit monorepo on |
…properly transipled
| @@ -0,0 +1 @@ | |||
| node-linker=hoisted | |||
There was a problem hiding this comment.
This makes storybook v5 work with pnpm (obviously at a cost of flat node_modules structure and hoisting of packages).
I figured it out the hard way and then I found this comment of pnpm's creator storybookjs/storybook#13428 (comment)
I assume this can only be considered as a workaround and perhaps migration to storybook v7 will unblock all benefits of pnpm for us.
| optimizeDeps: { | ||
| include: ['@emotion/react'], | ||
| }, | ||
| resolve: { | ||
| dedupe: ['@emotion/react'], | ||
| }, |
There was a problem hiding this comment.
For some reason adding this enables proper compilation with@emotion/babel-plugin when creating a production build of visual testing app with vite.
Based on emotion-js/emotion#2795 (comment)
| }, | ||
| "dependencies": { | ||
| "@babel/core": "7.22.8", | ||
| "@emotion/react": "^11.4.0", | ||
| "@emotion/styled": "^11.3.0", | ||
| "@commercetools-frontend/babel-preset-mc-app": "21.25.2", |
There was a problem hiding this comment.
This had to be changed from a devDependency to a regular dependency. Otherwise babel compilation with @emotion/babel-plugin didn't work properly.
|
Hey @commercetools/shield-team-fe, |
Weldon @kark , really great work 💯 After switching to node v16, I cannot seem to start locally after pnpm install which seemed to run fine, although I had a bunch of warnings. It does not start because of the following error: However, navigating to the directory(docs) manually, I can start the application with |
|
Thank you @ddouglasz 🙏
This is expected, current version of storybook doesn't work in node >17.
Oops, many thanks for noticing. |
|
I also tested this locally and both build and test scripts works fine. Regarding Storybook, it took a while to start but it did at the end (aprox. 3 minutes). This branch files loaded: Main branch: |
Thank you Carlos 🙇♂️
That's sadly true 🙁 Things even up in production builds,
but in webpack's development mode the difference in the main bundle size is huge. |
|
Hey @commercetools/shield-team-fe,
|
|
During today's meeting we discussed the current state of migration. The key takeaway was that we decided to work on a spike where we'll attempt to migrate Storybook to version 7. The migration is intended to allow us to use Vite as the bundler and determine if this will enable migration to pnpm without any workarounds. |
|
In the following branch
This allows to make a conclusion that the setup with storybook v7 and pnpm works fine (in the scope of the spike).
|
Wow! That's great news! 🎉 One thing we might do to have some comparison data is to configure current Storybook (v5) to use only the same stories as the ones migrated in Storybook v7 and then run a build to compare the timings. |
Thanks Carlos, I did what you suggested. Anyways, for the 5 stories (avatar, primary button, select input, rich text input, text input) here are the results:
|
Summary
Description