fix(ColorPicker): fix state updates for react 19#2646
Merged
Conversation
Contributor
Reviewer's GuideRefactors ColorPicker’s internal color/HSVA/input synchronization to be React 19–friendly by removing the internal update ref, deriving state more predictably, and debouncing color updates, while adding a debounce constant. Sequence diagram for HSVA updates with debounced color propagationsequenceDiagram
actor User
participant ColorControl
participant ColorPicker
participant DebouncedUpdate as updateColorByHsva
participant ParentComponent
User->>ColorControl: change color (drag/interaction)
ColorControl->>ColorPicker: updateHsva(updates)
activate ColorPicker
ColorPicker->>ColorPicker: setHsva(merge prevHsva, updates)
ColorPicker->>ColorPicker: isValidHsva(nextHsva) && !isSameHsva
ColorPicker->>DebouncedUpdate: schedule nextHsva
deactivate ColorPicker
Note over DebouncedUpdate: wait UPDATE_COLOR_DEBOUNCE_TIME
DebouncedUpdate->>ColorPicker: setColor(withAlpha ? hsvaToHexa : hsvaToHex)
ColorPicker->>ParentComponent: onUpdate(newColor) via useControlledState
ParentComponent-->>ColorPicker: passes updated value prop
ColorPicker->>ColorPicker: effectiveColor = color.trim() || DEFAULT_COLOR
ColorPicker->>ColorPicker: effect parses effectiveColor to hsva
Class diagram for updated ColorPicker state managementclassDiagram
class ColorPicker {
+anchor : HTMLDivElement | null
+color : string
+isOpen : boolean
+modeState : Modes
+hsva : HsvaColor
+inputValue : string
+withAlpha : boolean
+effectiveColor : string
+ColorPicker(props) React.FC
+updateHsva(updates : Partial_HsvaColor_) void
+resetInputValue() void
+applyInputValue() void
}
class ColorPickerConstants {
+DEFAULT_COLOR : string
+UPDATE_COLOR_DEBOUNCE_TIME : number
}
class HsvaColor {
+h : number
+s : number
+v : number
+a : number
}
class Modes {
}
class HooksAndUtils {
+useControlledState(value : string, defaultValue : string, onUpdate : Function) string
+parseColorToHsva(color : string, withAlpha : boolean) ParsedColor
+hsvaToHex(hsva : HsvaColor) string
+hsvaToHexa(hsva : HsvaColor) string
+getTextValueByMode(hsva : HsvaColor, mode : Modes, withAlpha : boolean) string
+isSameHsva(a : HsvaColor, b : HsvaColor) boolean
+isValidHsva(hsva : HsvaColor) boolean
+debounce(fn : Function, delay : number) Function
}
class ParsedColor {
+hsva : HsvaColor
+isValid : boolean
}
class UpdateColorByHsva {
+updateColorByHsva(nextHsva : HsvaColor) void
}
ColorPickerConstants <.. ColorPicker : uses
HooksAndUtils <.. ColorPicker : uses
ParsedColor <.. HooksAndUtils : returns
HsvaColor <.. ParsedColor : contains
HsvaColor <.. ColorPicker : state
Modes <.. ColorPicker : state
UpdateColorByHsva <.. ColorPicker : has
Flow diagram for ColorPicker color, HSVA, and input synchronizationflowchart TD
A[value prop or defaultValue] --> B["derive color via useControlledState"]
B --> C["effectiveColor = color.trim() || DEFAULT_COLOR"]
C --> D["effect: parseColorToHsva(effectiveColor, withAlpha)"]
D -->|valid| E["setHsva(parsed.hsva) if different by color"]
E --> F["effect: setInputValue(getTextValueByMode(hsva, modeState, withAlpha))"]
subgraph UserInput
G["user edits HexInput"] --> H["setInputValue(new text)"]
H --> I["onBlur applyInputValue"]
I --> J["parseColorToHsva(inputValue, withAlpha)"]
J -->|valid| K["setHsva(normalized.hsva)"]
K --> L["setColor(hsvaToHex or hsvaToHexa)"]
K --> F
end
subgraph DragControls
M["user drags picker controls"] --> N["updateHsva(partial hsva)"]
N --> O["setHsva(merged hsva) if valid and changed"]
O --> P["debounced updateColorByHsva(hsva)"]
P --> L
O --> F
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
effectiveColorinitialization now callscolor.trim()directly, which will throw ifcolorisundefinedornull; consider restoring the defensivecolor?.trim() || DEFAULT_COLORpattern or equivalent guarding. - The
modeStatestate variable is declared after it is first used in theinputValueuseStateinitializer (getTextValueByMode(hsva, modeState, withAlpha)), which will cause a runtime reference error due to the temporal dead zone; move themodeStatedeclaration above any usage. - The new
hsvasync effect comparesprevColor(derived fromprevHsva) toeffectiveColorvia strict string equality, which may fail for equivalent colors with different string representations (e.g., case differences or normalized alpha); consider normalizing both sides or comparing via parsed HSVA instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `effectiveColor` initialization now calls `color.trim()` directly, which will throw if `color` is `undefined` or `null`; consider restoring the defensive `color?.trim() || DEFAULT_COLOR` pattern or equivalent guarding.
- The `modeState` state variable is declared after it is first used in the `inputValue` `useState` initializer (`getTextValueByMode(hsva, modeState, withAlpha)`), which will cause a runtime reference error due to the temporal dead zone; move the `modeState` declaration above any usage.
- The new `hsva` sync effect compares `prevColor` (derived from `prevHsva`) to `effectiveColor` via strict string equality, which may fail for equivalent colors with different string representations (e.g., case differences or normalized alpha); consider normalizing both sides or comparing via parsed HSVA instead.
## Individual Comments
### Comment 1
<location path="src/components/lab/ColorPicker/ColorPicker.tsx" line_range="133-134" />
<code_context>
- setColor(nextHexValue);
- }
- }, [hsva, withAlpha, color, setColor]);
+ const updateColorByHsva = React.useMemo(() => {
+ return debounce((nextHsva: HsvaColor) => {
+ setColor(withAlpha ? hsvaToHexa(nextHsva) : hsvaToHex(nextHsva));
+ }, UPDATE_COLOR_DEBOUNCE_TIME);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The debounced `updateColorByHsva` function may outlive the component lifecycle and call `setColor` after unmount.
Unless this `debounce` implementation cancels pending calls on unmount, delayed invocations can still call `setColor` after unmount or after `withAlpha` has changed. Consider canceling the debounced function in a cleanup effect, or using a debounce helper that does this automatically, to avoid state updates on unmounted components and race conditions when `withAlpha` toggles.
Suggested implementation:
```typescript
const updateColorByHsva = React.useMemo(() => {
return debounce((nextHsva: HsvaColor) => {
setColor(withAlpha ? hsvaToHexa(nextHsva) : hsvaToHex(nextHsva));
}, UPDATE_COLOR_DEBOUNCE_TIME);
}, [setColor, withAlpha]);
React.useEffect(() => {
// Ensure no pending debounced calls fire after unmount or when the debounced
// instance is replaced (e.g. when `withAlpha` changes).
return () => {
if (typeof (updateColorByHsva as any)?.cancel === 'function') {
(updateColorByHsva as any).cancel();
}
};
}, [updateColorByHsva]);
const updateHsva = React.useCallback(
```
If your `debounce` helper exposes a different API for canceling (e.g. `clear` instead of `cancel`), update the cleanup accordingly:
- Replace `.cancel()` with the appropriate method name.
- If the debounced function type is already declared somewhere in the codebase, you can replace the `any` cast with that proper type for stronger typing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+133
to
+134
| const updateColorByHsva = React.useMemo(() => { | ||
| return debounce((nextHsva: HsvaColor) => { |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): The debounced updateColorByHsva function may outlive the component lifecycle and call setColor after unmount.
Unless this debounce implementation cancels pending calls on unmount, delayed invocations can still call setColor after unmount or after withAlpha has changed. Consider canceling the debounced function in a cleanup effect, or using a debounce helper that does this automatically, to avoid state updates on unmounted components and race conditions when withAlpha toggles.
Suggested implementation:
const updateColorByHsva = React.useMemo(() => {
return debounce((nextHsva: HsvaColor) => {
setColor(withAlpha ? hsvaToHexa(nextHsva) : hsvaToHex(nextHsva));
}, UPDATE_COLOR_DEBOUNCE_TIME);
}, [setColor, withAlpha]);
React.useEffect(() => {
// Ensure no pending debounced calls fire after unmount or when the debounced
// instance is replaced (e.g. when `withAlpha` changes).
return () => {
if (typeof (updateColorByHsva as any)?.cancel === 'function') {
(updateColorByHsva as any).cancel();
}
};
}, [updateColorByHsva]);
const updateHsva = React.useCallback(If your debounce helper exposes a different API for canceling (e.g. clear instead of cancel), update the cleanup accordingly:
- Replace
.cancel()with the appropriate method name. - If the debounced function type is already declared somewhere in the codebase, you can replace the
anycast with that proper type for stronger typing.
Contributor
|
Preview is ready. |
Contributor
|
🎭 Component Tests Report is ready. |
Estasie
approved these changes
Apr 9, 2026
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
2 participants
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.
Summary by Sourcery
Adjust ColorPicker state synchronization to be compatible with React 19 and reduce redundant updates.
Enhancements: