fix: surface controlled-value problems instead of silently degrading (M7)#25
Merged
Conversation
…(M7)
CalendarProvider had two controlled-component footguns:
1. A controlled `value` with the wrong shape for the selection mode
(range: not a { start, end }; multiple: not an array) fell through to
`undefined` and the component silently became uncontrolled — the prop
looked ignored. Now it warns: console.warn("[DatePicker] …").
2. An out-of-bounds controlled `value` fired onValueChange(null) on mount —
a controlled component spontaneously clearing the parent's value (and a
render-loop risk). When controlled it now warns and leaves the value
alone (the parent owns it); the uncontrolled auto-clear is unchanged.
Both warnings are ref-guarded to fire once per episode, not every render
(the effects re-run whenever the parent inlines `value` / `onValueChange`).
Tests: malformed range/multiple warn once; out-of-bounds controlled warns
once across re-renders and never fires onValueChange; out-of-bounds
uncontrolled still clears + fires (regression guard).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ Deploy Preview for colander-cal canceled.
|
Contributor
|
🎉 This PR is included in version 3.1.0-alpha.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
1 participant
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.
Audit item M7.
CalendarProviderhad two controlled-component footguns that failed silently.1. Malformed
valuesilently went uncontrolledIn range/multiple mode, a non-null
valuethat wasn't the right shape (range: not a{ start, end }; multiple: not an array) fell through toundefined→controlledDates === undefined→isControlled === false. The component quietly self-managed, so thevalueprop looked ignored.→ Now emits
console.warn("[DatePicker] …")(graceful uncontrolled fallback kept, but loud).2. Out-of-bounds controlled
valuefiredonValueChange(null)on mountThe single-mode cleanup effect cleared any committed date outside
min/maxviaonValueChange(null)— even when controlled, so a controlled component spontaneously told the parent to clear on mount (controlled-semantics violation + render-loop risk).→ When controlled, it now warns and leaves the value alone (the parent owns it). Uncontrolled auto-clear is unchanged.
Warn-once (from adversarial review)
A review pass flagged that both warnings would re-fire every render when the parent inlines
value/onValueChange(the effects' deps change identity). Both are now ref-guarded to warn once per episode.Tests (TDD, RED→GREEN)
onValueChange🤖 Generated with Claude Code