feat: add t.variant for union dispatch when members share a typename - #49
feat: add t.variant for union dispatch when members share a typename#49jimmy-phantom wants to merge 4 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jimmy-phantom
left a comment
There was a problem hiding this comment.
🤖 Automated review (Claude Code)
The mechanism holds up well on the primary paths: I verified parse dispatch, the definition-time guards, the registry enum merge, and per-variant live routing all behave as the PR and changeset describe, and the full unit suite passes. The inline comments cover one real bug in union composition (shared VariantGroup mutation), one routing inconsistency on the delete path, and one unreconciled runtime case (variant flips), each reproduced with a focused test.
Two smaller gaps live in code the diff does not touch:
packages/fetchium/src/testing/auto-generate.ts:generateUniononly picks shape entries that areinstanceof ValidatorDef, soVariantGroupentries are skipped and a variant-union field generates{}(verified), which cannot parse when the field is required.buildConstraintFieldRefsinpackages/fetchium/src/typeDefs.tskeys per-class constraints by typename, soconstraints: [[TextPost, {...}], [ThreadPost, {...}]]silently keeps only the last entry when the classes share a typename. Variant members are the first supported way to put two classes with one typename in the same liveArray, which makes this reachable.
…tion, generator support
Review findings on the t.variant PR:
- Nested-union merging copied VariantGroup entries by reference, so composing
a variant union into a larger union mutated the inner union, and merging two
variant unions threw. Group handling is now centralized in
addVariantDefToUnion, which only creates groups owned by the union being
built, so aliasing is impossible by construction and group-into-group
merging gets the same duplicate checks.
- Delete events resolved to the first def per typename instead of the def the
entity satisfies, handing onDelete a different class than onCreate received.
- generateUnion produced {} for variant-union fields.
- Docs: the variant is fixed for the lifetime of an entity; a mutable tag is a
state field, not a variant.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
From the review summary: |
jimmy-phantom
left a comment
There was a problem hiding this comment.
🤖 Automated review (Claude Code)
Re-review of the fix round: the centralized addVariantDefToUnion merge, parse dispatch, delete-path resolution, and generator changes all check out. One new verified defect in live collection event routing, detailed inline.
satisfiesDef was presence-based, so a variant tag's value never participated in event gating: a single-def binding accepted any sibling variant with the same field profile, and the multi-def fallback matched variants the binding never declared. A VariantSet field now satisfies only on a value match, which also makes entity-array reads variant-aware. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jimmy-phantom
left a comment
There was a problem hiding this comment.
🤖 Automated review (Claude Code)
Follow-up pass over the round-3 changes and their consumers. Two new issues, both verified with focused repros: delete events bypass the new variant gating (corrupts t.liveValue reducer state), and the dev-mode merged-def compatibility check rejects structurally identical variant union fields.
…ructurally Two follow-ups to the variant gating work: - Deletes bypassed the gate added for creates and updates: resolveEventDef short-circuited a single variant def without checking the tag, and the delete branch delivered defs[0] unconditionally, so liveValue onDelete reducers fired for sibling variants. Deletes now skip when the entity's tag names a variant the binding does not declare; id-only deletes still route. - fieldTypesCompatible had no VariantGroup branch, and since every union owns fresh groups, two structurally identical variant-union fields compared false and ValidatorDef.merge threw in dev on valid responses. Groups now compare structurally: same variant field, same keys, compatible members. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
The typename does two jobs: entity identity (the
[typename, id]cache key) and union dispatch. When two union members share a typename, dispatch silently collapses to the last member:Change
t.variant(value)marks the tag field. Identity stays on the typename; dispatch uses the variant:t.liveArraydispatch on(typename, variant)t.variantvalidates liket.constand is not part of the cache keyBehavior changes (hence minor)
(typename, variant)pair, mixed variant and non-variant members, and conflicting variant fields.t.liveValueover same-typename defs without variants routes events to the first def the entity satisfies, instead of always the last def.Verification
variant-unions.test.ts: definition guards, parse dispatch, single-variant entity arrays, live membership per variant, liveValue multi-def routingtsc --noEmitclean; production build smoke-tested (variant dispatch works, duplicate guard throws)🤖 Generated with Claude Code