fix: surface union branch errors instead of discarding them - #76
Open
1222hxy wants to merge 1 commit into
Open
Conversation
The union resolver collected each branch's ValidationError into `messages` and then threw without ever reading it, so a failing discriminated union reported every branch's full signature but never the offending property. Only branches that failed at a deeper path are reported: a branch that failed at the union's own path merely restates the mismatch the top-level message already carries, so primitive unions keep their current message. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Problem
The
unionresolver collects each branch'sValidationErrorintomessages, then throws without ever reading it:schemastery/packages/core/src/index.ts
Lines 704 to 714 in cf0b7e5
messagesis write-only. Linters do not flag it becausepushcounts as a use.For a discriminated union this discards exactly the information the caller needs. Given:
Before — every branch's full signature, but not the one thing that is wrong:
After:
Both detail lines were already computed one line earlier and thrown away.
Why only branches that failed deeper
A branch that failed at the union's own path only restates the mismatch the top-level line already reports, so reporting it is pure noise. A branch that failed at a deeper path names the offending property, which the caller cannot recover otherwise.
Filtering on path depth keeps the change free for the cases that gain nothing:
Absolute paths are preserved through nesting, so the detail stays actionable inside objects and arrays:
Duplicate messages are collapsed.
Scope
Only the message of an already-thrown error changes. Successful validation, the set of inputs that throw, and the error type are all untouched.
Tests
Added
error detailtopackages/core/tests/union.spec.tscovering the surfaced property errors, absolute paths under nesting, and the unchanged primitive-union message.yarn yakumo test coregoes from 45 to 46 passing with no new failures.Note on CI
mainis currently red independently of this change, so a red check here is expected:packages/core/tests/i18n.spec.tsfails onmain(it outlived cf0b7e5 "refa: remove i18n dependency").testscript passes-r esbuild-register, which currentyakumorejects withunknown option: "r".I left both alone to keep this PR to one concern, but happy to fix either here or separately if useful.