From 6b2c1a6c4ced816b57f50420262363d8cca4a884 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Wed, 6 Dec 2023 17:00:47 +0800 Subject: [PATCH] [base-ui] useControllableReducer warns when controlled props become uncontrolled (and vice versa) (#39096) --- docs/pages/base-ui/api/use-dropdown.json | 4 ++ docs/pages/base-ui/api/use-menu.json | 4 ++ docs/pages/base-ui/api/use-select.json | 4 ++ .../api-docs/use-dropdown/use-dropdown.json | 3 + .../api-docs/use-menu/use-menu.json | 3 + .../api-docs/use-select/use-select.json | 3 + packages/mui-base/src/Menu/Menu.tsx | 1 + packages/mui-base/src/Select/Select.test.tsx | 32 ++++++++++ packages/mui-base/src/Select/Select.tsx | 1 + .../mui-base/src/useDropdown/useDropdown.ts | 3 +- .../src/useDropdown/useDropdown.types.ts | 6 ++ packages/mui-base/src/useList/useList.ts | 2 + .../mui-base/src/useList/useList.types.ts | 6 ++ packages/mui-base/src/useMenu/useMenu.ts | 2 + .../mui-base/src/useMenu/useMenu.types.ts | 6 ++ packages/mui-base/src/useSelect/useSelect.ts | 2 + .../mui-base/src/useSelect/useSelect.types.ts | 6 ++ packages/mui-base/src/useTab/useTab.test.tsx | 4 +- .../src/utils/useControllableReducer.test.tsx | 62 +++++++++++++++++++ .../src/utils/useControllableReducer.ts | 32 ++++++++++ .../src/utils/useControllableReducer.types.ts | 5 ++ .../mui-material-next/src/Menu/Menu.test.tsx | 2 +- packages/mui-material-next/src/Menu/Menu.tsx | 2 + 23 files changed, 191 insertions(+), 4 deletions(-) diff --git a/docs/pages/base-ui/api/use-dropdown.json b/docs/pages/base-ui/api/use-dropdown.json index 0c3d4691bd3bec..72a551c82ee7aa 100644 --- a/docs/pages/base-ui/api/use-dropdown.json +++ b/docs/pages/base-ui/api/use-dropdown.json @@ -1,5 +1,9 @@ { "parameters": { + "componentName": { + "type": { "name": "string", "description": "string" }, + "default": "'useDropdown'" + }, "defaultOpen": { "type": { "name": "boolean", "description": "boolean" } }, "onOpenChange": { "type": { diff --git a/docs/pages/base-ui/api/use-menu.json b/docs/pages/base-ui/api/use-menu.json index 0c6f17b9dfd63e..f117ccfae6e480 100644 --- a/docs/pages/base-ui/api/use-menu.json +++ b/docs/pages/base-ui/api/use-menu.json @@ -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" diff --git a/docs/pages/base-ui/api/use-select.json b/docs/pages/base-ui/api/use-select.json index 5f4cd6d430a3ac..51f573b1423e3b 100644 --- a/docs/pages/base-ui/api/use-select.json +++ b/docs/pages/base-ui/api/use-select.json @@ -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": { diff --git a/docs/translations/api-docs/use-dropdown/use-dropdown.json b/docs/translations/api-docs/use-dropdown/use-dropdown.json index 5ee4ef4ebaf1c1..636ab4b7827d30 100644 --- a/docs/translations/api-docs/use-dropdown/use-dropdown.json +++ b/docs/translations/api-docs/use-dropdown/use-dropdown.json @@ -1,6 +1,9 @@ { "hookDescription": "", "parametersDescriptions": { + "componentName": { + "description": "The name of the component using useDropdown. For debugging purposes." + }, "defaultOpen": { "description": "If true, the dropdown is initially open." }, "onOpenChange": { "description": "Callback fired when the component requests to be opened or closed." diff --git a/docs/translations/api-docs/use-menu/use-menu.json b/docs/translations/api-docs/use-menu/use-menu.json index cf97ce23dcc386..07e143579e1a60 100644 --- a/docs/translations/api-docs/use-menu/use-menu.json +++ b/docs/translations/api-docs/use-menu/use-menu.json @@ -4,6 +4,9 @@ "autoFocus": { "description": "If true (Default) will focus the highligted item. If you set this prop to false 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 true, it will be possible to highlight disabled items." }, diff --git a/docs/translations/api-docs/use-select/use-select.json b/docs/translations/api-docs/use-select/use-select.json index e083d00f9f7b16..e9c367b8131ece 100644 --- a/docs/translations/api-docs/use-select/use-select.json +++ b/docs/translations/api-docs/use-select/use-select.json @@ -5,6 +5,9 @@ "description": "A function used to determine if two options' values are equal. By default, reference equality is used.
There is a performance impact when using the areOptionsEqual prop (proportional to the number of options). Therefore, it'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 true, the select will be open by default." }, "defaultValue": { "description": "The default selected value. Use when the component is not controlled." diff --git a/packages/mui-base/src/Menu/Menu.tsx b/packages/mui-base/src/Menu/Menu.tsx index b5cb6933cebc52..f9defa7cfe6e6b 100644 --- a/packages/mui-base/src/Menu/Menu.tsx +++ b/packages/mui-base/src/Menu/Menu.tsx @@ -50,6 +50,7 @@ const Menu = React.forwardRef(function Menu', () => { expect(hiddenInput).to.have.value(''); }); }); + + describe('warnings', () => { + it('should warn when switching from controlled to uncontrolled', () => { + const { setProps } = render( + , + ); + + 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( + , + ); + + expect(() => { + setProps({ value: 1 }); + }).toErrorDev( + 'useControllableReducer: The Select component is changing an uncontrolled prop to be controlled: selectedValues', + ); + }); + }); }); diff --git a/packages/mui-base/src/Select/Select.tsx b/packages/mui-base/src/Select/Select.tsx index e43968bd4eccfc..7b250dd7e733d3 100644 --- a/packages/mui-base/src/Select/Select.tsx +++ b/packages/mui-base/src/Select/Select.tsx @@ -144,6 +144,7 @@ const Select = React.forwardRef(function Select< onOpenChange: onListboxOpenChange, getOptionAsString, value: valueProp, + componentName: 'Select', }); const ownerState: SelectOwnerState = { diff --git a/packages/mui-base/src/useDropdown/useDropdown.ts b/packages/mui-base/src/useDropdown/useDropdown.ts index 802018f91fec62..e55186cc0dfbd1 100644 --- a/packages/mui-base/src/useDropdown/useDropdown.ts +++ b/packages/mui-base/src/useDropdown/useDropdown.ts @@ -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(''); const [triggerElement, setTriggerElement] = React.useState(null); const lastActionType = React.useRef(null); @@ -43,6 +43,7 @@ export function useDropdown(parameters: UseDropdownParameters = {}) { initialState: defaultOpen ? { open: true } : { open: false }, onStateChange: handleStateChange, reducer: dropdownReducer, + componentName, }); React.useEffect(() => { diff --git a/packages/mui-base/src/useDropdown/useDropdown.types.ts b/packages/mui-base/src/useDropdown/useDropdown.types.ts index 90ef8af1aee6d2..568e784a0a3b30 100644 --- a/packages/mui-base/src/useDropdown/useDropdown.types.ts +++ b/packages/mui-base/src/useDropdown/useDropdown.types.ts @@ -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 { diff --git a/packages/mui-base/src/useList/useList.ts b/packages/mui-base/src/useList/useList.ts index d069631a7a9280..6d44beef22dbd8 100644 --- a/packages/mui-base/src/useList/useList.ts +++ b/packages/mui-base/src/useList/useList.ts @@ -90,6 +90,7 @@ function useList< reducerActionContext = EMPTY_OBJECT as CustomActionContext, selectionMode = 'single', stateReducer: externalReducer, + componentName = 'useList', } = params; if (process.env.NODE_ENV !== 'production') { @@ -221,6 +222,7 @@ function useList< controlledProps, stateComparers, onStateChange: handleStateChange, + componentName, }); const { highlightedValue, selectedValues } = state; diff --git a/packages/mui-base/src/useList/useList.types.ts b/packages/mui-base/src/useList/useList.types.ts index 0bd41eeabd4014..33c12b4386ffa6 100644 --- a/packages/mui-base/src/useList/useList.types.ts +++ b/packages/mui-base/src/useList/useList.types.ts @@ -215,6 +215,12 @@ export interface UseListParameters< ListActionContext & CustomActionContext >, ) => State; + /** + * The name of the component using useList. + * For debugging purposes. + * @default 'useList' + */ + componentName?: string; } export interface ListItemState { diff --git a/packages/mui-base/src/useMenu/useMenu.ts b/packages/mui-base/src/useMenu/useMenu.ts index 0e86bd674aabc4..456f34b4ac8483 100644 --- a/packages/mui-base/src/useMenu/useMenu.ts +++ b/packages/mui-base/src/useMenu/useMenu.ts @@ -44,6 +44,7 @@ export function useMenu(parameters: UseMenuParameters = {}): UseMenuReturnValue disabledItemsFocusable = true, disableListWrap = false, autoFocus = true, + componentName = 'useMenu', } = parameters; const rootRef = React.useRef(null); @@ -115,6 +116,7 @@ export function useMenu(parameters: UseMenuParameters = {}): UseMenuReturnValue reducerActionContext, selectionMode: 'none', stateReducer: menuReducer, + componentName, }); useEnhancedEffect(() => { diff --git a/packages/mui-base/src/useMenu/useMenu.types.ts b/packages/mui-base/src/useMenu/useMenu.types.ts index 861e651a0db51a..f9f6bd606963c9 100644 --- a/packages/mui-base/src/useMenu/useMenu.types.ts +++ b/packages/mui-base/src/useMenu/useMenu.types.ts @@ -33,6 +33,12 @@ export interface UseMenuParameters { * The ref to the menu's listbox node. */ listboxRef?: React.Ref; + /** + * The name of the component using useMenu. + * For debugging purposes. + * @default 'useMenu' + */ + componentName?: string; } export interface UseMenuReturnValue { diff --git a/packages/mui-base/src/useSelect/useSelect.ts b/packages/mui-base/src/useSelect/useSelect.ts index c56af99326db97..abc9d52240fdf5 100644 --- a/packages/mui-base/src/useSelect/useSelect.ts +++ b/packages/mui-base/src/useSelect/useSelect.ts @@ -101,6 +101,7 @@ function useSelect( getOptionAsString = defaultOptionStringifier, getSerializedValue = defaultFormValueProvider, value: valueProp, + componentName = 'useSelect', } = props; const buttonRef = React.useRef(null); @@ -287,6 +288,7 @@ function useSelect( getItemAsString: stringifyOption, selectionMode: multiple ? 'multiple' : 'single', stateReducer: selectReducer, + componentName, }; const { diff --git a/packages/mui-base/src/useSelect/useSelect.types.ts b/packages/mui-base/src/useSelect/useSelect.types.ts index 217bd98c3c00fa..b94883b6350ee8 100644 --- a/packages/mui-base/src/useSelect/useSelect.types.ts +++ b/packages/mui-base/src/useSelect/useSelect.types.ts @@ -123,6 +123,12 @@ export interface UseSelectParameters; + /** + * The name of the component using useSelect. + * For debugging purposes. + * @default 'useSelect' + */ + componentName?: string; } interface UseSelectButtonSlotEventHandlers { diff --git a/packages/mui-base/src/useTab/useTab.test.tsx b/packages/mui-base/src/useTab/useTab.test.tsx index 829dfd1a175670..c20252849006af 100644 --- a/packages/mui-base/src/useTab/useTab.test.tsx +++ b/packages/mui-base/src/useTab/useTab.test.tsx @@ -18,7 +18,7 @@ describe('useTab', () => { function Test() { return ( - + @@ -43,7 +43,7 @@ describe('useTab', () => { function Test() { return ( - + diff --git a/packages/mui-base/src/utils/useControllableReducer.test.tsx b/packages/mui-base/src/utils/useControllableReducer.test.tsx index e6a77bd08eaacb..a6cbde41c2b518 100644 --- a/packages/mui-base/src/utils/useControllableReducer.test.tsx +++ b/packages/mui-base/src/utils/useControllableReducer.test.tsx @@ -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 = { + 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 ( +

+ {state.make} {state.model} ({state.productionYear}) +

+ ); + } + + const { container, setProps } = render(); + 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 = { + 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 ( +

+ {state.make} {state.model} ({state.productionYear}) +

+ ); + } + + const { container, setProps } = render(); + 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', () => { diff --git a/packages/mui-base/src/utils/useControllableReducer.ts b/packages/mui-base/src/utils/useControllableReducer.ts index 790719a937279b..544a46e84a066e 100644 --- a/packages/mui-base/src/utils/useControllableReducer.ts +++ b/packages/mui-base/src/utils/useControllableReducer.ts @@ -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)[key] !== undefined && + (controlledProps as Record)[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)[key] === undefined && + (controlledProps as Record)[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) => { diff --git a/packages/mui-base/src/utils/useControllableReducer.types.ts b/packages/mui-base/src/utils/useControllableReducer.types.ts index c4769af6c32ae9..e8883082e2d33c 100644 --- a/packages/mui-base/src/utils/useControllableReducer.types.ts +++ b/packages/mui-base/src/utils/useControllableReducer.types.ts @@ -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; } /* diff --git a/packages/mui-material-next/src/Menu/Menu.test.tsx b/packages/mui-material-next/src/Menu/Menu.test.tsx index 8671323165e063..5aa5a057c42c28 100644 --- a/packages/mui-material-next/src/Menu/Menu.test.tsx +++ b/packages/mui-material-next/src/Menu/Menu.test.tsx @@ -218,7 +218,7 @@ describe('', () => { it('should not focus list if autoFocus=false', () => { const { setProps } = render( - +
, ); diff --git a/packages/mui-material-next/src/Menu/Menu.tsx b/packages/mui-material-next/src/Menu/Menu.tsx index 0911d65102ce72..8ef8fdfae3cd1e 100644 --- a/packages/mui-material-next/src/Menu/Menu.tsx +++ b/packages/mui-material-next/src/Menu/Menu.tsx @@ -114,6 +114,7 @@ const MenuInner = React.forwardRef(function Menu(inProps, ref) { disabledItemsFocusable: Boolean(disabledItemsFocusable), disableListWrap: Boolean(disableListWrap), autoFocus, + componentName: 'Menu', }); const ownerState = { @@ -302,6 +303,7 @@ const Menu = React.forwardRef(function Menu(inProps, ref) { const { contextValue: dropdownContextValue } = useDropdown({ open, + componentName: 'Menu', }); return !upperDropdownContext ? (