Skip to content

fix: repair main after the MYR, MKD and COP merges - #557

Merged
BastiOfBerlin merged 1 commit into
spliit-app:mainfrom
BastiOfBerlin:fix/currency-data-build
Aug 13, 2026
Merged

fix: repair main after the MYR, MKD and COP merges#557
BastiOfBerlin merged 1 commit into
spliit-app:mainfrom
BastiOfBerlin:fix/currency-data-build

Conversation

@BastiOfBerlin

Copy link
Copy Markdown
Collaborator

fix: repair main after the MYR, MKD and COP merges

main is 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-edited src/lib/currency-data.json for the 23 locales their branch knew about, so after the merges ar has 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-types
src/lib/currency.ts(90,5): error TS7053: Element implicitly has an 'any' type because
expression of type '"USD" | "EUR" | ... | "COP"' can't be used to index type '{ ... }'.
  Property 'MKD' does not exist on type '{ USD: {...}; EUR: {...}; ... }'.

npm run check-formatting fails too, on three files:

[warn] src/lib/currency-data.json
[warn] src/lib/currency.ts
[warn] src/scripts/generateCurrencyData.ts

What this PR changes

1. Regenerate src/lib/currency-data.json with the existing npm 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:

Change Entries
added MKD, MYR, COP to the ar locale 3 — this is the actual build fix
changed MKD in the other 23 locales symbol_native денMKD, symbol denMKD, and name/name_plural to the currency-list wording

Worth 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 fails prettier -c src; that missing newline was Prettier's only complaint about the JSON. The writeFileSync call is also wrapped, since #521 left it at 86 characters.

3. src/lib/currency.ts — restore the indentation of the customChoice ternary in defaultCurrencyList. #521 re-indented that block by two spaces as collateral damage; the contents are unchanged, so this is formatting only.

supportedCurrencyCodes is untouched — MYR, MKD and COP all stay.

Verification

Full CI sequence from .github/workflows/ci.yml, on Node 24:

npm ci --ignore-scripts
npx prisma generate
npm run check-types        # silent
npm run lint               # 16 problems (0 errors, 16 warnings) — unchanged from before
npm run check-formatting   # All matched files use Prettier code style!
npx jest                   # 27 tests passed

Re-running npm run generate-currency-data after 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 main would make this class of merge skew fail on the PR rather than on main — all three PRs here passed individually and only conflicted semantically once combined.

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
@BastiOfBerlin
BastiOfBerlin merged commit c8f04df into spliit-app:main Aug 13, 2026
1 check passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants