Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[base-ui] useControllableReducer warns when controlled props become uncontrolled (and vice versa) #39096

Merged
merged 18 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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}>
DiegoAndai marked this conversation as resolved.
Show resolved Hide resolved
<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