fix: declaration emit for consumers, wider zod peer range, and an examples workspace - #35
Merged
Merged
Conversation
`EntityStatic` — what `Entity(tag)(fields, options)` returns — was not exported, so TypeScript had no name to write for it and serialised the whole static surface structurally into every downstream package building with `declaration: true`, repeating the field map a dozen times. A one-field entity emitted a 274,048-byte declaration; it is now 240. That expansion was two reported build failures, not verbosity: - a 30-member domain enum pushed the repeated field map past the compiler's serialisation ceiling (TS7056, #31); - a branded object field was expanded through `DeepReadonly` until zod's module-private `$brand` reached computed-key position, where it cannot be named across a module boundary (TS4020, #32). Both surfaced only at the consumer's build, after everything else was green. Pinned by the consumer fixture, whose wide-enum entity is held at the reported width on purpose — trimming it drops back under the ceiling and it silently stops guarding. The consumer declaration-emit pass now runs twice, adding TypeScript 5.9.3 through a `typescript-consumer` alias. The repo's own 7.0.2 native port does not enforce the 5.x serialised-length ceiling, which is exactly why TS7056 shipped while this gate stayed green. The zod peer range widens to ^4.3.0. Nothing needed 4.4 — the range was just the version current at the initial release — and the floor is measured: full surface typechecks, emits and passes its runtime assertions on 4.3.0 (#33). Closes #31 Closes #32 Closes #33 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses downstream TypeScript declaration-emit failures by ensuring EntityStatic is emitted by reference (not structurally expanded) and by tightening the project’s consumer-compatibility checks to include a real TypeScript 5.x pass. It also widens the zod peer dependency range to match the measured minimum supported minor.
Changes:
- Export
EntityStaticas a top-level type and expose it asEntity.Staticfor consumers that hand-annotate. - Add a second consumer declaration-emit pass using a
typescript-consumeralias pinned to TS 5.9.3 (to catch TS7056 that TS 7.0.2 doesn’t enforce). - Widen
zodpeer range from^4.4.0to^4.3.0, with documentation explaining the measured floor.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds a typescript-consumer catalog alias for TS 5.9.3 to enable dual TypeScript passes. |
| pnpm-lock.yaml | Locks the new typescript-consumer alias resolution and TS 5.9.3 package entry. |
| packages/entity/tsconfig.consumer5.json | New config extending the consumer emit config, with a separate outDir for the TS 5.x pass. |
| packages/entity/src/index.ts | Exports EntityStatic from types.ts to ensure consumers can emit by reference. |
| packages/entity/src/entity.ts | Adds Entity.Static namespace alias for EntityStatic via the existing *Src indirection pattern. |
| packages/entity/README.md | Documents the widened zod peer range and rationale (measured floor, monorepo ergonomics). |
| packages/entity/package.json | Updates typecheck to run both consumer passes; adds typescript-consumer; widens zod peer range. |
| packages/entity/consumer/index.ts | Extends the consumer fixture to exercise Entity.Static and pin prior TS4020/TS7056 regressions. |
| CLAUDE.md | Updates repo guidance to reflect the new four-pass typecheck gate behavior. |
| .changeset/wide-entity-declaration-emit.md | Adds a changeset describing the declaration-emit fix and peer range change for release notes. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
An exported `const` holding `Entity.union(...)` hit the same root cause as EntityStatic one type further along: with no top-level name for what it returns, TypeScript expanded the union's members structurally and reached zod's module-private `$brand` through any branded field — TS4023: Exported variable 'BillingRecord' has or is using name '$brand' from external module ".../zod/v4/core/core" but cannot be named. That is the second error reported in #32, which the EntityStatic export alone did not cover. `UnionMember` travels with it as EntityUnion's own constraint. Isolated by removing the export and watching TS4023 return. Refs #32 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two entities and the vocabulary they are built from: branded fields, a branded Money value object, generated/immutable/computed, invariants as values, nesting, a union and factories — with specs, and the two declaration-emit passes that will take over from packages/entity/consumer/. Writing it immediately found a real bug: an exported const holding Entity.union(...) failed with TS4023 on $brand, fixed separately. The dunning vocabulary is held at thirty members deliberately; the README and emit-guards.ts say why, since it is the sort of thing a reader would otherwise helpfully trim.
…/entity consumer/ proved a downstream library emitting its own declarations could build against this package, but it documented nothing and looked like nothing else in the stack. examples/billing-domain is that same proof as a readable example, and it resolves @btravstack/entity through the real `exports` rather than a paths mapping — closer to what a consumer does. Verified by breaking the library on purpose: with EntityStatic un-exported, billing-domain fails with TS4020 and TS7056, exactly as the old fixture did. Restored afterwards. Corrects a claim made when the second pass was added. It is NOT that the 7.x native port ignores TS7056 — both versions enforce it, and 5.9.3's threshold is simply lower. Measured on the old fixture's narrower entity: 5.9.3 reported TS7056, 7.0.2 accepted the same shape and reported only TS4020; widen the entity and both report it. The second pass is still worth its cost, for the sharper reason that a band of realistic domain widths fails for consumers and passes here. knip needed a config for the first time: introducing one replaces the defaults, so the *.test-d.ts exclusion and four config-file-only dependencies had to be spelled out. Each is verified referenced, and the reason is inline.
An oRPC contract and JSON Schema built from Organization.createInput / .updateInput / .output, with nothing restating the shape of an Organization. The spec pins the design rule both ways: the four plain ZodObjects convert in both directions, and handing the class to a converter throws, because it parses to an instance. The JSON Schema exports carry an explicit JsonSchema annotation. Without it TypeScript infers a type it cannot name from outside the package and consumers emitting declarations fail with TS2883 — the same class of problem as #31 and #32, met from the other side.
toJSON() out, make() back, over an in-memory Map. The specs pin what is easy to lose: _tag never reaching a row, behaviour surviving rehydration (not just data), and update leaving the entity in hand untouched. byId returns InvalidEntity | OrganizationNotFound rather than folding the two together — a missing row is a 404, a corrupt row is worth paging someone about, and collapsing them discards the only fact that separates them. The library defines no NotFound on purpose: whether an absent row is exceptional belongs to the repository, so the example models it. No try/catch anywhere in the file.
An overview plus a page per example, in the nav and sharing the guide sidebar so a reader landing on one still reaches every other page. The overview says what amqp-contract's does and what is true here: unlike every fenced block in the rest of the guide, this code compiles and its specs run in CI. Those snippets are checked by review and nothing else, so they can drift from the library without any build noticing.
btravers
changed the title
Contributor
Author
|
Correction to the original description, since it stated this the wrong way round. I wrote that the 7.x native port "does not enforce" Measured on the same code, only the entity's width changing:
The second pass still earns its cost, and the real reason is sharper than the one I gave: there is a band of realistic domain widths that fails for a consumer on 5.x and passes on the version this repo builds with. That band is exactly where #31 and #32 came from. The description above is updated, and |
`Entity.union("_tag", …)` could never match anything. `_tag` is
non-enumerable, so it is absent from toJSON() and from every row, which
means discriminantValues() registered no keys and make() rejected every
payload with:
[{"path":["_tag"],"message":"Invalid discriminant undefined; expected
one of "}]
— an empty set. union.ts documents this exact prohibition; the example
did the opposite, and the spec never called make() through the union, so
nothing caught it. Raised in review on #36.
Replaced with the modelling the union is for: CreditNote joins Invoice as
a sibling document, both declaring `kind` as a generated literal, and
BillingDocument dispatches on that. Four specs now cover it — both
members round-tripping to the right class, the discriminant surviving
toJSON, and an unknown value reporting a populated "expected one of".
The trap is written down in the package README and the docs page, since
reaching for _tag is the obvious wrong move and it fails silently.
refactor: replace the consumer fixture with an examples workspace
btravers
changed the title
Contributor
Author
|
The Copilot review above predates #36 and is now stale. Flagging rather than leaving it to mislead a reader. It reviewed 9 files; this PR now carries 38. Three specific claims in it are no longer true:
The declaration-emit fixture moved to Its summary line "to catch TS7056 that TS 7.0.2 doesn't enforce" also repeats the description's original error, which I have since corrected: both versions enforce The review on #36 was the substantive one — it caught a genuine defect ( |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
examples/billing-persistence/package.json:15
- This workspace imports
@btravstack/entityvia its realexports(pointing atdist/*). Sincepackages/entity/distis generated and not checked in, runningpnpm --filter @btravstack/entity-example-billing-persistence testfrom a clean repo can fail unless@btravstack/entityhas been built first. Prepending an explicit build makes this example runnable in isolation.
"scripts": {
"test": "vitest run",
"typecheck": "tsc --noEmit"
},
examples/billing-domain/package.json:15
- The example package imports
@btravstack/entityvia its realexports(which point atdist/*). Sincedist/is not in the repo, running this workspace’stest/typecheckdirectly (e.g.pnpm --filter @btravstack/entity-example-billing-domain test) will fail unless the entity package has been built first. Prepend an explicitpnpm --filter @btravstack/entity buildso the package is runnable in isolation as the README suggests.
"scripts": {
"test": "vitest run",
"typecheck": "tsc --noEmit && tsc -p tsconfig.emit.json && node ./node_modules/typescript-consumer/bin/tsc -p tsconfig.emit.json"
},
examples/billing-api/package.json:15
- This workspace imports
@btravstack/entitythrough its published entrypoints (which resolve todist/*). Becausedist/isn’t present until the entity package is built,pnpm --filter @btravstack/entity-example-billing-api testcan fail when run from a clean checkout. Consider building@btravstack/entityas part of the script so the example package is runnable on its own.
"scripts": {
"test": "vitest run",
"typecheck": "tsc --noEmit"
},
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.
Fixes the three issues from a real adoption — #31, #32, #33 — and replaces the consumer fixture with a runnable
examples/workspace (#36, merged in).One root cause behind #31 and #32
EntityStatic— whatEntity(tag)(fields, options)returns — was not exported. TypeScript had no name to write for it at a consumer's emit site, so it serialised the entire static surface structurally into any downstream package building withdeclaration: true: the construct signature, all fourZodObjects, both zod slots, the four phantom carriers,make/extend/factory— with the field map repeated a dozen times..d.tsemitted for a one-field entityTS7056. A 30-member domain enum pushed the repeated field map past the compiler's serialisation ceiling. The workaround cost both runtime membership validation and compile-time exhaustiveness.TS4020. A branded object was expanded throughDeepReadonlyuntil zod's module-private$brandreached computed-key position, where it cannot be named across a module boundary. Branded scalars escaped becausestring & $brand<"X">hitsDeepReadonly'sImmutableshort-circuit.TS4023. An exportedconstholdingEntity.union(...)had the same problem one type further along.EntityUnionandUnionMemberare exported too.All three surfaced only at the consuming package's build, long after
tsc --noEmit, the tests and everything else went green. Branded objects now work and stay deep-readonly — the "model it as a nested entity" workaround is no longer needed.#33 — zod peer range
Widened
^4.4.0→^4.3.0. Nothing needed 4.4; the range was just the version current at the initial release. The floor is measured: on 4.3.0 the full surface typechecks, emits declarations and passes its runtime assertions.Pinning zod in the adopting package alone does not work, and there is a concrete reason — zod encodes its own minor version in the type (
_zod.version.minor), so two resolved copies fail at the boundary withTS2322.The gate that let this ship
The declaration-emit pass now runs twice, on 7.0.2 and on 5.9.3 via a
typescript-consumeralias. Both versions enforceTS7056; 5.9.3's threshold is lower:TS4020onlyTS4020+TS7056TS4020+TS7056TS4020+TS7056There is a band of realistic domain widths that fails for consumers on 5.x and passes on the version this repo builds with. That band is where #31 and #32 came from.
The examples workspace (#36)
packages/entity/consumer/proved a downstream library could build against this package, and documented nothing.examples/billing-domaindoes both, and resolves@btravstack/entitythrough its realexportsasworkspace:*rather than the oldpathsmapping. Two siblings —billing-api(the fourZodObjects → oRPC contract + JSON Schema) andbilling-persistence(toJSON()out,make()back) — put the design rule into the directory structure.No Docker, no broker, no database:
pnpm testruns all of it.It found two bugs while being written, which is the argument for it: the
TS4023union export above, and a brokenEntity.union("_tag", …)in the example itself —_tagis non-enumerable, so it is absent from every row and the union matched nothing. Both invisible to the 145-test suite, because vitest never typechecks and one of them lived only in emitted declarations.Verification
Full gate green on the merged branch, CI included, across Node 22 / 24 / 26:
format --check,lint,typecheck(6 tasks),test(167 specs),knip,build.The migration's acceptance test — remove an export, confirm the new fixture still fails:
examples/billing-domainreportsEntityStaticTS4020+TS7056EntityUnionTS4023Beyond the gate, the packed tarball was installed into a standalone project on TypeScript 5.9.3 and exercised on zod 4.3.0 and 4.4.3 — declaration emit clean for the wide-enum entity, the branded-object entity and the full documented surface, runtime assertions passing on both.
🤖 Generated with Claude Code