Migrate frontend & e2e from npm to pnpm

[MooseTheRebel]

What changed?

Moves frontend/ and test/e2e/ from npm to pnpm. I rebuilt the branch and force-pushed onto a fresh main, so it's now 6 commits:

1. Fix phantom devDependencies for pnpm compatibility — cherry-picked from Fix phantom devDependencies for pnpm compatibility #1769. Adds @eslint/js and typescript-eslint as explicit devDependencies in frontend/, so pnpm's strict node_modules doesn't break on eslint's config.
2. Migrate frontend and e2e to pnpm — the core migration (pnpm import, new pnpm-lock.yaml/pnpm-workspace.yaml, dropped package-lock.json) for both frontend/ and test/e2e/, plus the matching CI workflow changes (validate.yml, deploy-production.yml, deploy-staging.yml, and the e2e nightly workflows): pnpm/action-setup, cache: 'pnpm', and the Node 20→24 bump pnpm 11 requires.
3. Fix Dockerfiles and e2e uuid pnpm override — migrates frontend/Dockerfile and frontend/deploy.dockerfile to pnpm, and moves test/e2e's npm-only overrides field (pinning uuid) to pnpm-workspace.yaml, since pnpm silently ignores overrides in package.json.
4. Bump engines to Node 22, dedup typescript-eslint — raises engines.node to >=22.13.0 (pnpm 11 / vite 8's actual floor) in both frontend/ and test/e2e/, and drops the @typescript-eslint/{eslint-plugin,parser} pins that duplicate the bundled typescript-eslint package.
5. Update frontend and e2e READMEs for pnpm — updates both READMEs' install/run instructions from npm/npx to pnpm.
6. Bump checkout and setup-node actions to v7 — bumps actions/checkout and actions/setup-node to v7 across the 8 workflow files touched above. Keeps cache: 'pnpm' explicit — setup-node's automatic caching via the packageManager field only applies when it's set to npm, not pnpm.

Why?

Quoting @davinotdavid from the issue description:

• Strict dependency isolation (no phantom deps, it forces me to be explicit about my dependencies which I think is the right way to do deps cause no hidden surprises)
• Disk space savings and CI performance improvements (lighter node_modules cause no duplicates, efficiently uses symlinks)
• All dependency lifecycle scripts are ignored by default (need to whitelist each package that requires its scripts to run)

Reviewer feedback addressed

From the previous round of review:

• [aatchison] Blocking: deploy.dockerfile/Dockerfile still use npm — fixed in commit 3.
• [aatchison] Blocking: test/e2e overrides silently dropped by pnpm — fixed in commit 3.
• [aatchison] Blocking: rebase onto main + regenerate lockfiles — done; branch is rebuilt directly on top of current main.
• [aatchison] Blocking: prove a clean pnpm install --frozen-lockfile — verified for both frontend/ and test/e2e/ at each step.
• [aatchison] engines.node floor too low for pnpm/vite — fixed in commit 4.
• [aatchison] Duplicate @typescript-eslint toolchain — fixed in commit 4.
• [devmount / rwood-moz] Update frontend/README.md and test/e2e/README.md — fixed in commit 5.
• [devmount] Bump actions/checkout/actions/setup-node to a current version — fixed in commit 6.
• [devmount] davinotdavid: is deploy.dockerfile still used? — yes: confirmed by aatchison's review, it feeds the multi-arch GHCR image for the EKS deploy + Kargo promotion pipeline.

Reproduce this PR yourself

Commits 2, 3, 4, and 6 above were generated by migrate-to-pnpm.sh. I wrote this as a scripted-patching experiment for a different project and figured it fit here too. I'm also trying to be careful about trust & security. I didn't want to hand over a pnpm lockfile I generated on my machine and ask you to take it on faith 🙂

You can run the script yourselves to generate the lockfiles, not just reviewing a diff of ones I made. Tested against main @ e9cca499 — if main has since diverged non-trivially, the cherry-pick step fails with a clear message rather than silently producing something different from this PR. Requires Node >=22.13.0 and pnpm >=10 (e.g. via corepack enable); the script checks both and fails before touching any files if they're missing.

git clone https://github.com/thunderbird/appointment.git appointment-pnpm-repro
cd appointment-pnpm-repro
git status --porcelain  # should be empty

curl -O https://gist.githubusercontent.com/MooseTheRebel/78949877babc85ff3b9d6994e854dd46/raw/migrate-to-pnpm.sh
chmod +x migrate-to-pnpm.sh

Recreate the diff, uncommitted:

./migrate-to-pnpm.sh

This leaves you with one uncommitted diff (staged + unstaged) matching this PR exactly. Nothing is committed for you, so you can review, git diff, or amend as you like.

Or recreate the exact commits (needs local git setup — creates local commits only, pushes nothing):

./migrate-to-pnpm.sh --with-step-commits

This reproduces the PR's 6 commits, including the #1769 cherry-pick, so you can step through the migration with git log -p.

The script cherry-picks #1769's fix as step 0, then runs the pnpm migration, the Dockerfile/overrides fixes, the engines/dedup cleanup, the README updates, and the actions version bump. Each step checks whether it's already applied and skips if so, so it's safe to re-run — but it expects a clean working tree and won't run around unrelated local changes.

Limitations and Notes

• A project member will have to authorize running the github actions (apologies in advance if any of them fail)
• The migration deliberately removes npm lockfiles; there's no in-place rollback, but recovery is a normal git operation: git reset --hard HEAD && git clean -fd (uncommitted mode) or resetting to the pre-migration commit (commit mode) — same idea as reverting this PR or reviving the files from git history.
• E2E jobs were bumped from node-version: 20 to node-version: 24 — pnpm 11 requires Node ≥22.13 and fails outright on Node 20. Per rwood-moz's review, this is safe: the E2E jobs run in BrowserStack (not installing Playwright browsers locally), where the Node 24 / Playwright 1.59 interaction that blocks local browser installs doesn't apply.

Applicable Issues

#1731

Screenshots

N/A

[markstos]

Tagging @davinotdavid for review as the person who opened the issue and @rwood-moz as our e2e test expert. On my initial scan through, it looked good.

[davinotdavid]

Thanks for the tag! Tagging @devmount as I believed he had a plan queued to tackle this so it would be nice to compare notes to check completeness.

Two parts the worry me a little bit:

1. It has been a pain to maintain to E2E tests as matching node versions with working browserstack versions are quite troublesome (different versions here require manual intervention to check what works and what doesn't on mobile tests iirc). @rwood-moz would be a great person to verify those.

2. We are currently changing the way we build our frontend application in regards to infra / devops [reference PR] so I'd also love to have a review from @aatchison as well.

[MooseTheRebel]

@davinotdavid That's fair, and I appreciate the worry 🙂

To reduce scope and complexity, I've opened #1753 .

[devmount]

@MooseTheRebel Thank you so much for taking the time and contribute to this repo! I have two notes below that apply to most of the workflow files and need approval from @rwood-moz.

Also these change requests:

1. If we move to pnpm, we should also update the docs in frontend/README.md and test/e2e/README.md.
2. We're currently still building the frontend with npm install iirc (frontend/Dockerfile). We should use pnpm here too for consistency and have it available in the frontend container. There is also a frontend/deploy.dockerfile, @davinotdavid do you know if we still use this or can we delete it 🤔

[rwood-moz]

Thank you for your patch @MooseTheRebel.

From an E2E test point of view this LGTM. I pulled the PR onto my machine and ran E2E tests locally as well as in BrowserStack via pnpm, and they ran successfully. The GHA workflow changes also LGTM.

My only request is to please update the test/e2e/README as @devmount also mentioned.

Looks like you have some merge conflicts and you'll need to rebase, and also please await approvals from the others, but I'm approving in regards to the E2E changes. Thanks again!

[aatchison]

Review from Platform/Infra — thanks for driving the pnpm migration! 🙏

Ran a broad merge-safety + cross-repo build review. One critical blocker for our EKS/GHCR image build, a few merge-gating items, and two inline engines suggestions below. Note up front: allowBuilds: (map form) is correct pnpm 11 syntax — no change needed there.

🔴 Blocking — breaks our multi-arch GHCR / EKS build

This PR deletes frontend/package-lock.json, but the container builds still use npm and fail once it's gone:

• frontend/deploy.dockerfile → npm ci (the multi-arch GHCR image that feeds our EKS deploy + Kargo promotion)
• frontend/Dockerfile → npm install (local dev)

Neither file is in this PR's diff, so I can't inline-suggest them, but they must migrate in lockstep. Proposed for frontend/deploy.dockerfile (replace the npm ci / npm run build steps):

RUN corepack enable
RUN cd /build/frontend && pnpm install --frozen-lockfile
RUN cd /build/frontend && pnpm run build

and frontend/Dockerfile (local dev): RUN corepack enable && pnpm install --frozen-lockfile. The GHCR workflows themselves need no change (docker-build only). Happy to open a companion PR with these so they land as a matched set.

🟠 Before merge

1. Rebase onto main + regenerate pnpm-lock.yaml. The branch predates #1749's security bumps (eslint, @sentry/vite-plugin, @typescript-eslint/*, @vue/test-utils); merging as-is reverts them, and there's a package-lock.json modify/delete conflict.
2. Prove a clean install. pnpm 11's strictDepBuilds defaults to true, so an incomplete allowBuilds list makes pnpm install --frozen-lockfile fail hard (ERR_PNPM_IGNORED_BUILDS). CI hasn't run on this PR yet — please confirm a clean install + build for both frontend/ and test/e2e/, especially after the rebase regenerates the lockfile.
3. test/e2e overrides is silently dropped. Top-level overrides is an npm-only field; pnpm ignores it, so uuid is no longer pinned to 11.1.1 (resolves to 8.x/9.x). Move it under pnpm.overrides in package.json (or overrides: in pnpm-workspace.yaml) and regenerate the lock.
4. Rollback runway: consider keeping package-lock.json for one release as an escape hatch, and splitting the Node 20→24 bump into its own PR so the runtime upgrade and the package-manager migration can revert independently.

🟡 Nits

• Duplicate @typescript-eslint toolchain: adding typescript-eslint@8.61.1 while the explicit @typescript-eslint/eslint-plugin/parser@8.28.0 pins remain resolves two full toolchains — drop the explicit pins or align versions.
• frontend/README.md, test/e2e/README.md, and frontend/Dockerfile still reference npm; no corepack enable onboarding note.
• Nice catch promoting the @eslint/js / typescript-eslint phantom deps — exactly right for pnpm's strict node_modules.

Inline engines suggestions below. Thanks again — glad to pair on the Dockerfile migration.

[MooseTheRebel]

Hi all 👋 Thank you again for the open source work you do and for the detailed feedback on this PR!

Hopefully, the changes I made match what you were hoping to see. If not, please let me know and I'm happy to make additional updates. The gist I created (migrate-to-pnpm.sh) might have been a bit of over-engineering. I started on it mostly to help myself with an issue on a different project 🙂

I try to keep my diffs small and my engineering right-sized, so please don't take this PR as a signal of my typical style. The linked PR is more my speed, and it's available for you all if you prefer just that smaller change.

Let me know if there's anything I can do to help with this PR, or a different issue entirely where I could contribute!

EDIT: Oh, also, I suppose someone will have to approve github actions to run. Apologies in advance if any of them fail (hopefully not 🤞 ).

[aatchison]

@MooseTheRebel Taking another look. Thanks for this effort!

[aatchison]

Re-review — one new blocker.
🔴 Five suggestions inline below
Two nits with no inline anchor: .dockerignore's header comment still describes npm ci, and package.json's format script still uses npx.

Heads-up: green CI won't clear the --mode bug — it wouldn't catch it either.

[aatchison]

Verified on f3bcdbf0 — approving. ✅

Re-ran the checks locally rather than reading the diff: --mode now reaches vite in both deploy jobs (built with the real APP_ENV values, stage and prod — the stage Sentry DSN + PostHog key and the prod URLs bake in correctly), vitest now actually receives --run, clean pnpm install --frozen-lockfile, lint + build + 97/97 tests, and both images rebuild end to end.

CI still hasn't been authorized on this PR, so this approval rests on that local run rather than on green checks. @devmount's earlier change request is what's still gating the merge.

[MooseTheRebel]

@aatchison That was fast! 😄 Thank you for the detailed feedback, and I'm sorry I didn't catch those things before. I took the additional step of simulating a "red/green" test locally, using act, and the following workflow files (attached below):

• mode-bug.yml — reproduces the stage/prod frontend build issue
• dashdash-bug.yml — reproduces the lint/test issues

You can try these yourself, using:

# Reproduce and verify the fix for the build --mode bug (deploy-staging.yml, both the
# stage and prod jobs use this same pattern). Runs the buggy command first (red),
# then the fixed command (green).
act workflow_dispatch -W mode-bug.yml --container-architecture linux/amd64

# Reproduce and verify the fix for lint -- --fix and test -- --run
act workflow_dispatch -W dashdash-bug.yml --container-architecture linux/amd64

Both confirmed the bugs as you described — I also fixed the two nits (.dockerignore comment, package.json format script).

dashdash-bug.yml

mode-bug.yml

[MooseTheRebel]

@devmount Sorry for the spam, I tried using the "Re-request review" button to ping you, but it seems like Github isn't doing anything when I click that button 🤔

Please re-review this PR and let me know if there are any changes needed. Thank you again! 🙂

[devmount]

Thanks a lot for all your work on this! I'm currently on vacation and approved without having looked at the changes again to enable the merge.

[aatchison]

CI has now genuinely run against this tree: fork PRs never trigger validate.yml (see #1774), so I pushed 51a2e5d7 to ci/verify-pnpm-1746 in-repo and run 30471822364 came back green on all four jobs — frontend lint/build/vitest included. Approved to merge from my side @markstos @MooseTheRebel ; I will delete ci/verify-pnpm-1746 once this lands.