Skip to content

Commit

Permalink
[base-ui] useControllableReducer warns when controlled props become u…
Browse files Browse the repository at this point in the history
…ncontrolled (and vice versa) (mui#39096)
  • Loading branch information
mj12albert authored and mnajdova committed Dec 6, 2023
1 parent 1dafef9 commit 9c52c66
Show file tree
Hide file tree
Showing 23 changed files with 191 additions and 4 deletions.
4 changes: 4 additions & 0 deletions docs/pages/base-ui/api/use-dropdown.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{
"parameters": {
"componentName": {
"type": { "name": "string", "description": "string" },
"default": "'useDropdown'"
},
"defaultOpen": { "type": { "name": "boolean", "description": "boolean" } },
"onOpenChange": {
"type": {
Expand Down
4 changes: 4 additions & 0 deletions docs/pages/base-ui/api/use-menu.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{
"parameters": {
"autoFocus": { "type": { "name": "boolean", "description": "boolean" }, "default": "true" },
"componentName": {
"type": { "name": "string", "description": "string" },
"default": "'useMenu'"
},
"disabledItemsFocusable": {
"type": { "name": "boolean", "description": "boolean" },
"default": "true"
Expand Down
4 changes: 4 additions & 0 deletions docs/pages/base-ui/api/use-select.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
"buttonRef": {
"type": { "name": "React.Ref<Element>", "description": "React.Ref<Element>" }
},
"componentName": {
"type": { "name": "string", "description": "string" },
"default": "'useSelect'"
},
"defaultOpen": { "type": { "name": "boolean", "description": "boolean" }, "default": "false" },
"defaultValue": {
"type": {
Expand Down
3 changes: 3 additions & 0 deletions docs/translations/api-docs/use-dropdown/use-dropdown.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"hookDescription": "",
"parametersDescriptions": {
"componentName": {
"description": "The name of the component using useDropdown. For debugging purposes."
},
"defaultOpen": { "description": "If <code>true</code>, the dropdown is initially open." },
"onOpenChange": {
"description": "Callback fired when the component requests to be opened or closed."
Expand Down
3 changes: 3 additions & 0 deletions docs/translations/api-docs/use-menu/use-menu.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
"autoFocus": {
"description": "If <code>true</code> (Default) will focus the highligted item. If you set this prop to <code>false</code> the focus will not be moved inside the Menu component. This has severe accessibility implications and should only be considered if you manage focus otherwise."
},
"componentName": {
"description": "The name of the component using useMenu. For debugging purposes."
},
"disabledItemsFocusable": {
"description": "If <code>true</code>, it will be possible to highlight disabled items."
},
Expand Down
3 changes: 3 additions & 0 deletions docs/translations/api-docs/use-select/use-select.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"description": "A function used to determine if two options&#39; values are equal. By default, reference equality is used.<br>There is a performance impact when using the <code>areOptionsEqual</code> prop (proportional to the number of options). Therefore, it&#39;s recommented to use the default reference equality comparison whenever possible."
},
"buttonRef": { "description": "The ref of the trigger button element." },
"componentName": {
"description": "The name of the component using useSelect. For debugging purposes."
},
"defaultOpen": { "description": "If <code>true</code>, the select will be open by default." },
"defaultValue": {
"description": "The default selected value. Use when the component is not controlled."
Expand Down
1 change: 1 addition & 0 deletions packages/mui-base/src/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const Menu = React.forwardRef(function Menu<RootComponentType extends React.Elem

const { contextValue, getListboxProps, dispatch, open, triggerElement } = useMenu({
onItemsChange,
componentName: 'Menu',
});

const anchor = anchorProp ?? triggerElement;
Expand Down
32 changes: 32 additions & 0 deletions packages/mui-base/src/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1352,4 +1352,36 @@ describe('<Select />', () => {
expect(hiddenInput).to.have.value('');
});
});

describe('warnings', () => {
it('should warn when switching from controlled to uncontrolled', () => {
const { setProps } = render(
<Select value={1}>
<Option value={1}>One</Option>
<Option value={2}>Two</Option>
</Select>,
);

expect(() => {
setProps({ value: undefined });
}).toErrorDev(
'useControllableReducer: The Select component is changing a controlled prop to be uncontrolled: selectedValues',
);
});

it('should warn when switching between uncontrolled to controlled', () => {
const { setProps } = render(
<Select>
<Option value={1}>One</Option>
<Option value={2}>Two</Option>
</Select>,
);

expect(() => {
setProps({ value: 1 });
}).toErrorDev(
'useControllableReducer: The Select component is changing an uncontrolled prop to be controlled: selectedValues',
);
});
});
});
1 change: 1 addition & 0 deletions packages/mui-base/src/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ const Select = React.forwardRef(function Select<
onOpenChange: onListboxOpenChange,
getOptionAsString,
value: valueProp,
componentName: 'Select',
});

const ownerState: SelectOwnerState<OptionValue, Multiple> = {
Expand Down
3 changes: 2 additions & 1 deletion packages/mui-base/src/useDropdown/useDropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { dropdownReducer } from './dropdownReducer';
* - [useDropdown API](https://mui.com/base-ui/react-menu/hooks-api/#use-dropdown)
*/
export function useDropdown(parameters: UseDropdownParameters = {}) {
const { defaultOpen, onOpenChange, open: openProp } = parameters;
const { defaultOpen, onOpenChange, open: openProp, componentName = 'useDropdown' } = parameters;
const [popupId, setPopupId] = React.useState<string>('');
const [triggerElement, setTriggerElement] = React.useState<HTMLElement | null>(null);
const lastActionType = React.useRef<string | null>(null);
Expand All @@ -43,6 +43,7 @@ export function useDropdown(parameters: UseDropdownParameters = {}) {
initialState: defaultOpen ? { open: true } : { open: false },
onStateChange: handleStateChange,
reducer: dropdownReducer,
componentName,
});

React.useEffect(() => {
Expand Down
6 changes: 6 additions & 0 deletions packages/mui-base/src/useDropdown/useDropdown.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ export interface UseDropdownParameters {
* This is a controlled counterpart of `defaultOpen`.
*/
open?: boolean;
/**
* The name of the component using useDropdown.
* For debugging purposes.
* @default 'useDropdown'
*/
componentName?: string;
}

export interface UseDropdownReturnValue {
Expand Down
2 changes: 2 additions & 0 deletions packages/mui-base/src/useList/useList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ function useList<
reducerActionContext = EMPTY_OBJECT as CustomActionContext,
selectionMode = 'single',
stateReducer: externalReducer,
componentName = 'useList',
} = params;

if (process.env.NODE_ENV !== 'production') {
Expand Down Expand Up @@ -221,6 +222,7 @@ function useList<
controlledProps,
stateComparers,
onStateChange: handleStateChange,
componentName,
});

const { highlightedValue, selectedValues } = state;
Expand Down
6 changes: 6 additions & 0 deletions packages/mui-base/src/useList/useList.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ export interface UseListParameters<
ListActionContext<ItemValue> & CustomActionContext
>,
) => State;
/**
* The name of the component using useList.
* For debugging purposes.
* @default 'useList'
*/
componentName?: string;
}

export interface ListItemState {
Expand Down
2 changes: 2 additions & 0 deletions packages/mui-base/src/useMenu/useMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export function useMenu(parameters: UseMenuParameters = {}): UseMenuReturnValue
disabledItemsFocusable = true,
disableListWrap = false,
autoFocus = true,
componentName = 'useMenu',
} = parameters;

const rootRef = React.useRef<HTMLElement>(null);
Expand Down Expand Up @@ -115,6 +116,7 @@ export function useMenu(parameters: UseMenuParameters = {}): UseMenuReturnValue
reducerActionContext,
selectionMode: 'none',
stateReducer: menuReducer,
componentName,
});

useEnhancedEffect(() => {
Expand Down
6 changes: 6 additions & 0 deletions packages/mui-base/src/useMenu/useMenu.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ export interface UseMenuParameters {
* The ref to the menu's listbox node.
*/
listboxRef?: React.Ref<Element>;
/**
* The name of the component using useMenu.
* For debugging purposes.
* @default 'useMenu'
*/
componentName?: string;
}

export interface UseMenuReturnValue {
Expand Down
2 changes: 2 additions & 0 deletions packages/mui-base/src/useSelect/useSelect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ function useSelect<OptionValue, Multiple extends boolean = false>(
getOptionAsString = defaultOptionStringifier,
getSerializedValue = defaultFormValueProvider,
value: valueProp,
componentName = 'useSelect',
} = props;

const buttonRef = React.useRef<HTMLElement>(null);
Expand Down Expand Up @@ -287,6 +288,7 @@ function useSelect<OptionValue, Multiple extends boolean = false>(
getItemAsString: stringifyOption,
selectionMode: multiple ? 'multiple' : 'single',
stateReducer: selectReducer,
componentName,
};

const {
Expand Down
6 changes: 6 additions & 0 deletions packages/mui-base/src/useSelect/useSelect.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ export interface UseSelectParameters<OptionValue, Multiple extends boolean = fal
* Set to `null` to deselect all options.
*/
value?: SelectValue<OptionValue, Multiple>;
/**
* The name of the component using useSelect.
* For debugging purposes.
* @default 'useSelect'
*/
componentName?: string;
}

interface UseSelectButtonSlotEventHandlers {
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-base/src/useTab/useTab.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('useTab', () => {

function Test() {
return (
<Tabs>
<Tabs defaultValue={1}>
<TabsList>
<TestTab />
</TabsList>
Expand All @@ -43,7 +43,7 @@ describe('useTab', () => {

function Test() {
return (
<Tabs>
<Tabs defaultValue={1}>
<TabsList>
<TestTab />
</TabsList>
Expand Down
62 changes: 62 additions & 0 deletions packages/mui-base/src/utils/useControllableReducer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,68 @@ describe('useControllableReducer', () => {
setProps({ make: 'Mazda' });
expect(container.firstChild).to.have.text('Mazda 3 (2022)');
});

it('warns when a controlled prop becomes uncontrolled', () => {
const reducerParameters: ControllableReducerParameters<TestState, SetMakeAction> = {
reducer: (state) => state,
initialState: { make: 'Mazda', model: '3', productionYear: 2022 },
};

function TestComponent(props: { make: string }) {
const [state] = useControllableReducer({
...reducerParameters,
controlledProps: {
make: props.make,
},
componentName: 'TestComponent',
});
return (
<p>
{state.make} {state.model} ({state.productionYear})
</p>
);
}

const { container, setProps } = render(<TestComponent make="Tesla" />);
expect(container.firstChild).to.have.text('Tesla 3 (2022)');

expect(() => {
setProps({ make: undefined });
}).to.toErrorDev(
'useControllableReducer: The TestComponent component is changing a controlled prop to be uncontrolled: make',
);
});

it('warns when an uncontrolled prop becomes controlled', () => {
const reducerParameters: ControllableReducerParameters<TestState, SetMakeAction> = {
reducer: (state) => state,
initialState: { make: 'Mazda', model: '3', productionYear: 2022 },
};

function TestComponent(props: { make?: string }) {
const [state] = useControllableReducer({
...reducerParameters,
controlledProps: {
make: props.make,
},
componentName: 'TestComponent',
});
return (
<p>
{state.make} {state.model} ({state.productionYear})
</p>
);
}

const { container, setProps } = render(<TestComponent />);
expect(container.firstChild).to.have.text('Mazda 3 (2022)');

expect(() => {
setProps({ make: 'Tesla' });
}).to.toErrorDev(
'useControllableReducer: The TestComponent component is changing an uncontrolled prop to be controlled: make',
);
});
});

describe('param: stateComparers', () => {
Expand Down
32 changes: 32 additions & 0 deletions packages/mui-base/src/utils/useControllableReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,40 @@ export function useControllableReducer<
stateComparers = EMPTY_OBJECT,
onStateChange = NOOP,
actionContext,
componentName = '',
} = parameters;

const controlledPropsRef = React.useRef(controlledProps);

if (process.env.NODE_ENV !== 'production') {
// eslint-disable-next-line react-hooks/rules-of-hooks
React.useEffect(() => {
Object.keys(controlledProps).forEach((key) => {
if (
(controlledPropsRef.current as Record<string, unknown>)[key] !== undefined &&
(controlledProps as Record<string, unknown>)[key] === undefined
) {
console.error(
`useControllableReducer: ${
componentName ? `The ${componentName} component` : 'A component'
} is changing a controlled prop to be uncontrolled: ${key}`,
);
}

if (
(controlledPropsRef.current as Record<string, unknown>)[key] === undefined &&
(controlledProps as Record<string, unknown>)[key] !== undefined
) {
console.error(
`useControllableReducer: ${
componentName ? `The ${componentName} component` : 'A component'
} is changing an uncontrolled prop to be controlled: ${key}`,
);
}
});
}, [controlledProps, componentName]);
}

// The reducer that is passed to React.useReducer is wrapped with a function that augments the state with controlled values.
const reducerWithControlledState = React.useCallback(
(state: State, action: ActionWithContext<Action, ActionContext>) => {
Expand Down
5 changes: 5 additions & 0 deletions packages/mui-base/src/utils/useControllableReducer.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ export interface ControllableReducerParameters<
* Additional properties that will be added to the action.
*/
actionContext?: ActionContext;
/**
* The name of the component using useControllableReducer.
* For debugging purposes.
*/
componentName?: string;
}

/*
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material-next/src/Menu/Menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ describe('<Menu />', () => {

it('should not focus list if autoFocus=false', () => {
const { setProps } = render(
<Menu anchorEl={document.createElement('div')} autoFocus={false}>
<Menu anchorEl={document.createElement('div')} autoFocus={false} open={false}>
<div tabIndex={-1} />
</Menu>,
);
Expand Down
2 changes: 2 additions & 0 deletions packages/mui-material-next/src/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ const MenuInner = React.forwardRef(function Menu(inProps, ref) {
disabledItemsFocusable: Boolean(disabledItemsFocusable),
disableListWrap: Boolean(disableListWrap),
autoFocus,
componentName: 'Menu',
});

const ownerState = {
Expand Down Expand Up @@ -302,6 +303,7 @@ const Menu = React.forwardRef(function Menu(inProps, ref) {

const { contextValue: dropdownContextValue } = useDropdown({
open,
componentName: 'Menu',
});

return !upperDropdownContext ? (
Expand Down

0 comments on commit 9c52c66

Please sign in to comment.