fix(InputNumber): keep previous value on unparseable input, emit null when cleared#6772
fix(InputNumber): keep previous value on unparseable input, emit null when cleared#677261021 wants to merge 2 commits into
Conversation
Reka UI's NumberField emits `undefined` for both a cleared input and a non-empty value that fails to parse (e.g. "." or "-" with Enter/blur), destroying the previous model value in the second case — reported as "can return NaN" since `isNaN(undefined)` is true and undefined poisons arithmetic downstream. Route all model updates through onUpdate() (the auto-forwarded update:modelValue handler bypassed it entirely in non-optional mode) and drop the update when the committed text is non-empty but unparseable: the controlled NumberFieldRoot then restores the last formatted value on its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…type The emit type is `number | null` (undefined only with the .optional modifier), but clearing the field leaked `undefined` at runtime regardless of the modifier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
commit: |
🔗 Linked issue
Resolves #6743
❓ Type of change
📚 Description
First, a root-cause correction for the issue title: no
NaNis ever emitted. Committing unparseable text like.or-(Enter/blur) makes reka-ui's NumberField emitundefined— the reporter'sisNaN(v)check reads true becauseisNaN(undefined)is true, andundefinedpoisons downstream arithmetic into NaN. Reka'sapplyInputValueconflates two situations that deserve different handling: a cleared input and non-empty unparseable text both nuke the model toundefined(and destroy the previous value).Two commits, separable:
1.
fix(InputNumber): keep previous value when committing unparseable input— what the reporter asked for ("maybe just revert to the previous value instead?"), matching react-spectrum's behavior. All model updates now flow throughonUpdate()— previously the auto-forwardedupdate:modelValuehandler bypassed it entirely in non-.optionalmode (and double-emitted in.optionalmode). When the committed value isundefinedbut the input still contains text, the update is dropped; sinceNumberFieldRootruns controlled, it restores the last formatted value on its own. Verified in the added test:.+ Enter on a model of5→ no emit, input text back to5.2.
fix(InputNumber): emit null when cleared to match the declared model type— the typing-soundness half reported by @danekslama in the issue comments: the emit type isnumber | null(undefinedonly with the.optionalmodifier), but clearing leakedundefinedat runtime regardless. Clearing now emitsnull, orundefinedwith.optional(unchanged). This one is mildly behavior-visible for consumers who relied on the untypedundefined— happy to drop this commit if you'd rather keep it wrapper-compatible, the first commit stands alone.Tests cover all three paths (unparseable revert / clear →
null/ clear +.optional→undefined); existing suites and snapshots unchanged (110 tests green,vue-tscclean).Upstream note: the revert-on-unparseable behavior arguably belongs in reka-ui's
applyInputValueeventually, but the wrapper can't wait on that and needs thenullconvention regardless — no reka-ui issue exists for it yet, I can file one.📝 Checklist