Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 664a86f

Browse files
committed
chore(usecontrolledoruncontrolled): emulate React's uncontrolled warn if value is set without onChange or readOnly
1 parent 7c618cb commit 664a86f

File tree

7 files changed

+83
-32
lines changed

7 files changed

+83
-32
lines changed

‎src/Checkbox/Checkbox.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,10 @@ const Checkbox = forwardRef<HTMLInputElement, CheckboxProps>(
207207
ref
208208
) => {
209209
const [state, setState] = useControlledOrUncontrolled({
210-
value: checked,
211-
defaultValue: defaultChecked
210+
defaultValue: defaultChecked,
211+
onChange,
212+
readOnly: otherProps.readOnly ?? disabled,
213+
value: checked
212214
});
213215

214216
const handleChange = useCallback(

‎src/ColorInput/ColorInput.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@ const ColorInput = forwardRef<HTMLInputElement, ColorInputProps>(
128128
ref
129129
) => {
130130
const [valueDerived, setValueState] = useControlledOrUncontrolled({
131-
value,
132-
defaultValue
131+
defaultValue,
132+
onChange,
133+
readOnly: otherProps.readOnly ?? disabled,
134+
value
133135
});
134136

135137
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {

‎src/NumberInput/NumberInput.tsx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ type NumberInputProps = {
1414
disabled?: boolean;
1515
max?: number;
1616
min?: number;
17+
readOnly?: boolean;
1718
step?: number;
1819
onChange?: (newValue: number) => void;
1920
style?: React.CSSProperties;
@@ -105,6 +106,7 @@ const NumberInput = forwardRef<HTMLInputElement, NumberInputProps>(
105106
max,
106107
min,
107108
onChange,
109+
readOnly,
108110
step = 1,
109111
style,
110112
value,
@@ -114,8 +116,10 @@ const NumberInput = forwardRef<HTMLInputElement, NumberInputProps>(
114116
ref
115117
) => {
116118
const [valueDerived, setValueState] = useControlledOrUncontrolled({
117-
value,
118-
defaultValue
119+
defaultValue,
120+
onChange,
121+
readOnly,
122+
value
119123
});
120124

121125
const handleInputChange = useCallback(
@@ -169,6 +173,7 @@ const NumberInput = forwardRef<HTMLInputElement, NumberInputProps>(
169173
onChange={handleInputChange}
170174
disabled={disabled}
171175
type='number'
176+
readOnly={readOnly}
172177
ref={ref}
173178
fullWidth
174179
onBlur={onBlur}
@@ -177,15 +182,15 @@ const NumberInput = forwardRef<HTMLInputElement, NumberInputProps>(
177182
<StyledButton
178183
data-testid='increment'
179184
variant={variant}
180-
disabled={disabled}
185+
disabled={disabled||readOnly}
181186
onClick={stepUp}
182187
>
183188
<StyledButtonIcon invert />
184189
</StyledButton>
185190
<StyledButton
186191
data-testid='decrement'
187192
variant={variant}
188-
disabled={disabled}
193+
disabled={disabled||readOnly}
189194
onClick={stepDown}
190195
>
191196
<StyledButtonIcon />

‎src/Slider/Slider.spec.tsx‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,20 @@ function createTouches(
1414
}
1515

1616
describe('<Slider />', () => {
17+
beforeAll(() => {
18+
jest
19+
.spyOn(HTMLElement.prototype, 'getBoundingClientRect')
20+
.mockImplementation(
21+
() =>
22+
({
23+
width: 100,
24+
height: 20,
25+
bottom: 20,
26+
left: 0
27+
} as DOMRect)
28+
);
29+
});
30+
1731
it('should call handlers', () => {
1832
const handleChange = jest.fn();
1933
const handleChangeCommitted = jest.fn();
@@ -76,7 +90,7 @@ describe('<Slider />', () => {
7690
});
7791

7892
it('defaults to horizontal orientation', () => {
79-
const { getByRole } = renderWithTheme(<Slider value={0} />);
93+
const { getByRole } = renderWithTheme(<Slider defaultValue={0} />);
8094

8195
expect(getByRole('slider')).toHaveAttribute(
8296
'aria-orientation',
@@ -86,7 +100,7 @@ describe('<Slider />', () => {
86100
it('should forward mouseDown', () => {
87101
const handleMouseDown = jest.fn();
88102
const { container } = renderWithTheme(
89-
<Slider onMouseDown={handleMouseDown} value={0} />
103+
<Slider onMouseDown={handleMouseDown} defaultValue={0} />
90104
);
91105
const slider = container.firstElementChild as HTMLElement;
92106
fireEvent.mouseDown(slider);
@@ -103,13 +117,6 @@ describe('<Slider />', () => {
103117
);
104118
const slider = container.firstElementChild as HTMLElement;
105119
// mocking containers size
106-
slider.getBoundingClientRect = () =>
107-
({
108-
width: 100,
109-
height: 20,
110-
bottom: 20,
111-
left: 0
112-
} as DOMRect);
113120
const thumb = getByRole('slider');
114121

115122
fireEvent.touchStart(
@@ -292,7 +299,7 @@ describe('<Slider />', () => {
292299
describe('prop: orientation', () => {
293300
it('when vertical, should render with aria-orientation attribute set to "vertical" ', () => {
294301
const { getByRole } = renderWithTheme(
295-
<Slider orientation='vertical' value={0} />
302+
<Slider orientation='vertical' defaultValue={0} />
296303
);
297304

298305
expect(getByRole('slider')).toHaveAttribute(
@@ -314,13 +321,17 @@ describe('<Slider />', () => {
314321
const slider = container.firstElementChild as HTMLElement;
315322

316323
// mocking containers size
317-
slider.getBoundingClientRect = () =>
318-
({
319-
width: 20,
320-
height: 100,
321-
bottom: 100,
322-
left: 0
323-
} as DOMRect);
324+
jest
325+
.spyOn(HTMLElement.prototype, 'getBoundingClientRect')
326+
.mockImplementation(
327+
() =>
328+
({
329+
width: 20,
330+
height: 100,
331+
bottom: 100,
332+
left: 0
333+
} as DOMRect)
334+
);
324335

325336
fireEvent.touchStart(
326337
slider,

‎src/Slider/Slider.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,9 +313,10 @@ const Slider = forwardRef<HTMLDivElement, SliderProps>(
313313
) => {
314314
const Groove = variant === 'flat' ? StyledFlatGroove : StyledGroove;
315315
const vertical = orientation === 'vertical';
316-
const [valueDerived, setValueState] = useControlledOrUncontrolled({
317-
value,
318-
defaultValue: defaultValue ?? min
316+
const [valueDerived = min, setValueState] = useControlledOrUncontrolled({
317+
defaultValue,
318+
onChange: onChange ?? onChangeCommitted,
319+
value
319320
});
320321

321322
const {

‎src/TreeView/TreeView.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,19 @@ function TreeInner<T>(
321321
ref: React.ForwardedRef<HTMLUListElement>
322322
) {
323323
const [expandedInternal, setExpandedInternal] = useControlledOrUncontrolled({
324+
defaultValue: defaultExpanded,
325+
onChange: onNodeToggle,
326+
onChangePropName: 'onNodeToggle',
324327
value: expanded,
325-
defaultValue: defaultExpanded
328+
valuePropName: 'expanded'
326329
});
327330

328331
const [selectedInternal, setSelectedInternal] = useControlledOrUncontrolled({
332+
defaultValue: defaultSelected,
333+
onChange: onNodeSelect,
334+
onChangePropName: 'onNodeSelect',
329335
value: selected,
330-
defaultValue: defaultSelected
336+
valuePropName: 'selected'
331337
});
332338

333339
const toggleMenu = useCallback(

‎src/common/hooks/useControlledOrUncontrolled.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,20 @@
11
import React, { useState, useCallback } from 'react';
22

33
export default function useControlledOrUncontrolled<T>({
4+
defaultValue,
5+
onChange,
6+
onChangePropName = 'onChange',
7+
readOnly,
48
value,
5-
defaultValue
9+
valuePropName ='value'
610
}: {
7-
value: T | undefined;
811
defaultValue: T;
12+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
13+
onChange?: (...args: any[]) => void;
14+
onChangePropName?: string;
15+
readOnly?: boolean;
16+
value: T | undefined;
17+
valuePropName?: string;
918
}): [T, (newValue: React.SetStateAction<T>) => void] {
1019
const isControlled = value !== undefined;
1120
const [controlledValue, setControlledValue] = useState(defaultValue);
@@ -17,5 +26,20 @@ export default function useControlledOrUncontrolled<T>({
1726
},
1827
[isControlled]
1928
);
29+
30+
// Because we provide `onChange` even to uncontrolled components, React's
31+
// default uncontrolled warning must be reimplemented. This also deals with
32+
// props that are different from `value`.
33+
if (isControlled && typeof onChange !== 'function' && !readOnly) {
34+
const message = `Warning: You provided a \`${valuePropName}\` prop to a component without an \`${onChangePropName}\` handler.${
35+
valuePropName === 'value'
36+
? `This will render a read-only field. If the field should be mutable use \`defaultValue\`. Otherwise, set either \`${onChangePropName}\` or \`readOnly\`.`
37+
: `This breaks the component state. You must provide an \`${onChangePropName}\` function that updates \`${valuePropName}\`.`
38+
}`;
39+
40+
// eslint-disable-next-line no-console
41+
console.warn(message);
42+
}
43+
2044
return [isControlled ? value : controlledValue, handleChangeIfUncontrolled];
2145
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /