fix: repair main after the MYR, MKD and COP merges - #557
Merged
BastiOfBerlin merged 1 commit intoAug 13, 2026
Conversation
spliit-app#486, spliit-app#507 and spliit-app#521 each hand-edited src/lib/currency-data.json on a base that predated the Arabic locale (spliit-app#540), so the merged result has `ar` with 31 currencies while the other 23 locales have 34. getCurrency() indexes the union of all locale objects with the union of all supported codes, so the missing keys fail `npm run check-types`: src/lib/currency.ts(90,5): error TS7053: Element implicitly has an 'any' type ... Property 'MKD' does not exist on type '{ USD: ...; }' Regenerate currency-data.json with `npm run generate-currency-data` so every locale carries every supported code. Besides filling the `ar` gap, this replaces the hand-written MKD values from spliit-app#507 with the ones currency-list produces (symbol_native `ден` -> `MKD`), bringing MKD in line with how the rest of the file is generated. Also fix the two formatting regressions that fail `npm run check-formatting`: restore the indentation of the ternary in defaultCurrencyList that spliit-app#521 re-indented, and wrap the generator's writeFileSync call. The generator now writes a trailing newline as well, so a freshly generated data file is Prettier-clean instead of always failing the check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011GxpJPm1oVMoATQHHg9wyP
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>
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
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.
fix: repair main after the MYR, MKD and COP merges
mainis currently red. #486 (MYR), #507 (MKD) and #521 (COP) were each green on their own base, but none of them was rebased onto the branch that added the Arabic locale (#540). All three hand-editedsrc/lib/currency-data.jsonfor the 23 locales their branch knew about, so after the mergesarhas 31 currencies while every other locale has 34.getCurrency()indexes the union of all locale objects with the union of all supported codes, so the missing keys break type checking:npm run check-formattingfails too, on three files:What this PR changes
1. Regenerate
src/lib/currency-data.jsonwith the existingnpm run generate-currency-data, so every locale carries every supported code. Verified: 24 locales × 34 currencies, no gaps.The only entries that differ from the current
main:MKD,MYR,COPto thearlocaleMKDin the other 23 localessymbol_nativeден→MKD,symbolden→MKD, andname/name_pluralto thecurrency-listwordingWorth flagging explicitly: #507 hand-wrote those MKD values rather than taking them from
currency-list, so regenerating replaces them. That brings MKD in line with how every other currency in the file is produced — after this PR the file is exactly what the generator emits. If you'd rather keepденas the native symbol, it should go through the generator (e.g. a small override map) so the file stays reproducible; happy to add that here if you prefer.2.
src/scripts/generateCurrencyData.ts— write a trailing newline. Without it, a freshly generated data file always failsprettier -c src; that missing newline was Prettier's only complaint about the JSON. ThewriteFileSynccall is also wrapped, since #521 left it at 86 characters.3.
src/lib/currency.ts— restore the indentation of thecustomChoiceternary indefaultCurrencyList. #521 re-indented that block by two spaces as collateral damage; the contents are unchanged, so this is formatting only.supportedCurrencyCodesis untouched — MYR, MKD and COP all stay.Verification
Full CI sequence from
.github/workflows/ci.yml, on Node 24:Re-running
npm run generate-currency-dataafter the commit leaves a clean working tree, which confirms the checked-in JSON is exactly the generator's output and that the trailing-newline fix holds.Suggestion
Enabling Require branches to be up to date before merging on
mainwould make this class of merge skew fail on the PR rather than onmain— all three PRs here passed individually and only conflicted semantically once combined.