Repository navigation
feat!: require an identity field on every aggregate - #167
Conversation
An aggregate root is what other aggregates reference and what a repository loads, so `Entity.aggregate` now rejects a field map with no `identity` field: a compile error (an inline rejection literal) plus a declaration-time defect, the redeclaration precedent. `sameIdentityAs` is therefore always available on an aggregate. `Entity` keeps identity optional, since it also models aggregate parts and value-like records. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 36 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The change is small, self-contained, consistently follows existing type-rejection and runtime-defect precedents, and is covered by both a type-level and a runtime test with no issues found.
Review effort: Balanced
Findings: None
What changed in this PR
This PR makes identity mandatory for aggregates, closing a gap where Entity.aggregate (shipped in 0.8.0) allowed an aggregate root with no identity field even though a root is precisely what other aggregates reference and what a repository loads by. The rule is enforced at two levels: a compile-time rejection (an inline __anAggregateRootNeedsAnIdentityField literal intersected onto the field-map argument, so TypeDoc gets no new internal name) and a runtime throw for callers who bypass the types. Plain Entity is intentionally left unchanged, since it also models aggregate parts and value-like records that have no identity.
Changes:
- Added a type-level identity requirement (
[IdentityKeys<S>] extends [never]) plus a runtime guard that throws, naming the tag, when no field is flaggedidentity: true. - Added a negative type-test (
@ts-expect-error) and a runtime-throw spec test. - Updated docs (declaration reference, how-to, CLAUDE.md, typedoc exclusions) and a Breaking changeset.
| File | Description |
|---|---|
| packages/entity/src/aggregate.ts | Adds the type-level rejection on the field-map argument and a runtime throw mirroring entity.ts identity detection and the base.ts defect precedent. |
| packages/entity/src/aggregate.test-d.ts | Adds a @ts-expect-error case proving an aggregate without an identity field does not compile. |
| packages/entity/src/aggregate.spec.ts | Adds a runtime test asserting the declaration throws, naming the aggregate. |
| docs/typedoc.json | Adds IdentityKeys to intentionallyNotExported to keep TypeDoc warnings at 1. |
| docs/reference/declaration.md | Documents the mandatory identity field for aggregates. |
| docs/how-to/model-an-event-driven-aggregate.md | Notes the identity requirement in the how-to guide. |
| CLAUDE.md | Records the new aggregate identity rule in the architecture notes. |
| .changeset/aggregate-requires-identity.md | Minor Breaking changeset with the migration line. |
The type-level check matches the existing sameIdentityAs typing (types.ts:318,652), the runtime guard is identical to the identity detection in entity.ts:129, and the throw follows the base.ts redeclaration precedent (tag-prefixed message with oxlint-disable unthrown/no-throw). The existing Subscription example already flags identity: true, so no example changes are needed, and the minor bump is correct for a breaking change on a 0.x package.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
Entity.aggregatenow requires at least one field flaggedidentity: true. A field map without one is a compile error (an inline__anAggregateRootNeedsAnIdentityFieldrejection on the field map, so TypeDoc gets no new internal name), and a declaration that gets past the types throws while it runs. That follows the precedent of a root's redeclared field.sameIdentityAsis therefore always available on an aggregate.Entityis unchanged: identity stays optional, because it also models aggregate parts and value-like records that have none.Entity.aggregateshipped without the rule): a minor changeset marked Breaking, with the migration line.Test plan
aggregate.test-d.ts: an aggregate without an identity field doesn't compileaggregate.spec.ts: forced past the types, the declaration throws, naming the aggregate🤖 Generated with Claude Code