Skip to content

Commit

Permalink
chore: Drop yarn2nix and switch to pnpm.
Browse files Browse the repository at this point in the history
This is a wide-ranging change in which we drop `yarn2nix` (and Yarn
itself) in favor using `pnpm` directly, with no Nix layer in-between
the `pnpm` lockfile and the packages we use for builds. This means
we're giving up on most of what Nix provides -- here we only use it to
ensure that everyone is using the same version of `pnpm`, sourced from
`nipxkgs` via our `nix develop` shell.

We also now run most CI on GitHub Actions, rather than Buildkite. See
the notes below for a detailed justification.

I'm doing this reluctantly, but `yarn2nix` is falling further and
further behind the Nodejs ecosystem, it always feels like we're just
one major package version bump away from everything breaking, and
there's no viable replacement on the horizon for `yarn2nix` that I can
see. Nix is not really buying us much in this repo to begin with, so
we don't lose that much with this change. Hopefully this leads to
better DX going forward overall.

Note that the `README.md` is heavily edited to replace all the `yarn`
commands with the equivalent `pnpm` commands (where they exist; we
have lost some functionality by dropping Yarn, see below), and I
wouldn't be surprised if there are some discrepancies in the
documentation vs the reality. Please file an issue if you notice any.

A few more items of note:

* I have bumped our `nixpkgs` pin here, but only to get a more recent
version of `pnpm`. I have not bumped any Nodejs package versions.

* `pnpm` is more strict about dependencies, so I've had to add a few
that Yarn was magically finding for us.

* We've lost the nice `wsrun watch` and `wsrun watch:storybook`
commands that would run the entire project in `watch` (HMR) mode with
a single command. This is because `wsrun` is designed to work with
Yarn workspaces only, and doesn't work with `pnpm` workspaces. As far
as I can tell, there's no equivalent to `wsrun` for `pnpm`, which
means that if you want the run the frontend or our Storybook in HMR
mode and automatically pick up changes made to the other packages in
the repo upon which the frontend/Storybook depend, you'll need to open
multiple shells and run a `pnpm watch` command in each one,
simultaneously.

* With this change, we build the frontend and test it (or at least,
will test it, once we have some tests to run!) on GitHub Actions, not
on Buildkite. The reasons for this are that while the Nix Buildkite
plugin is wonderful, and works much better than any Nix-related GitHub
Action that I've seen, there are no comparable Buildkite plugins for
installing a particular version of `pnpm`, caching `pnpm` builds
across multiple CI runs, or deploying to Chromatic. Therefore, if we
stuck with Buildkite CI for this repo, we would need to implement all
that functionality ourselves, probably via bespoke scripts. That's a
waste of time when there are existing vendor-provided GitHub Actions
which do all of those things for us. We still do have a few minor
checks & some Nix shell caching CI jobs on Buildkite, but most likely
those will eventually go away and be replaced by equivalent GitHub
Actions, so that we only need to look in one place for CI results.

* On that note, we do not implement the push-to-Chromatic CI step
here, as I want to test just building the app before we get into
trying to build Storybook in CI. (However, I have been able to
successfully build and use Storybook in my local shell with these
changes.) Support for this is coming in a subsequent commit.

* Speaking of Storybook, we have to do some ugly hacks to make it work
with `pnpm`. Many people have trouble with this and there are a litany
of issues, many with different workarounds that are sometimes
successful, sometimes not; see:

pnpm/pnpm#4268
storybookjs/storybook#13428
https://github.com/storybookjs/builder-vite/blob/782261437b3cb3e2f3d013cd37f1faa14863f205/packages/builder-vite/README.md#installation
storybookjs/builder-vite#55
storybookjs/builder-vite#237
https://github.com/IanVS/vite-storybook-pnpm-shame
https://github.com/jdk2pq/vite-storybook-pnpm-vue

Until Storybook fix their various issues, this looks like the proper
"fix", but it will require quite a bit of work, I suspect:

pnpm/pnpm#4268 (comment)

However, in the interest of time, I've chosen this option, which is
the least bad workaround I've seen that worked for me:

storybookjs/storybook#13428 (comment)
  • Loading branch information
dhess committed May 26, 2022
1 parent e6fccfd commit 3588372
Show file tree
Hide file tree
Showing 20 changed files with 16,070 additions and 15,751 deletions.
7 changes: 0 additions & 7 deletions .buildkite/pipeline.chromatic.yaml

This file was deleted.

13 changes: 0 additions & 13 deletions .buildkite/pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,3 @@ steps:
nix develop --print-build-logs --profile /tmp/primer-app --command cachix --verbose --config "$CACHIX_CONFIG" push "$CACHIX_CACHE" /tmp/primer-app
agents:
os: "darwin"
- label: ":chromatic: Trigger Chromatic deployment"
trigger: primer-app-chromatic
# This can't be async, because we need to be sure the Chromatic
# deployment succeeded before merging via Bors.
async: false
build:
message: "${BUILDKITE_MESSAGE}"
commit: "${BUILDKITE_COMMIT}"
branch: "${BUILDKITE_BRANCH}"
env:
BUILDKITE_PULL_REQUEST: "${BUILDKITE_PULL_REQUEST}"
BUILDKITE_PULL_REQUEST_BASE_BRANCH: "${BUILDKITE_PULL_REQUEST_BASE_BRANCH}"
BUILDKITE_PULL_REQUEST_REPO: "${BUILDKITE_PULL_REQUEST_REPO}"
40 changes: 40 additions & 0 deletions .github/workflows/build-primer-app.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: Build the frontend

on:
push:
branches:
- main
- gh-readonly-queue/main/*
pull_request:

jobs:
build:
permissions:
contents: read
id-token: write
deployments: write
runs-on: ubuntu-latest

steps:
- name: Checkout repo
uses: actions/checkout@v3
with:
# Required by the Chromatic GitHub Action
fetch-depth: 0

- name: Setup pnpm
uses: pnpm/action-setup@v2.2.1
with:
version: 7.1.0

- name: Setup nodejs
uses: actions/setup-node@v3.2.0
with:
node-version: 14
cache: "pnpm"

- name: Install dependencies
run: pnpm install --frozen-lockfile

- name: Build the frontend
run: pnpm build
9 changes: 3 additions & 6 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ tsconfig.tsbuildinfo
# tsserver
.log/

# yarn
# dependencies
node_modules/
yarn-debug.log*
yarn-error.log*
/.pnp
.pnp.js

# vite
.vite/
Expand All @@ -40,6 +40,3 @@ build-storybook.log
# VSCode
*.code-workspace
.vscode

# Ultra
.ultra.cache.json
5 changes: 5 additions & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
engine-strict=true
auto-install-peers=true
public-hoist-pattern[]=*storybook*
public-hoist-pattern[]=*react*
public-hoist-pattern[]=*babel*
2 changes: 1 addition & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
README.md
node_modules/
dist/
yarn.lock
pnpm-lock.yaml
.direnv/
.log
result*
Expand Down
289 changes: 91 additions & 198 deletions README.md

Large diffs are not rendered by default.

8 changes: 0 additions & 8 deletions bors.toml

This file was deleted.

108 changes: 72 additions & 36 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3588372

Please sign in to comment.