Skip to content

Commit 15b90a4

Browse files
adamalstonadamalston
authored
refactor: rewrite useControllableState in typescript (#18961)
1 parent 34532d8 commit 15b90a4

File tree

6 files changed

+288
-153
lines changed

6 files changed

+288
-153
lines changed

‎packages/react/src/components/Menu/MenuItem.tsx‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import cx from 'classnames';
99
import PropTypes from 'prop-types';
1010
import React, {
11-
ChangeEventHandler,
1211
ComponentProps,
1312
FC,
1413
ForwardedRef,
@@ -338,7 +337,7 @@ export interface MenuItemSelectableProps
338337
/**
339338
* Provide an optional function to be called when the selection state changes.
340339
*/
341-
onChange?: ChangeEventHandler<HTMLLIElement>;
340+
onChange?: (checked: boolean) => void;
342341

343342
/**
344343
* Controls the state of this option.
@@ -500,7 +499,7 @@ export interface MenuItemRadioGroupProps<Item>
500499
/**
501500
* Provide an optional function to be called when the selection changes.
502501
*/
503-
onChange?: ChangeEventHandler<HTMLLIElement>;
502+
onChange?: (selectedItem: Item) => void;
504503

505504
/**
506505
* Provide props.selectedItem to control the state of this radio group. Must match the type of props.items.
@@ -527,7 +526,7 @@ export const MenuItemRadioGroup = forwardRef(function MenuItemRadioGroup<Item>(
527526
const [selection, setSelection] = useControllableState({
528527
value: selectedItem,
529528
onChange,
530-
defaultValue: defaultSelectedItem,
529+
defaultValue: defaultSelectedItem ?? ({} as Item),
531530
});
532531

533532
function handleClick(item, e) {

‎packages/react/src/components/TreeView/TreeNode.tsx‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ import { CaretDown } from '@carbon/icons-react';
99
import classNames from 'classnames';
1010
import PropTypes from 'prop-types';
1111
import React, {
12-
ComponentType,
13-
FunctionComponent,
1412
useEffect,
1513
useRef,
1614
useState,
17-
MutableRefObject,
15+
type ComponentType,
16+
type FunctionComponent,
17+
type MouseEvent,
18+
type MutableRefObject,
1819
} from 'react';
1920
import { keys, match, matches } from '../../internal/keyboard';
2021
import { useControllableState } from '../../internal/useControllableState';
@@ -133,10 +134,17 @@ const TreeNode = React.forwardRef<HTMLElement, TreeNodeProps>(
133134

134135
const controllableExpandedState = useControllableState({
135136
value: isExpanded,
136-
onChange: onToggle,
137-
defaultValue: defaultIsExpanded,
137+
onChange: (newValue: boolean) => {
138+
onToggle?.(undefined as unknown as MouseEvent, {
139+
id,
140+
isExpanded: newValue,
141+
label,
142+
value,
143+
});
144+
},
145+
defaultValue: defaultIsExpanded ?? false,
138146
});
139-
const uncontrollableExpandedState = useState(isExpanded);
147+
const uncontrollableExpandedState = useState(isExpanded ?? false);
140148
const [expanded, setExpanded] = enableTreeviewControllable
141149
? controllableExpandedState
142150
: uncontrollableExpandedState;
@@ -328,7 +336,7 @@ const TreeNode = React.forwardRef<HTMLElement, TreeNodeProps>(
328336

329337
if (!enableTreeviewControllable) {
330338
// sync props and state
331-
setExpanded(isExpanded);
339+
setExpanded(isExpanded ?? false);
332340
}
333341
}, [
334342
children,

‎packages/react/src/components/TreeView/TreeView.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
/**
2-
* Copyright IBM Corp. 2016, 2023
2+
* Copyright IBM Corp. 2016, 2025
33
*
44
* This source code is licensed under the Apache-2.0 license found in the
55
* LICENSE file in the root directory of this source tree.
66
*/
77

88
import classNames from 'classnames';
99
import PropTypes from 'prop-types';
10-
import React, { useEffect, useRef, useState } from 'react';
10+
import React, { useEffect, useRef, useState, type SyntheticEvent } from 'react';
1111
import { keys, match, matches } from '../../internal/keyboard';
1212
import { useControllableState } from '../../internal/useControllableState';
1313
import { usePrefix } from '../../internal/usePrefix';
@@ -101,7 +101,11 @@ const TreeView: TreeViewComponent = ({
101101

102102
const controllableSelectionState = useControllableState({
103103
value: preselected,
104-
onChange: onSelect,
104+
onChange: (newSelected) => {
105+
onSelect?.(undefined as unknown as SyntheticEvent<HTMLUListElement>, {
106+
activeNodeId: newSelected[0],
107+
});
108+
},
105109
defaultValue: [],
106110
});
107111
const uncontrollableSelectionState = useState(preselected ?? []);

‎packages/react/src/internal/__tests__/useControllableState-test.js‎

Lines changed: 159 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,127 @@
11
/**
2-
* Copyright IBM Corp. 2016, 2023
2+
* Copyright IBM Corp. 2016, 2025
33
*
44
* This source code is licensed under the Apache-2.0 license found in the
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import { render, screen } from '@testing-library/react';
8+
import { fireEvent, render, screen } from '@testing-library/react';
99
import userEvent from '@testing-library/user-event';
1010
import React, { useState } from 'react';
1111
import { useControllableState } from '../useControllableState';
1212

13+
const TextInput = ({ onChange, value: controlledValue }) => {
14+
const [value, setValue] = useControllableState({
15+
value: controlledValue,
16+
defaultValue: '',
17+
onChange,
18+
});
19+
20+
return (
21+
<input
22+
data-testid="input"
23+
type="text"
24+
onChange={(event) => {
25+
setValue(event.target.value);
26+
}}
27+
value={value}
28+
/>
29+
);
30+
};
31+
32+
const ControlledTextInput = () => {
33+
const [value, setValue] = useState('');
34+
35+
return <TextInput value={value} onChange={setValue} />;
36+
};
37+
38+
const Toggle = ({ defaultControlled }) => {
39+
const [value, setValue] = useState('');
40+
const [controlled, setControlled] = useState(defaultControlled);
41+
42+
return (
43+
<>
44+
<TextInput
45+
value={controlled ? value : undefined}
46+
onChange={controlled ? setValue : undefined}
47+
/>
48+
<button
49+
data-testid="toggle"
50+
type="button"
51+
onClick={() => {
52+
setControlled(!controlled);
53+
}}>
54+
toggle
55+
</button>
56+
</>
57+
);
58+
};
59+
60+
const FunctionalInput = () => {
61+
const [value, setValue] = useControllableState({ defaultValue: 0 });
62+
63+
return (
64+
<>
65+
<div data-testid="display">{value}</div>
66+
<button
67+
data-testid="increment"
68+
onClick={() => {
69+
setValue((prev) => prev + 1);
70+
}}>
71+
Increment
72+
</button>
73+
</>
74+
);
75+
};
76+
77+
const UncontrolledInput = ({ onChange }) => {
78+
const [value, setValue] = useControllableState({
79+
defaultValue: '',
80+
onChange,
81+
});
82+
83+
return (
84+
<input
85+
data-testid="input"
86+
type="text"
87+
onChange={(event) => {
88+
setValue(event.target.value);
89+
}}
90+
value={value}
91+
/>
92+
);
93+
};
94+
95+
const DefaultValueInput = ({ defaultValue }) => {
96+
const [value, setValue] = useControllableState({ defaultValue });
97+
98+
return (
99+
<input
100+
data-testid="input"
101+
type="text"
102+
onChange={(event) => {
103+
setValue(event.target.value);
104+
}}
105+
value={value}
106+
/>
107+
);
108+
};
109+
110+
const NoOnChangeInput = () => {
111+
const [value, setValue] = useControllableState({ defaultValue: '' });
112+
113+
return (
114+
<input
115+
data-testid="input"
116+
type="text"
117+
onChange={(event) => {
118+
setValue(event.target.value);
119+
}}
120+
value={value}
121+
/>
122+
);
123+
};
124+
13125
describe('useControllableState', () => {
14126
test('uncontrolled', async () => {
15127
render(<TextInput />);
@@ -47,51 +159,54 @@ describe('useControllableState', () => {
47159

48160
warn.mockRestore();
49161
});
50-
});
51162

52-
function TextInput({ onChange, value: controlledValue }) {
53-
const [value, setValue] = useControllableState({
54-
value: controlledValue,
55-
defaultValue: '',
56-
onChange,
163+
test('should handle functional updater correctly', async () => {
164+
render(<FunctionalInput />);
165+
166+
expect(screen.getByTestId('display').textContent).toBe('0');
167+
await userEvent.click(screen.getByTestId('increment'));
168+
expect(screen.getByTestId('display').textContent).toBe('1');
169+
await userEvent.click(screen.getByTestId('increment'));
170+
expect(screen.getByTestId('display').textContent).toBe('2');
57171
});
58172

59-
function handleOnChange(event) {
60-
setValue(event.target.value);
61-
}
173+
test('should call `onChange` with cumulative values for each character typed (uncontrolled)', async () => {
174+
const onChangeMock = jest.fn();
175+
const text = '🟦 ⬜ ⬛ 👁️ 🐝 Ⓜ️ 🟦 ⬜ ⬛';
62176

63-
return (
64-
<input
65-
data-testid="input"
66-
type="text"
67-
onChange={handleOnChange}
68-
value={value}
69-
/>
70-
);
71-
}
177+
render(<UncontrolledInput onChange={onChangeMock} />);
72178

73-
function ControlledTextInput() {
74-
const [value, setValue] = useState('');
75-
return <TextInput value={value} onChange={setValue} />;
76-
}
179+
const input = screen.getByTestId('input');
77180

78-
function Toggle({ defaultControlled }) {
79-
const [value, setValue] = useState('');
80-
const [controlled, setControlled] = useState(defaultControlled);
81-
return (
82-
<>
83-
<TextInput
84-
value={controlled ? value : undefined}
85-
onChange={controlled ? setValue : undefined}
86-
/>
87-
<button
88-
data-testid="toggle"
89-
type="button"
90-
onClick={() => {
91-
setControlled(!controlled);
92-
}}>
93-
toggle
94-
</button>
95-
</>
96-
);
97-
}
181+
await userEvent.type(input, text);
182+
expect(onChangeMock).toHaveBeenCalledTimes(text.length);
183+
184+
[...text].forEach((_, i) => {
185+
const expected = text.slice(0, i + 1);
186+
187+
expect(onChangeMock.mock.calls[i][0]).toBe(expected);
188+
});
189+
});
190+
191+
test('should maintain `defaultValue` after re-render (uncontrolled)', () => {
192+
const { rerender } = render(<DefaultValueInput defaultValue="initial" />);
193+
const input = screen.getByTestId('input');
194+
195+
expect(input.value).toBe('initial');
196+
197+
fireEvent.change(input, { target: { value: 'changed' } });
198+
expect(input.value).toBe('changed');
199+
200+
rerender(<DefaultValueInput defaultValue="initial" />);
201+
expect(input.value).toBe('changed');
202+
});
203+
204+
test('should work without `onChange` callback', async () => {
205+
render(<NoOnChangeInput />);
206+
207+
const input = screen.getByTestId('input');
208+
209+
await userEvent.type(input, 'test');
210+
expect(input.value).toBe('test');
211+
});
212+
});

0 commit comments

Comments
 (0)