Skip to content

Conversation

@shaungrady
Copy link

Fix cyclic union discrimination

Fixes #1547

Problem

Discriminated unions containing circular references were completely excluded from discrimination, causing validation to check all branches and produce confusing error messages referencing irrelevant branches.

Solution

Allow cyclic unions to use property-based discrimination (e.g., a type discriminator field) while blocking structural discrimination that would attempt to describe the entire recursive structure.

The fix filters discrimination candidates to only include those with path.length > 0, ensuring cyclic unions discriminate on specific properties rather than overall object structure.

Changes

  • Remove early bailout for cyclic unions in discriminate()
  • Filter candidates to exclude structural discrimination (path.length === 0) for cyclic unions
  • Add test validating discrimination works and error messages only reference the relevant branch

Example

const s = scope({
  AChild: { type: "'AChild'", children: "(AParent)[] > 0" },
  AParent: { type: "'AParent'", children: "(AChild)[] > 0" },
  BChild: { type: "'BChild'", children: "unknown[]" },
  BParent: { type: "'BParent'", layout: "number[]", children: "(BChild)[] > 0" }
})

const Thing = s.type("AParent | BParent")

Thing({ type: "BParent", layout: "", children: [...] })
// Before: "children[1].type must be 'AChild' (was 'BChild') or layout must be an array (was string)"
// After:  "layout must be an array (was string)"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To do

Development

Successfully merging this pull request may close these issues.

Discriminated union with circular references reports errors from wrong branch

1 participant