fix(set): coerce set elements without duplicating originals#1299
Open
spokodev wants to merge 1 commit into
Open
fix(set): coerce set elements without duplicating originals#1299spokodev wants to merge 1 commit into
spokodev wants to merge 1 commit into
Conversation
A set() whose element struct coerces returned a Set containing both the
original and coerced elements, e.g. create(new Set(['1','2']), set(toNumber))
yielded Set{'1','2',1,2}. The result then failed the struct's own
validation, breaking the create()/validate() guarantee that the returned
value matches the struct.
Root cause: the coercion write-back in run() uses Set.add(), which can only
insert a member and never replace one positionally, so each coerced element
accumulated alongside its original. Clear the set once on the first coerced
write-back so only coerced members remain, and snapshot the set entries via
[...value] so clearing the live Set during write-back cannot corrupt
iteration.
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
Coercing a
set()whose element struct coerces returns aSetthat contains both the original and the coerced elements, and the result fails the struct's own validation:This breaks the documented
create()/validate(..., { coerce: true })guarantee that the returned value is guaranteed to match the struct, and that coercion works recursively.array()(writes back by index) andmap()(writes back by key) already handle this correctly; onlyset()is affected.Root cause
The coercion write-back in
run()(src/utils.ts) doesvalue.add(coercedValue)for sets.Set.add()can only insert a member, it can never replace one positionally, so the coerced element is appended next to the still-present original. Arrays overwrite by index and maps overwrite by key, but a set has no positional handle, so both'1'and1survive.Fix
run(), clear the set once on the first coerced write-back so only coerced members remain, thenadd()accumulates them.set().entries(src/structs/types.ts), snapshot the members with[...value]before yielding, so clearing the liveSetduring write-back cannot corrupt the iteration.The change is scoped to the set branch and is a no-op when there is no element struct or no coercion.
Tests
Added
test/validation/set/coerce-element.ts(matches the existing fixture style):Red before the fix (
Set{'1','2',1,2}), green after. Full suite passes: 226/226.Also verified by hand: empty set,
set()with no element struct, set nested in an object, coercion collapsing two inputs to one member (dedup), per-element failure path, and non-coercing rejection.