build(pnpm): migrate to pnpm#1933
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the project from npm to pnpm as the primary package manager. The changes include updating the release configuration, documentation, and all package scripts to use pnpm commands. Additionally, a new pnpm-workspace.yaml file has been introduced, and the package.json has been updated with pnpm-specific engine requirements and dependency overrides. One review comment suggests using the $typescript variable in the pnpm overrides to ensure the TypeScript version remains synchronized with devDependencies.
spike-rabbit
left a comment
There was a problem hiding this comment.
I think we also must use the pnpm semantic-elease plugin. If not npm will install a bunch of things when releasing.
With that:
- you can use alias in the peer dependencies to workspace deps
- I hope it works with the dist packages as well somehow... may needs scripting
81ca237 to
44ade0a
Compare
|
I remember we discussed about using pnpm in past and one of the reason for not switching was: What exactly got changed now? |
@chintankavathia yes most of our consumer still use
So let's discuss it in the core-meeting what is the right approach. |
| "prismjs": "1.30.0", | ||
| "qrcode-generator": "2.0.4", | ||
| "react": "19.2.5", | ||
| "react-dom": "19.2.5", |
There was a problem hiding this comment.
please confirm here: is this really needed to move live-preview dependencies here?
There was a problem hiding this comment.
We have two options:
- we add them to the root package so they are automatically node_modules top level dependencies
- we add add them to the specific dependencies to hoist patterns so the are add to node_modules top level dependencies
spike-rabbit
left a comment
There was a problem hiding this comment.
To provide some more details regarding releasing:
In pnpm workspaces one can declare (peer)dependencies using the workspace: syntax.
With that I hope we can eliminate the use of custom updating the peer dependencies.
The only problem is, that I don't know how this behaves with releasing from a dist directory, so when the actual workspace package is not the target.
44ade0a to
2dc8e2a
Compare
|
@spliffone @spike-rabbit 3 hours ago, new pnpm major release v11: https://github.com/pnpm/pnpm/releases/tag/v11.0.0 Thanks to @pwuersch |
There was a problem hiding this comment.
Hey guys, chiming in here because @kfenner was asking for some feedback from users that have used pnpm before.
pnpm has sped up our local installs quite a bit since we migrated away from Yarn already three years ago. Probably NPM has become more competitive over the years. The headlining reasons to use pnpm are quite convincing:
- Shared package store, which is faster and more efficient,
- Allow-listing of dependencies that are allowed to execute package lifecycle scripts -> mitigates almost every recent supply-chain attack we've seen, as the compromised package must be in the allowed dependencies to cause damage.
While this is already great, the real benefit in working with projects that use pnpm is the developer experience:
- Unmet
peerDependenciesare printed in an actually useful way, - It still adheres to most of the vanilla npm configuration (e.g., overriding registry URLs and other settings via
.npmrc), - Very nice integration in
mise-en-placeorcorepack, whichever you prefer, - Amazing lockfiles with diffs you can actually read.
All in all, I enjoy working with pnpm and it's become my default choice for a package manager when using Node.js.
| - run: npm ci --prefer-offline --no-audit --include=optional | ||
| - run: npx playwright test --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} | ||
| - run: pnpm install --frozen-lockfile | ||
| - run: pnpm exec playwright test --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} |
There was a problem hiding this comment.
Nit: the exec here is technically not required (but can add some clarity I guess): https://pnpm.io/cli/exec#examples
41131b6 to
e98d9fb
Compare
2cd4908 to
3c125f9
Compare
Sadly pnpm 11 handles optionalDependencies differently and I wasn't able to link the brand dependency in the root node_modules folder which means the prepare brand script is always failing and the pipeline never succeed. So I guess we should go with version 10 at the moment. |
3c125f9 to
040c1e6
Compare
Hmm, is this a good sign? |
@spliffone locally, with a normal Also the |
Yes, I started the migration again and dropped a few settings. Now it seems to work |
c9e05c9 to
54fb13e
Compare
- use pnpm/action-setup from v6 - add missing schematics devDependencies for CI build - use host-level auth token for all code.siemens.com registries - use semantic-release-pnpm plugin
b1c2300 to
c1c4479
Compare
1f41493 to
21a9b91
Compare
|
See #1900 Summary: (+) way more powefull than npm (-) more configuraiton Vote: 👍 We want PNPM as the new package manager @siemens/siemens-element-members Please share your voice on the package manager topic! |
@spliffone I would say the voting is complete and the majority is positive about switching to pnpm. Should we do this migration for Element v50 so we stick to npm here on the main branch until we start merging stuff for v50 and v49.x also stays on npm, OK? |
See #1900
node_modulesstructurenode_modules. Transitive deps are stored under.pnpm/and symlinked.node_modules.node_modulespnpm env)node_modulesare in sync with lock file)ng serve(vite) require a flatnode_modulesto run the dev server which require adjustments in the PNPM configuration to hoist dependenciestarballUrlin the lock file are incorrectly for code.siemens.comChallenges:
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: