Skip to content

Commit 200266a

Browse files
committed
fix(select): prevent spurious onValueChange on controlled value updates
When a controlled Select receives a new value prop (e.g., from form.reset()), onValueChange was incorrectly called with an empty string, overwriting the valid controlled value. This fix guards the internal onChange handler to skip empty value updates when a valid controlled value exists, ensuring onValueChange only fires for genuine user interactions or form autofill. Fixes #3135, #3249, #3693
1 parent 2bab24a commit 200266a

File tree

3 files changed

+101
-1
lines changed

3 files changed

+101
-1
lines changed
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@radix-ui/react-select": patch
3+
---
4+
5+
fix(select): prevent spurious onValueChange calls with empty string in forms
6+
7+
When a controlled Select component inside a form receives an external value
8+
update (e.g., from form.reset()), onValueChange was incorrectly being called
9+
with an empty string, which would overwrite the valid controlled value.
10+
11+
This fix guards the internal onChange handler to skip empty value updates when
12+
a valid controlled value already exists, ensuring onValueChange only fires for
13+
genuine user interactions or form autofill scenarios.
14+
15+
Fixes #3135, #3249, #3693

apps/storybook/stories/select.stories.tsx

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,84 @@ export const Cypress = () => {
923923
);
924924
};
925925

926+
/**
927+
* This story demonstrates the fix for #3135: Controlled Select in a form
928+
* should not call onValueChange with empty string when value is updated externally.
929+
*
930+
* Steps to verify:
931+
* 1. Click "Set to Apple" - value should change to "apple" and onValueChange count stays at 1
932+
* 2. Before fix: onValueChange would be called twice (once with "apple", once with "")
933+
* 3. After fix: onValueChange is only called once with "apple"
934+
*/
935+
export const ControlledInFormExternalUpdate = () => {
936+
const [value, setValue] = React.useState<string>('');
937+
const [changeCount, setChangeCount] = React.useState(0);
938+
const [lastValue, setLastValue] = React.useState<string>('');
939+
940+
const handleValueChange = (newValue: string) => {
941+
setChangeCount((c) => c + 1);
942+
setLastValue(newValue);
943+
setValue(newValue);
944+
};
945+
946+
return (
947+
<form style={{ padding: 50 }}>
948+
<div style={{ marginBottom: 20 }}>
949+
<strong>Test for #3135 fix:</strong>
950+
<p>onValueChange call count: {changeCount}</p>
951+
<p>Last onValueChange value: "{lastValue}"</p>
952+
<p>Current value: "{value}"</p>
953+
</div>
954+
955+
<div style={{ marginBottom: 20 }}>
956+
<button type="button" onClick={() => setValue('apple')} style={{ marginRight: 10 }}>
957+
Set to Apple (external update)
958+
</button>
959+
<button type="button" onClick={() => setValue('banana')} style={{ marginRight: 10 }}>
960+
Set to Banana (external update)
961+
</button>
962+
<button type="button" onClick={() => setValue('')}>
963+
Clear value
964+
</button>
965+
</div>
966+
967+
<Label>
968+
Select a fruit:
969+
<Select.Root value={value} onValueChange={handleValueChange}>
970+
<Select.Trigger className={styles.trigger}>
971+
<Select.Value placeholder="Pick a fruit" />
972+
<Select.Icon />
973+
</Select.Trigger>
974+
<Select.Portal>
975+
<Select.Content className={styles.content}>
976+
<Select.Viewport className={styles.viewport}>
977+
<Select.Item className={styles.item} value="apple">
978+
<Select.ItemText>Apple</Select.ItemText>
979+
<Select.ItemIndicator className={styles.indicator}>
980+
<TickIcon />
981+
</Select.ItemIndicator>
982+
</Select.Item>
983+
<Select.Item className={styles.item} value="banana">
984+
<Select.ItemText>Banana</Select.ItemText>
985+
<Select.ItemIndicator className={styles.indicator}>
986+
<TickIcon />
987+
</Select.ItemIndicator>
988+
</Select.Item>
989+
<Select.Item className={styles.item} value="orange">
990+
<Select.ItemText>Orange</Select.ItemText>
991+
<Select.ItemIndicator className={styles.indicator}>
992+
<TickIcon />
993+
</Select.ItemIndicator>
994+
</Select.Item>
995+
</Select.Viewport>
996+
</Select.Content>
997+
</Select.Portal>
998+
</Select.Root>
999+
</Label>
1000+
</form>
1001+
);
1002+
};
1003+
9261004
type PaddedElement = 'content' | 'viewport';
9271005

9281006
interface ChromaticSelectProps extends React.ComponentProps<typeof Select.Trigger> {

packages/react/select/src/select.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,14 @@ const Select: React.FC<SelectProps> = (props: ScopedProps<SelectProps>) => {
225225
autoComplete={autoComplete}
226226
value={value}
227227
// enable form autofill
228-
onChange={(event) => setValue(event.target.value)}
228+
onChange={(event) => {
229+
const newValue = event.target.value;
230+
// Prevent spurious empty string updates from internal sync events
231+
// when controlled value exists (fixes #3135)
232+
if (newValue !== '' || value === '' || value === undefined) {
233+
setValue(newValue);
234+
}
235+
}}
229236
disabled={disabled}
230237
form={form}
231238
>

0 commit comments

Comments
 (0)