Fix other workflows to install Corepack as prereq #246
Conversation
A recent commit upgraded the version of Yarn to v4 and removed the Yarn binary from the repo, thereby requiring that Corepack be installed in order to install dependencies. The `build-lint-test` was updated to install Corepack, but not the documentation-related workflows, so they are failing. This commit fixes those workflows to install Corepack. It also cleans up some work that was done in previous commits: - In the `build-lint-test` workflow we ensure that `prepare` is run once per Node version we are testing and that `build` and `lint` use the latest Node version we are testing. - In steps where we are installing Node just to gain access to the `corepack` executable, we use `.nvmrc` (the version of Node we use for development) to know which version of Node to install rather than using the latest LTS (`lts/*`). For jobs that do not need to concern themselves with being run in multiple Node versions, this ensures that consistent Node versions are used in this step vs. the step that is used to simply restore the Yarn cache. - The checkout step always goes first, this way `.nvmrc` can be read.
| strategy: | ||
| matrix: | ||
| node-version: [18.x, 20.x] | ||
| steps: | ||
| - name: Use Node.js | ||
| - uses: actions/checkout@v4 | ||
| - name: Install Corepack via Node | ||
| uses: actions/setup-node@v4 |
There was a problem hiding this comment.
The checkout step always goes first, this way
.nvmrccan be read.
There was a valid reason to do actions/setup-node before checkout because of the action not being corepack-compatible. IIRC it throws an error when the preinstalled yarn version does not match the packageManager one in package.json (which is then working after a corepack enable).
Could potentially cause issues with any dependencies that rely on the specific package manager version in their installation step, if any (since setup-node performs dependency installation in presence of package.json)?
There was a problem hiding this comment.
I didn't find evidence in the source that it installs dependencies — I think you still have to do that yourself.
It does run yarn to figure out how to cache Yarn files, and Yarn v1 is preinstalled on the GitHub runners, and Yarn v1 throws if packageManager refers to a non-1.x release. So you're right about that. But... setup-node should only try to run yarn if you pass the cache option to setup-node. I left that option off the first setup-node call (as you'd done) and I re-ran setup-node with caching after installing Corepack. I don't see how that could cause issues, but do you still see a case in which this could fail?
As for your final point:
Could potentially cause issues with any dependencies that rely on the specific package manager version in their installation step
Does Yarn consult packageManager for dependencies? I wasn't aware that it did that. I thought it only consulted it for the root package.
There was a problem hiding this comment.
Does Yarn consult packageManager for dependencies? I wasn't aware that it did that. I thought it only consulted it for the root package.
It does a few somewhat unexpected things, yes.. Among them: yarnpkg/berry#6258
| @@ -7,41 +7,47 @@ jobs: | |||
| prepare: | |||
| name: Prepare | |||
| runs-on: ubuntu-latest | |||
| strategy: | |||
| matrix: | |||
| node-version: [18.x, 20.x] | |||
There was a problem hiding this comment.
| node-version: [18.x, 20.x] | |
| node-version: [18.x, 20.x, 22.x] |
| runs-on: ubuntu-latest | ||
| needs: | ||
| - prepare | ||
| strategy: | ||
| matrix: | ||
| node-version: [18.x, 20.x] |
There was a problem hiding this comment.
| node-version: [18.x, 20.x] | |
| node-version: [18.x, 20.x, 22.x] |
| runs-on: ubuntu-latest | ||
| needs: | ||
| - prepare | ||
| strategy: | ||
| matrix: | ||
| node-version: [18.x, 20.x] |
There was a problem hiding this comment.
| node-version: [18.x, 20.x] | |
| node-version: [18.x, 20.x, 22.x] |
A recent commit upgraded the version of Yarn to v4 and removed the Yarn binary from the repo, thereby requiring that Corepack be installed in order to install dependencies. The
build-lint-testwas updated to install Corepack, but not the documentation-related workflows, so they are failing.This commit fixes those workflows to install Corepack. It also cleans up some work that was done in previous commits:
build-lint-testworkflow we ensure thatprepareis run once per Node version we are testing and thatbuildandlintuse the latest Node version we are testing.corepackexecutable, we use.nvmrc(the version of Node we use for development) to know which version of Node to install rather than using the latest LTS (lts/*). For jobs that do not need to concern themselves with being run in multiple Node versions, this ensures that consistent Node versions are used in this step vs. the step that is used to simply restore the Yarn cache..nvmrccan be read.