CI Baseline - #564
Merged
Merged
Conversation
eslint.config.mjs is the config ESLint 9 actually loads; the .eslintrc.json sitting next to it has been inert since the flat config landed. Keeping both only invites edits to the file that has no effect.
`noEmit` is set, so tsc never emits and SWC handles transpilation — the
target here only decides what type-checks. At es5 that means TS2802 on any
iteration of a Map or a Set:
error TS2802: Type 'Map<string, number>' can only be iterated through
when using the '--downlevelIteration' flag or with a '--target' of
'es2015' or higher.
So `for (const [k, v] of map)` and `[...map.values()]` are both rejected,
and code that needs them has to be written around the checker rather than
for the reader. ES2017 is the current Next.js default.
This also has to happen eventually regardless: TypeScript 6 deprecates the
es5 target and TypeScript 7 removes it.
There are seven test suites and 101 tests in the repository, and CI runs none of them — `npm test` passes locally and nothing enforces that it keeps passing. Add it as a step after the existing checks. Also moves checkout and setup-node from v3 to v6. v3 runs on the Node 16 runner, which GitHub has been warning about for some time, and records the reason the job pins Node 24: npm 10 refuses to read lockfiles written by npm 11, which is what Dependabot produces.
Weekly checks on all three ecosystems. Minor and patch updates are grouped into a single PR per ecosystem so a quiet week produces one review rather than six; majors stay separate, since those are the ones worth reading on their own. The github-actions ecosystem is included because nothing was watching the workflow pins, which is how checkout and setup-node ended up three majors behind.
BastiOfBerlin
pushed a commit
to BastiOfBerlin/spliit
that referenced
this pull request
Aug 13, 2026
Brings in spliit-app#563 (build-image.sh parsing) and spliit-app#564 (CI baseline), both ours, both landed byte-identical -- verified with `git diff <branch> upstream/main --stat` before merging: each branch differed from upstream only by the other PR's files. Two conflicts, both our own code returning after a squash: - scripts/build-image.sh -- took upstream. Identical logic; only the comment differed, and upstream carries the reworded version written for the PR (the fork's said "see upstream spliit-app#219", wrong voice once it is upstream). - .github/dependabot.yml -- took upstream, a strict superset: minor/patch grouping per ecosystem plus the github-actions ecosystem, neither of which the fork's original config had. Auto-merged and checked by hand: - .github/workflows/ci.yml -- gained upstream's `npm test` step. CI on this fork now runs the suite too, which it did not before. - tsconfig.json -- kept the fork's `types: [node, jest]` on top of the shared `target: ES2017`. That line is TypeScript 6-specific and is deliberately held back for the major-upgrades PR. Four of the five touched files are now byte-identical to upstream; tsconfig differs only by the held-back `types` line. Verified with Node 24: check-types, check-formatting, lint (19 warnings, 0 errors), npm test -- 9 suites, 148 tests.
This was referenced Aug 13, 2026
BastiOfBerlin
added a commit
that referenced
this pull request
Aug 17, 2026
# deps: major framework upgrades Part of the series in #553, and the one I'd most like a second opinion on. Six commits, 31 files plus the lockfile. **Lead with the evidence, because it is the whole argument for this PR: your E2E suite passes 41/41 against this branch** — a real Chromium against a real Postgres 16, the full migration history applied from scratch, the built app served by `next start`. Every upgrade below is exercised by that run, including the one that changes how deployments connect to the database. ## What moves | | from | to | |---|---|---| | Tailwind | 3.4.19 | 4.3.2 | | zod | 3.25.76 | 4 | | `@hookform/resolvers` | 3.10.0 | 5 | | openai | 4.104.0 | 6 | | TypeScript | 5.9.3 | 6.0.3 | | jest | 29.7.0 | 30.4.2 | | lucide-react | 0.501.0 | 1.17.0 | | `@types/react` / `-dom` | 18.3.x | 19 | | `@types/node` | 20.19.43 | 24 | | Prisma | 6.19.3 | 7.9.1 | Versions are compared against what the lockfile actually installs, not what the manifest declares — the method note from #576, which is also why `next` is absent from this table: 16.3.1 is already installed here. ## Six commits, in review order 1. **`engines`** — Node 24 / npm 11, which CI and the Dockerfile have used since #543. 2. **Tailwind 4.** 3. **zod 4 + resolvers 5 + openai 6.** 4. **Dropping the casts zod 4 makes unnecessary.** Droppable on its own. 5. **TypeScript 6, jest 30, lucide-react 1, React 19 types.** 6. **Prisma 7 + the pg driver adapter.** Last and self-contained, so it can be split off with one rebase. Every commit was checked to resolve independently — `npm install --dry-run` against each of the six, no ERESOLVE. That check is here because it caught a real defect: see the openai note below. ## Prisma 7 — the part that needs your judgement It changes how every deployment reaches Postgres, so it is the last commit and nothing else depends on it. - **Connection URLs leave `schema.prisma`** for a new `prisma.config.ts`. The same two variables are read in the same order, so **no self-hoster has to change any configuration**: `POSTGRES_PRISMA_URL` and `POSTGRES_URL_NON_POOLING` keep their exact current meaning. - **The client needs an explicit driver adapter** — `PrismaPg` over the pooled URL. `pg` was already a dependency. - **The client generates into `src/generated/prisma`** rather than node_modules, so 17 imports move to `@/generated/prisma/client` or `.../browser`. The directory is gitignored, dockerignored and lint-excluded. What I could verify, and did, rather than reasoning from the changelog: - `prisma migrate deploy` applies the **full migration history to an empty database** under Prisma 7. - It also works from a **production-only install** (`npm ci --omit=dev --ignore-scripts`) — the exact tree the runtime stage has, which is what the container runs at start-up. This was the failure mode I most expected and it does not happen. - The runtime stage does **not** need the generated client copied in. I checked by deleting `src/generated` and starting the server: it serves and reaches the database, because the build bundles the client into `.next`. So the runtime stage stops running `prisma generate` entirely. Two consequences found by building rather than reading: - `src/lib/api.ts` exported `randomId`, and two client components imported it from there. Harmless under Prisma 6; under Prisma 7 it drags the pg adapter and `pg` into the browser bundle and **the build fails**. `randomId` moves to `src/lib/random.ts`, and `api.ts` re-exports it so server callers are untouched. - The browser namespace exports `Prisma.Decimal` as a value only, so `src/trpc/client.tsx` derives the instance type for its superjson registration. **What I could not verify: `docker build` itself.** There is no Docker daemon where I work. The Dockerfile changes are three targeted edits — copy `prisma.config.ts`, move `prisma generate` after the source copy, drop the regenerate from the runtime stage — and each was reasoned from a behaviour I tested outside the image, but the build has not been run. That is the one thing this PR needs from someone who can run it. ## zod 4 is the most interesting change zod 4 splits a schema's input and output types, and `expenseFormSchema` coerces heavily — `z.coerce.date()`, the string-or-number amount union, several `.default()`s. So what react-hook-form holds while you type genuinely is not what the resolver returns on submit. Under zod 3 both were `z.infer` and the gap was papered over with casts. `ExpenseFormValues` stays `z.output` (so no caller changes) and a new `ExpenseFormInput` is `z.input`, with the form typed `useForm<ExpenseFormInput, any, ExpenseFormValues>`. **The measurable payoff is commit 4: nine of the twelve `as any` casts in `expense-form.tsx` disappear**, because the string values the form assigns to `shares`, `originalAmount` and `paidFor` now type-check as themselves. The two `as any` in `schemas.ts` go too, since `z.enum()` takes an enum object directly: ```ts z.enum<SplitMode, [SplitMode, ...SplitMode[]]>(Object.values(SplitMode) as any) z.enum(SplitMode) ``` Being straight about the trade: the upgrade also **adds seven casts**, mostly `form.watch('expenseDate') as Date`, because the input type of `z.coerce.date()` is `unknown`. Those are narrow casts to real types rather than `any`, and they sit exactly where raw form state is read as parsed state. Net across the file: twelve `any` become three `any` plus seven typed casts. The three casts left are not zod's doing and I left them alone rather than widen the diff — `SplittingOptions` modelling what localStorage holds, Radix typing `onValueChange` as `(value: string)`, and an `isNaN(date as any)` that really wants `date.getTime()`. `required_error` is gone in zod 4, replaced by an `error` callback. The callback returns the existing message key only when the input is missing, which I checked directly rather than trusting: parsing `{}` still yields `titleRequired`, `amountRequired` and `paidByRequired`, and a one-character title still yields `min2` rather than the required key. So `SchemaErrors` lookups are unchanged. **openai 6 is in this PR because zod 4 forces it, not because I wanted to bundle it.** openai 4 declares `peerOptional zod@"^3.23.8"`, so zod 4 and openai 4 cannot resolve together — `npm install` fails outright with ERESOLVE. I originally had openai queued for a later PR and only found this because I build-checked each commit in isolation; the first version of the zod commit carried a lockfile that npm had produced by skipping re-resolution against an existing tree, and it would have failed for anyone installing from clean. The SDK bump needs no code change: the existing `chat.completions.create` call type-checks against 6 unmodified. ## Tailwind 4 `tailwind.config.js` is **unchanged**. v4 reads it through `@config`, which keeps the CSS-variable palette, the radius scale and the accordion keyframes working as-is rather than rewriting the theme into CSS-first `@theme` in the same commit as the upgrade. Worth doing eventually; not worth doing here. Two default-scale changes reach this codebase. I compared the compiled stylesheet instead of guessing: - **`shadow-sm` now emits what v3 called `shadow`** — a 3px blur instead of 2px. Three call sites: the card primitive, the recent-group card, the active tab. `shadow-md` is unchanged. - **`outline-none` now sets `outline-style: none`**, where v3 set a transparent 2px outline. All 23 uses pair it with `focus-visible:ring-2`, so the focus indicator is unaffected in normal rendering; the difference only shows in forced-colors mode. `rounded-sm` is unaffected because the config overrides the radius scale, and no site uses a bare `ring`, so v4 dropping the default ring from 3px to 1px does not bite. Net: this is a compatibility-mode migration, not a v4-idiom migration, and the only visual delta I can find is three slightly heavier shadows. ## TypeScript 6 and the rest - `types: ["node", "jest"]` — TS 6 changed ambient `@types` resolution, and without the list every `@types/*` in the tree loads into every compilation. That narrowing removes the ambient `Global` interface `src/lib/prisma.ts` used, so it now uses `globalThis`. - **TS 6 breaks `npm run generate-currency-data`.** The new TS5011 diagnostic makes `ts-node` demand an explicit `rootDir`. Setting it to the project root fixes the script and changes nothing else, since `noEmit` is set and every included file is already under it. Confirmed by regenerating: `currency-data.json` comes back byte-identical — which incidentally re-verifies #557's repair. - `@types/react` 19 catches up with the `react` 19.2.8 already installed; they had been a major apart since #479, flagged in #576. - **lucide-react 1 dropped its brand icons**, so the GitHub mark on the home page comes from `@radix-ui/react-icons` — already a direct dependency, and already the source of that same mark in the group list and the theme toggle. - jest 30 and `prettier-plugin-organize-imports` 4 needed nothing: 148 tests pass unmodified and the formatter produces identical output. ## Verification Against `4983038`, Node 24 / npm 11: `npm ci --ignore-scripts`, `npx prisma generate`, `check-types`, `check-formatting`, `npm test` (9 suites, 148 tests), `npm run lint` (**19 warnings / 0 errors — unchanged from `main`**), and a full `npm run build`. **E2E: 41/41.** Real Postgres 16, migrations applied from empty, the built app under `next start`, driven through `E2E_BASE_URL`. Not `scripts/e2e.sh` and not the image build — same app and same specs, but it does not exercise Docker. ## What I deliberately left out - **Newer majors that exist today**: TypeScript 7, eslint 10, nanoid 6, openai 7, `content-disposition` 3, `@types/node` 26, `react-intersection-observer` 11, `@testing-library/jest-dom` 7. Every one is a version this combination has never run. The strongest thing this PR has is that the exact set above has been running in my fork in production and passes your suite; adding untried versions would spend that for nothing. #564's grouped Dependabot config will propose them on its own schedule, which is the right way for them to arrive. - **The `@theme` migration** of `tailwind.config.js`, per above. - The two moderate `npm audit` advisories (transitive `uuid` via `next-s3-upload`) are unchanged before and after this PR. --------- Co-authored-by: Claude <noreply@anthropic.com>
BastiOfBerlin
added a commit
that referenced
this pull request
Aug 17, 2026
# build: standalone runtime image Last one in the series in #553. Three commits, 5 files, no dependency changes. **The runtime image is about 2.4× larger than it needs to be.** It installs a second, production-only `node_modules` and copies the whole `.next` directory on top. Almost none of that second install is ever loaded: `npm ci --omit=dev` resolves every production dependency, including the parts of packages the app never imports and the transitive tail behind them. Next.js can answer the "what is actually needed" question directly. `output: 'standalone'` traces the module graph reachable from the server and emits exactly those files, plus a `server.js` entry point. ## Measured, not estimated Both images built with `docker build` from the same commit, same base image, sizes read from `docker image inspect`: | | uncompressed | compressed (what a `docker pull` transfers) | |---|---|---| | `main` today | 1.6 GB | 316.2 MB | | this branch | 670 MB | 143.5 MB | | | **−58%** | **−55%** | Essentially the whole difference is one layer. From `docker history` on `main`: ``` 1.05GB COPY /usr/app/node_modules ./node_modules 41.5MB COPY /usr/app/.next ./.next ``` and on this branch: ``` 264MB COPY /opt/prisma-cli/node_modules ./node_modules 74.8MB COPY /usr/app/.next/standalone ./ 3.43MB COPY /usr/app/.next/static ./.next/static ``` The traced server that actually runs the app is **74.8 MB**, against 1.05 GB of installed dependencies. ## Three consequences worth reviewing - **The `runtime-deps` stage is gone.** It existed only to produce that `node_modules`. With it goes the class of bug #552 had to fix, where `--omit=optional` stripped sharp's platform binaries out of that install — tracing keeps a file because something reaches it, not because of which dependency bucket it was declared in. - **The Prisma CLI needs its own stage.** `migrate deploy` runs at container start, but the CLI is not part of the app's module graph, so nothing traces it. It also cannot just be copied out of the base stage: that stage installs with `--ignore-scripts` (the repo's `postinstall` runs `migrate deploy`, which can't run at build time), so `@prisma/engines` never fetches the schema engine `migrate deploy` needs. A small isolated install of the same pinned version, with scripts, produces a complete CLI. It reads the version out of the base stage rather than hardcoding it, so it cannot drift from the lockfile. - **The entrypoint invokes both by path.** A standalone image has no `package.json` scripts and no `node_modules/.bin` on `PATH`, so `npx prisma` would try to fetch the CLI over the network at container start, and `npm run start` has nothing to run. ## A bug this introduced, caught by running the image **Next.js copies `.env` into the standalone output.** The build stage does `COPY scripts/build.env .env` for its mocked values, so the first version of this image shipped those mocks at `/usr/app/.env` — a database URL pointing at `db`, `S3_UPLOAD_SECRET=AAAA…`, `OPENAI_API_KEY=XXXX…`. Today's image has no such file, so this would have been a regression. Real configuration from the container environment takes precedence, so it would not have broken a correctly-configured deployment. The bad case is quieter: a variable the operator *forgot* to set would resolve to a build placeholder instead of failing, and `POSTGRES_PRISMA_URL` in particular would silently point at a host called `db`. The build now deletes it, and the deletion is folded into the commit that causes the problem. Worth knowing about generally — it is a property of `output: 'standalone'`, not of this repo. ## Verified by running the image Against a real PostgreSQL 16 with an **empty** database: - `prisma migrate deploy` applies the full migration history from scratch, and the server starts — so the isolated CLI stage really is complete. - `/api/health/readiness` returns 200, which per `compose.e2e.yaml`'s own reasoning proves the schema is migrated and the app is serving. - Pages, static assets and `public/` files all serve. - **The full E2E suite passes 41/41 against the container itself**, rather than against `npm run start` as in the earlier PRs in this series. - **#591's runtime configuration still works through the standalone build**, which was the thing I most expected to break. The same image, restarted with a different environment: | | no runtime vars | `BASE_URL` + `DEFAULT_CURRENCY_CODE` set | |---|---|---| | `robots.txt` sitemap URL | `http://localhost:3000/…` | `https://standalone.example.com/…` | | `sitemap.xml` `<loc>` | `http://localhost:3000` | `https://standalone.example.com` | | new-group currency | `USD` | `EUR` | ## An observation for later, not part of this PR The Prisma CLI stage is now the largest layer at 264 MB, bigger than the app itself. Measuring an `npm install prisma@7.9.1` in isolation: ``` 43M @prisma/studio-core 34M effect 26M @electric-sql 19M @prisma/dev 7.7M elkjs 7.2M react-dom ``` Roughly 140 MB of the CLI is Prisma Studio and `prisma dev` — a graph-layout library and a React renderer — reached through `prisma`'s regular `dependencies`, so `--omit=optional` does not drop them. `migrate deploy` needs `@prisma/engines` (23 MB) and `@prisma/config` (56 KB). I have **not** tried to prune it here. A hand-maintained deny-list of package directories is exactly the kind of thing that breaks silently on the next Prisma upgrade, and this PR is already a large enough change to the runtime image. Flagging it as the obvious next target if image size stays interesting. ## What I could not verify - **`linux/arm64`.** Only amd64 here. The change is architecture-independent — no new binaries, and tracing produces the same file list — but the CD matrix builds arm64 natively and that leg has not been exercised. - **The GHA layer cache.** `cache-from`/`cache-to` only do anything on a real Actions runner, so commit 3 gets its first real test when you cut a tag. It is a separate commit and drops cleanly if you would rather not take it. I deliberately left out the action-version bumps that were on my list for this PR: #564 added `github-actions` to Dependabot, so it will propose them itself with release notes to check against, which beats me asserting a set of pins. --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
chore: CI and lint baseline
Part of the series in #553. Four small independent commits, no source changes —
this is the gate that everything after it in the series gets measured against,
which is why I'd like it in early.
1. CI runs the tests
mainhas 7 test suites and 101 tests, and CI runs none of them.npm testpasses today; nothing stops that changing. This adds one step after the existing
checks.
That number is recent — #548 brought two suites, and #559/#560/#562 brought five
more — so this isn't a long-standing gap so much as one that just opened. The
rest of #553 adds more, and I'd rather they land against a gate that exists.
2. TypeScript target
es5→ES2017noEmitis set, sotscnever emits anything and SWC does the actualtranspilation. The target here decides only what type-checks, and at
es5that means TS2802 on any iteration of a
MaporSet:Both
for (const [k, v] of map)and[...map.values()]are rejected. I hitthis porting #562 and wrote around it —
Map.forEachandArray.fromwhereiteration would read better — and flagged there that I'd propose the bump
separately rather than smuggle it in. This is that.
It also has to happen eventually regardless: TypeScript 6 deprecates the
es5target and TypeScript 7 removes it.
Not included: the
"types": ["node", "jest"]restriction that goes withthis in my fork. That one is specifically about TypeScript 6 changing how
ambient
@typesglobals are resolved, so it belongs with the TypeScript bump,not here.
I have deliberately not gone back and rewritten the
forEach/Array.fromcode now that the constraint is gone — it reads fine, and churning it would make
this PR look bigger than it is.
3. Drop the legacy
.eslintrc.jsoneslint.config.mjsis what ESLint 9 loads. The.eslintrc.jsonnext to it hasbeen inert since the flat config landed, and the only thing keeping it around
can do is attract an edit that silently has no effect.
Lint output is unchanged: 16 warnings, 0 errors, before and after.
4. Dependabot
New
.github/dependabot.yml: weekly npm, Docker, and GitHub Actions.Two choices here are mine and easy to strip if you disagree, so I want to
flag them rather than bury them:
separate. Ungrouped weekly npm updates produced nine separate PRs in my fork
over a few months, all of which I ended up reviewing as one batch anyway.
github-actionsis included. Nothing was watching the workflow pins,which is how
checkoutandsetup-nodeended up on v3 — the Node 16 runner.This PR moves them to v6 by hand; Dependabot is what stops it recurring.
If you'd rather not have Dependabot at all, commit 4 drops cleanly on its own
and the other three stand without it.
Also in this PR
The
setup-nodestep gets a comment recording why it pins Node 24: npm 10refuses to read lockfiles written by npm 11, which is what Dependabot produces.
That combination is a confusing failure if you meet it without the note.
What is not here
npm run generate-currency-datais listed under this item in #553 as broken. Itisn't — I re-checked, and
src/scripts/generateCurrencyData.tsis byte-identicalbetween my fork and
main. The breakage is TS5011 from my fork's TypeScript 6,so the fix belongs with that upgrade. I've corrected the tracking issue.
Verification
Against
67a8f8d:npm ci --ignore-scripts,npx prisma generate,check-types,lint(16 warnings / 0 errors, all pre-existing),check-formatting,npm test— 7 suites, 101 tests, all passing. The TS2802claim above was verified by running
tscat both targets over a four-lineprobe.