chore: Upgrade from yarn classic (v1.22) to pnpm v10#66
Conversation
f45dc26 to
c17a0c5
Compare
This reverts commit f61b164.
…g test-app.css file
2a93c09 to
e149125
Compare
| "ember-cli-sri": "^2.1.1", | ||
| "ember-cli-terser": "^4.0.2", | ||
| "ember-json-viewer": "*", | ||
| "ember-json-viewer": "workspace:*", |
There was a problem hiding this comment.
This is the pnpm way to refer to a workspace package, see https://pnpm.io/workspaces#workspace-protocol-workspace
| @@ -0,0 +1,115 @@ | |||
| /** | |||
There was a problem hiding this comment.
Added to upgrade eslint 8 to 9 for the addon/ dir, copied with minor changes from test-app's eslint config
| @@ -1,9 +1,9 @@ | |||
| import babel from '@rollup/plugin-babel'; | |||
There was a problem hiding this comment.
Renamed to .mjs to remove a deprecation warning from rollup.
The other changes are cosmetic, applied by prettier to fix linting.
| "docs-app" | ||
| ], | ||
| "scripts": { | ||
| "preinstall": "npx only-allow pnpm" |
There was a problem hiding this comment.
pnpm-maintained package, suggested by https://dev.to/andreychernykh/yarn-npm-to-pnpm-migration-guide-2n04
| "start": "ember serve", | ||
| "test": "concurrently \"npm:lint\" \"npm:test:*\" --names \"lint,test:\" --prefixColors auto", | ||
| "test:ember": "ember test", | ||
| "test:ember-compatibility": "ember try:each" |
There was a problem hiding this comment.
Dropped this because we don't want it to be included when doing pnpm test.
We run the scenarios individually in the workflow
| "@embroider/compat": "^3.4.8", | ||
| "@embroider/core": "^3.4.8", |
There was a problem hiding this comment.
pnpm identified that these had been missing before (yarn classic had been hoisting them from elsewhere which made them available to the test-app. pnpm doesn't allow that, so they have to be explicitly added)
| - name: Install Dependencies | ||
| if: steps.node-cache.outputs.cache-hit != 'true' | ||
| run: yarn --frozen-lockfile | ||
| run: pnpm install |
eliasdawson-addepar
left a comment
There was a problem hiding this comment.
Is there a reason you prefer to use volta to manage pnpm version? @yanglinz's recommendation was to use corepack for pnpm management and only use Volta for node version.
I misunderstood that. I'll retool this once again to use volta only for node and use corepack for pnpm. |
8c0bfe9 to
d542b77
Compare
|
I think this is on the right track now. Deferring to @yanglinz for alignment on the right timing to introduce corepack into an Addepar repo. I would doubt most people have it configured today, but with your approval that doesn't need to prevent us from starting to use it inside Web Core (likely the only people who will touch this code in the next few months). |
| @@ -20,40 +20,45 @@ jobs: | |||
| steps: | |||
| - uses: actions/checkout@v4 | |||
| - uses: volta-cli/action@v4 | |||
There was a problem hiding this comment.
This sets up node, using the node version from package.json#volta.node
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: volta-cli/action@v4 | ||
| - uses: pnpm/action-setup@v4 |
There was a problem hiding this comment.
This sets up pnpm, using the pnpm version from package.json#packageManager

Upgrades from yarn classic as a package-manager to pnpm.
Followed guidance from https://dev.to/andreychernykh/yarn-npm-to-pnpm-migration-guide-2n04 and https://pnpm.io/installation.
In some cases I referred to existing v2 addons w/ pnpm (like ember-page-title) as a reference.
Also looked at https://github.com/embroider-build/addon-blueprint.
pnpm surfaced some mistakes where dependencies hadn't been correctly defined (but were hoisted when using yarn classic). It also complains more loudly about deprecated deps which led me to upgrade eslint 8 to 9 for
addon/.This PR diverges a little from the github workflow guidance from pnpm (https://pnpm.io/continuous-integration#github-actions) in that it uses volta's cli action instead of setup-node.
This change sets up this repo so that:
This uses actions/cache directly, and adds an extra step to each job to find the pnpm "store" path to use for the cache. I determined how to do this by looking at how setup-node does it when
cache: 'pnpm'is used (https://github.com/actions/setup-node/blob/48b90677b6048efbc723b11a94acb950d3f1ac36/src/cache-utils.ts#L30-L38), and found some examples in the wild like: https://gist.github.com/belgattitude/838b2eba30c324f1f0033a797bab2e31.Other changes: