fix(literal): use Object.is so literal(NaN) accepts NaN (#1529)#1535
fix(literal): use Object.is so literal(NaN) accepts NaN (#1529)#1535sanjibani wants to merge 1 commit into
Conversation
`literal(NaN)` was an unsatisfiable schema: the runtime value check used `===`, and `NaN === NaN` is `false`, so every input - including the `NaN` the schema was constructed from - failed validation. `Literal` is typed as `bigint | boolean | number | string | symbol`, and `literal(NaN)` is a fully type-checked construction. The sibling `picklist` schema compares with `Array.includes` (SameValueZero) and already returns `true` for `picklist([NaN])` against `NaN`, so the two schemas gave opposite answers for the same value. Switch the single comparison in `library/src/schemas/literal/literal.ts` from `===` to `Object.is`. `Object.is(NaN, NaN)` is `true`, so `literal(NaN)` now accepts `NaN`. This matches the equality migration already in flight: open-circle#1517 moves `_merge` (used by `intersect`) to `Object.is`, and open-circle#1477 moves `value` / `notValue` / `values` / `notValues` to `Object.is`. Neither of those touched `literal`, so this closes the same NaN gap one layer up. Added two regression tests: - `for NaN literal` in the no-issues suite: `literal(NaN)` accepts `NaN` (was failing before the fix). - `for invalid NaN literal` in the issues suite: `literal(NaN)` still rejects other numbers, booleans, strings, symbols, etc. (`NaN` is the only value that should match.) Closes open-circle#1529
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe literal schema's runtime comparison was changed from strict equality (===) to Object.is when checking whether a dataset value matches the configured literal. This resolves an issue where literal(NaN) rejected NaN due to NaN === NaN evaluating to false. Two tests were added: one confirming literal(NaN) accepts NaN without schema issues, and another confirming literal(NaN, 'message') rejects non-NaN inputs with an issue expecting 'NaN'. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Closes #1529.
Bug
`literal(NaN)` was an unsatisfiable schema: the runtime value check used `===`, and `NaN === NaN` is `false`, so every input - including the `NaN` the schema was constructed from - failed validation.
`Literal` is typed as `bigint | boolean | number | string | symbol`, and `literal(NaN)` is a fully type-checked construction. The sibling `picklist` schema compares with `Array.includes` (SameValueZero) and already returns `true` for `picklist([NaN])` against `NaN`, so the two schemas gave opposite answers for the same value.
Fix
Switch the single comparison in `library/src/schemas/literal/literal.ts` from `===` to `Object.is`. `Object.is(NaN, NaN)` is `true`, so `literal(NaN)` now accepts `NaN`.
This matches the equality migration already in flight: #1517 moves `_merge` (used by `intersect`) to `Object.is`, and #1477 moves `value` / `notValue` / `values` / `notValues` to `Object.is`. Neither of those touched `literal`, so this closes the same NaN gap one layer up.
Trade-off
Like #1517 and #1477, `Object.is` also distinguishes `+0` from `-0` (they are equal under `===` and SameValueZero). The `Literal` union is `bigint | boolean | number | string | symbol`; `-0` is a valid `number`, and `literal(-0)` and `literal(0)` would still be distinguishable as different construction values but would accept each other's runtime values. The previous `===` already collapsed `+0` and `-0` at construction time, so the new behavior is a strict relaxation of `-0` distinguishability, not a tightening of any contract.
Tests
Added two regression tests:
2 files changed, +35/-1.
Summary by CodeRabbit
NaNand signed zero.literal(NaN)now acceptsNaNcorrectly and returns no validation issues.