Skip to content

Add a type-level test suite and align client types with runtime behavior - #33

Open
kac-axelor wants to merge 13 commits into
axelor:devfrom
kac-axelor:type-safety
Open

kac-axelor wants to merge 13 commits into
axelor:devfrom
kac-axelor:type-safety

Conversation

@kac-axelor

@kac-axelor kac-axelor commented Jun 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The client's declared result types and what the runtime actually delivers had
drifted apart across payloads, row counts, and aggregates — including one real
runtime bug (decimal sums truncated by parseInt). This PR adds a type-level
test harness (pnpm test:types) whose first commit is the complete spec of
expectations, then fixes every mismatch, finishing with a redesigned aggregate
result contract.

How to review

Read commit by commit: the first commit asserts 63 expectations (13 red on
dev by design), every later commit is pure fix code, and the failing count
is the narrative:

Commit test:types after
Add type-level test harness and client type expectations 13 failed / 63
Type row-count results as numbers 12
Type _count pagination metadata as a number 11
Drop null from selected collection payloads 10
Carry lazy value nullability inside the Json/Text/Binary aliases 9
Exclude relations and lazy columns from PayloadSimple 8
Return computed aggregates as the driver's exact strings 7
Type min/max results as the column's model type or null 6
Admit null for groupBy values of nullable columns 2
Convert min/max/groupBy aggregate values by column type 2 (runtime fix — proven by e2e)
Make mappedBy fields optional on nested creates 0
Cover remaining aggregate scenarios in e2e tests 0 (coverage)

Payload and repository fixes

Surface Was declared Runtime truth Now declared
count / delete / updateAll / deleteAll Promise<ID> (string | number) always a number Promise<number>
_count pagination metadata string number from getCount() number
Selected collections T[] | null when optional always an array ([] when empty) T[]
Selected lazy fields (Json/Text/Binary) Json | null (null outside) promise always present; awaited value can be null Promise<V | null>
create/update without select (PayloadSimple) included relations and lazy fields only plain scalar columns are loaded relations and lazy fields excluded
Nested create back-reference (mappedBy) required auto-filled by the runtime omittable

Aggregate contract (breaking change)

Computed values — no column type exists for these, so a result type is
designed; the value is always exact and conversion is the caller's choice:

Operation Was declared Was runtime Now (declared = runtime)
count number number string, never null (no rows → "0")
sum number parseInt truncated decimals (3.75 → 3); null untyped string | null, exact
avg number float; null untyped string | null, exact

Column values — min/max/groupBy return stored values, which now keep the
field's own type, exactly as a find would deliver them:

Field type Was declared Was runtime Now (declared = runtime)
Decimal number / BigDecimal number if positive, raw string if negative BigDecimal
Int number (no null) number number
String string "007" grouped as the number 7 string
Enum enum member (no null) string enum member
Boolean boolean (no null) boolean; null bucket untyped boolean
Date string Date object date string
Time string string string
DateTime Date (no null) Date Date
bigint id string parsed to number string
NULL (empty set / null group) absent from types null delivered | null for nullable columns and empty sets

Conversion is driven by column metadata (resolved through relations), not by
what the value looks like.

Migration

Before After
row.sum.total + 10 Number(row.sum.total) + 10
row.count.id > 0 Number(row.count.id) > 0
exact money math on sum new BigDecimal(row.sum.total)
min.population === 1.25 min.population?.equals(new BigDecimal("1.25"))
very large counts BigInt(row.count.id)

Tests

Suite Command Result
Type expectations pnpm test:types 63/63 at tip (red by design before each fix)
e2e (real PostgreSQL) pnpm test:e2e 226/226, incl. per-cell aggregate proofs
Unit pnpm test 147/147

- pnpm test:types typechecks the *.test-d.ts suites via vitest typecheck mode; its global setup only generates the test client, no database needed.
- The suites assert what the runtime should deliver: payload shapes from doQuery/ensureLazy, row counts as numbers, and aggregate values per their contract — computed aggregates (count/sum/avg) as the driver's exact strings, min/max/groupBy as the column's model type (Decimal stays BigDecimal).
- 13 of 63 assertions fail on this commit by design: they are the expectations, and each following commit fixes one contract until the suite is green.
count() returns getCount() and delete/updateAll/deleteAll return
`affected ?? 0` — always numbers at runtime. Promise<ID> forced
callers to handle a string case that never occurs.
doQuery assigns the result of getCount() to record._count.
doQuery assigns an array to collection properties — empty when there
are no related records, never null. The null came from the model
field's optionality, which does not apply to query results.
- loadLazy resolves the column value or null on every read, so the aliases declare Promise<V | null> at the source instead of payload types patching it in.
- PayloadSelect passes lazy fields through unchanged, dropping only the model field's optionality null: ensureLazy always installs the getter on selected fields.
- The old payload type Json | null claimed the field itself could be null while hiding that the awaited value can be.
create/createAll without a select load getSimpleSelect() — plain
scalar columns only. OmitByType never matched optional fields (their
null member fails the extends check), so every nullable relation and
text/jsonb/oid column leaked into the declared result.
- count/sum/avg have no column type to preserve: pg widens them (sum(int) is bigint, avg(int) is numeric) and the driver delivers exact numeric strings, which now pass through untouched.
- Converting to the right precision becomes the caller's choice: Number() for display math, BigDecimal for exact arithmetic, BigInt for huge counts.
- count is never null (NULL becomes "0"); sum/avg stay null over empty or all-null sets; parseInt truncation of decimal sums (3.75 read as 3) disappears along with the parsing.
- min/max return stored column values, so the result keeps the column's type: ints are numbers, strings stay strings, timestamps are Dates, and Decimal columns are BigDecimal — the Date/string/boolean conditional chain with its number fallthrough collapses to T | null.
- null covers the empty or all-null set, which the database reports as NULL.
runAggregate passes column nulls through for groupBy; the null group
is a real bucket. The declared type stripped it via NonNullable.
- min/max/groupBy carry stored column values, so each keeps its column's model type — the old value-shape sniffing returned negative decimals as raw strings, grouped numeric-looking string keys like "007" as the number 7, parsed bigint ids to numbers against their string typing, and let date columns through as driver Date objects.
- The aggregate path now resolves to the column's metadata (walking relations) and converts only where the driver's shape differs from entity hydration: numeric columns become BigDecimal, date columns become date strings via TypeORM's own DateUtils; everything else passes through.
- e2e covers negative decimal min/max/groupBy as BigDecimal, leading-zero string group keys, bigint id group keys, and date/time/timestamp aggregates.
The ORM auto-fills a collection's mappedBy field from the parent
record, yet nested creates still demanded it, forcing callers to pass
dummy links like `contact: {}`.

Carry the schema's mappedBy key into the generated model types through
a branded collection type — ToMany<T, MappedBy> — emitted for OneToMany
fields, and relax exactly that field in nested CreateArgs. Mirror the
rule in the GraphQL schema with per-relation CreateWithout<Field>Input
variants, generated only when the target declares the field required.

Direct creates keep requiring the field: there is no parent to fill it
from. Runtime tests drop the now-unnecessary contact: {} workaround.
- null group buckets for nullable columns, empty-set nulls and the "0" count, boolean and enum grouping, grouping through relations, and integer sum/avg as exact strings with min/max as numbers.
- count/sum/avg arrive as the database's exact decimal strings; converting to the right precision (Number, BigDecimal, BigInt) is the caller's choice.
- min/max/groupBy keep the column's model type, including BigDecimal for Decimal columns, with nulls for empty sets and the null group bucket.
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.

1 participant