-
Notifications
You must be signed in to change notification settings - Fork 25
Replace Yarn with pnpm #5545
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
base: master
Are you sure you want to change the base?
Replace Yarn with pnpm #5545
Conversation
|
✅ Deploy Preview for hpe-theme-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for unrivaled-bublanina-3a9bae ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for hpe-design-icons-grommet failed. Why did it fail? →
|
| })(); | ||
|
|
||
| global.sessionStorage = mockSessionStorage; | ||
| globalThis.sessionStorage = mockSessionStorage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| color: global.hpe.focusIndicator.boxShadow.color, // focusBoxShadowParts[global.hpe.focusIndicator.boxShadowParts.length - 1], | ||
| size: global.hpe.focusIndicator.boxShadow.spread, // focusBoxShadowParts[focusBoxShadowParts.length - 2], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Context for other reviewers grommet/grommet-theme-hpe#549
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reference. This was failing and best I could tell it need to be removed
Sulaymon333
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MikeKingdom Thank you for putting this PR up. I left some few comments.
aries-core/package.json
Outdated
| "@types/react": "^19.1.10", | ||
| "@types/react-dom": "^19.1.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "@types/react": "catalog:", | |
| "@types/react-dom": "catalog:", |
Should we consider using these packages from the workspace catalogue as well since they are pretty much the same versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. There's more tuning of dependencies to do over time also.
| ### Applying the HPE theme | ||
|
|
||
| Once your project is set up with ReactJS and Grommet, make sure you have the latest [grommet-theme-hpe](https://github.com/grommet/grommet-theme-hpe) theme via `npm` or `yarn`. This will help ensure that your application is aligned with the HPE Design System colors, fonts, and default component styles. | ||
| Once your project is set up with ReactJS and Grommet, make sure you have the latest [grommet-theme-hpe](https://github.com/grommet/grommet-theme-hpe) theme via `npm`, `pnpm` or `yarn`. This will help ensure that your application is aligned with the HPE Design System colors, fonts, and default component styles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The instruction here seems unclear to me. Did we intend to say that
"make sure you have the latest HPE theme from grommet-theme-hpe theme via any of the following commands
pnpm install grommet-theme-hpe // or
bun add grommet-theme-hpe // or
yarn add grommet-theme-hpe // or
npm install grommet-theme-hpe "
Instead of
make sure you have the latest grommet-theme-hpe theme via
npm,pnpmoryarn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can tune the guidance later. I'll consider that a little out of scope with this PR since it's mainly focused on our internal build

These changes replace yarn as the package manager with pnpm.
With these changes, many common dependencies can be managed in a "catalog" (defined in pnpm-workspace.yaml).
By default, pnpm is more strict and doesn't allow packages to be "hoisted" like yarn does. It's possible to configure pnpm to allow hoisting, but it's generally better to avoid it for cleaner dependencies.
One situation where hoisting is currently happening is how
aries-corehas a storybook whose stories refer to components inaries-site. This basically causes a circular dependency betweenaries-coreandaries-site. Lint also currently fails on these stories with pnpm. The plan to fix this is move the examples fromaries-sitetoaries-coreso they can be exported within the workspace.aries-sitewill reference them from there. The storybook for them can then be moved out ofaries-coreto a standalone storybook inapps(where it can also reference those examples now fromaries-core. This will likely be done in a follow on PR.Deploy Preview
What does this PR do?
What are the relevant issues?
Where should the reviewer start?
How should this be manually tested?
Any background context you want to provide?
Screenshots (if appropriate)
Should this PR be mentioned in Design System updates?
Is this change backwards compatible or is it a breaking change?