Skip to content

fix: context provider values not memoized #8578

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ export default [{
"react/display-name": OFF,
"react/jsx-curly-spacing": [ERROR, "never"],
"react/jsx-indent-props": [ERROR, ERROR],
"react/jsx-no-constructed-context-values": ERROR,
"react/jsx-no-duplicate-props": ERROR,
"react/jsx-no-literals": OFF,
"react/jsx-no-undef": ERROR,
Expand Down Expand Up @@ -504,4 +505,4 @@ export default [{
rules: {
"react/react-in-jsx-scope": OFF,
},
}];
}];
4 changes: 3 additions & 1 deletion packages/@react-aria/dnd/stories/VirtualizedListBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,10 @@ React.forwardRef(function (props: any, ref) {
let focusedKey = dropState.target?.type === 'item' ? dropState.target.key : state.selectionManager.focusedKey;
let persistedKeys = useMemo(() => focusedKey != null ? new Set([focusedKey]) : null, [focusedKey]);

const context = useMemo(() => ({state, dropState}), [dropState, state]);

return (
<Context.Provider value={{state, dropState}}>
<Context.Provider value={context}>
<Virtualizer
{...mergeProps(collectionProps, listBoxProps)}
ref={domRef}
Expand Down
4 changes: 2 additions & 2 deletions packages/@react-aria/interactions/src/PressResponder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ React.forwardRef(({children, ...props}: PressResponderProps, ref: ForwardedRef<F
let isRegistered = useRef(false);
let prevContext = useContext(PressResponderContext);
ref = useObjectRef(ref || prevContext?.ref);
let context = mergeProps(prevContext || {}, {
let context = useMemo(() => mergeProps(prevContext || {}, {
...props,
ref,
register() {
Expand All @@ -35,7 +35,7 @@ React.forwardRef(({children, ...props}: PressResponderProps, ref: ForwardedRef<F
prevContext.register();
}
}
});
}), [prevContext, props, ref]);

useSyncRef(prevContext, ref);

Expand Down
16 changes: 10 additions & 6 deletions packages/@react-aria/interactions/src/useFocusable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import {DOMAttributes, FocusableDOMProps, FocusableElement, FocusableProps, RefObject} from '@react-types/shared';
import {focusSafely} from './';
import {getOwnerWindow, isFocusable, mergeProps, mergeRefs, useObjectRef, useSyncRef} from '@react-aria/utils';
import React, {ForwardedRef, forwardRef, MutableRefObject, ReactElement, ReactNode, useContext, useEffect, useRef} from 'react';
import React, {ForwardedRef, forwardRef, MutableRefObject, ReactElement, ReactNode, useContext, useEffect, useMemo, useRef} from 'react';
import {useFocus} from './useFocus';
import {useKeyboard} from './useKeyboard';

Expand Down Expand Up @@ -50,12 +50,16 @@ function useFocusableContext(ref: RefObject<FocusableElement | null>): Focusable
export const FocusableProvider:
React.ForwardRefExoticComponent<FocusableProviderProps & React.RefAttributes<FocusableElement>> =
React.forwardRef(function FocusableProvider(props: FocusableProviderProps, ref: ForwardedRef<FocusableElement>) {
let {children, ...otherProps} = props;
let {children} = props;
let objRef = useObjectRef(ref);
let context = {
...otherProps,
ref: objRef
};
let context = useMemo(() => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const {children: _, ...otherProps} = props;
return {
...otherProps,
ref: objRef
};
}, [objRef, props]);

return (
<FocusableContext.Provider value={context}>
Expand Down
9 changes: 7 additions & 2 deletions packages/@react-aria/overlays/src/PortalProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import React, {createContext, JSX, ReactNode, useContext} from 'react';
import React, {createContext, JSX, ReactNode, useContext, useMemo} from 'react';

export interface PortalProviderProps {
/** Should return the element where we should portal to. Can clear the context by passing null. */
Expand All @@ -29,8 +29,13 @@ export const PortalContext: React.Context<PortalProviderContextValue> = createCo
export function UNSAFE_PortalProvider(props: PortalProviderProps): JSX.Element {
let {getContainer} = props;
let {getContainer: ctxGetContainer} = useUNSAFE_PortalContext();

const context = useMemo(() => ({
getContainer: getContainer === null ? undefined : getContainer ?? ctxGetContainer
}), [ctxGetContainer, getContainer]);

return (
<PortalContext.Provider value={{getContainer: getContainer === null ? undefined : getContainer ?? ctxGetContainer}}>
<PortalContext.Provider value={context}>
{props.children}
</PortalContext.Provider>
);
Expand Down
11 changes: 8 additions & 3 deletions packages/@react-spectrum/accordion/src/Accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {Button, DisclosureGroup, DisclosureGroupProps, DisclosurePanelProps, Dis
import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium';
import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium';
import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils';
import React, {createContext, forwardRef} from 'react';
import React, {createContext, forwardRef, useMemo} from 'react';
import styles from '@adobe/spectrum-css-temp/components/accordion/vars.css';
import {useLocale} from '@react-aria/i18n';
import {useProviderProps} from '@react-spectrum/provider';
Expand All @@ -34,8 +34,13 @@ export const Accordion = /*#__PURE__*/(forwardRef as forwardRefType)(function Ac
props = useProviderProps(props);
let {styleProps} = useStyleProps(props);
let domRef = useDOMRef(ref);

const context = useMemo(() => ({
isQuiet: props.isQuiet || false}
), [props.isQuiet]);

return (
<InternalAccordionContext.Provider value={{isQuiet: props.isQuiet || false}}>
<InternalAccordionContext.Provider value={context}>
<DisclosureGroup
{...props}
{...styleProps}
Expand Down Expand Up @@ -89,7 +94,7 @@ export const DisclosurePanel = /*#__PURE__*/(forwardRef as forwardRefType)(funct
<RACDisclosurePanel
ref={domRef}
{...styleProps as Omit<React.HTMLAttributes<HTMLElement>, 'role'>}
className={classNames(styles, 'spectrum-Accordion-itemContent', styleProps.className)}
className={classNames(styles, 'spectrum-Accordion-itemContent', styleProps.className)}
{...props}>
{props.children}
</RACDisclosurePanel>
Expand Down
7 changes: 6 additions & 1 deletion packages/@react-spectrum/card/src/CardView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,13 @@ export const CardView = React.forwardRef(function CardView<T extends object>(pro
let persistedKeys = useMemo(() => focusedKey != null ? new Set([focusedKey]) : null, [focusedKey]);

// TODO: does aria-row count and aria-col count need to be modified? Perhaps aria-col count needs to be omitted

const context = useMemo(() => ({
state, isQuiet, layout: cardViewLayout, cardOrientation, renderEmptyState
}), [cardOrientation, cardViewLayout, isQuiet, renderEmptyState, state]);

return (
<CardViewContext.Provider value={{state, isQuiet, layout: cardViewLayout, cardOrientation, renderEmptyState}}>
<CardViewContext.Provider value={context}>
<Virtualizer
{...gridProps}
{...styleProps}
Expand Down
5 changes: 3 additions & 2 deletions packages/@react-spectrum/card/stories/Card.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {getDescription, getImage} from './utils';
import {Heading, Text} from '@react-spectrum/text';
import {Image} from '@react-spectrum/image';
import {Meta, StoryObj} from '@storybook/react';
import React, {Dispatch, SetStateAction, useState} from 'react';
import React, {Dispatch, SetStateAction, useMemo, useState} from 'react';
import {SpectrumCardProps} from '@react-types/card';
import {usePress} from '@react-aria/interactions';
import {useProvider} from '@react-spectrum/provider';
Expand Down Expand Up @@ -401,9 +401,10 @@ let SelectableCard = (props: SpectrumCardProps) => {
isSelected: () => !prev.selectionManager.isSelected()
}
}))});
const context = useMemo(() => ({state}), [state]);
return (
<div style={{width: '208px'}} {...pressProps}>
<CardViewContext.Provider value={{state}}>
<CardViewContext.Provider value={context}>
<CardBase {...props} />
</CardViewContext.Provider>
</div>
Expand Down
6 changes: 4 additions & 2 deletions packages/@react-spectrum/color/src/ColorSwatchPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import {ColorSwatchPicker as AriaColorSwatchPicker, ColorSwatchPickerItem as AriaColorSwatchPickerItem} from 'react-aria-components';
import {Color} from '@react-types/color';
import {DOMRef, StyleProps, ValueBase} from '@react-types/shared';
import React, {forwardRef, ReactElement, ReactNode} from 'react';
import React, {forwardRef, ReactElement, ReactNode, useMemo} from 'react';
import {SpectrumColorSwatchContext, SpectrumColorSwatchProps} from './ColorSwatch';
import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'};
import {useDOMRef, useStyleProps} from '@react-spectrum/utils';
Expand Down Expand Up @@ -51,6 +51,8 @@ export const ColorSwatchPicker = forwardRef(function ColorSwatchPicker(props: Sp
let {styleProps} = useStyleProps(props);
let domRef = useDOMRef(ref);

const context = useMemo(() => ({useWrapper, size, rounding}), [rounding, size]);

return (
<AriaColorSwatchPicker
{...otherProps}
Expand All @@ -67,7 +69,7 @@ export const ColorSwatchPicker = forwardRef(function ColorSwatchPicker(props: Sp
}
}
})({density})}>
<SpectrumColorSwatchContext.Provider value={{useWrapper, size, rounding}}>
<SpectrumColorSwatchContext.Provider value={context}>
{props.children}
</SpectrumColorSwatchContext.Provider>
</AriaColorSwatchPicker>
Expand Down
6 changes: 3 additions & 3 deletions packages/@react-spectrum/dialog/src/DialogContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import {DialogContext} from './context';
import {Modal} from '@react-spectrum/overlays';
import React, {JSX, ReactElement, useState} from 'react';
import React, {JSX, ReactElement, useMemo, useState} from 'react';
import {SpectrumDialogContainerProps} from '@react-types/dialog';
import {useOverlayTriggerState} from '@react-stately/overlays';

Expand Down Expand Up @@ -50,11 +50,11 @@ export function DialogContainer(props: SpectrumDialogContainerProps): JSX.Elemen
setLastChild(child);
}

let context = {
let context = useMemo(() => ({
type,
onClose: onDismiss,
isDismissable
};
}), [isDismissable, onDismiss, type]);

let state = useOverlayTriggerState({
isOpen: !!child,
Expand Down
8 changes: 4 additions & 4 deletions packages/@react-spectrum/dialog/src/DialogTrigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {DialogContext} from './context';
import {Modal, Popover, Tray} from '@react-spectrum/overlays';
import {OverlayTriggerState, useOverlayTriggerState} from '@react-stately/overlays';
import {PressResponder} from '@react-aria/interactions';
import React, {Fragment, JSX, ReactElement, useEffect, useRef} from 'react';
import React, {Fragment, JSX, ReactElement, useEffect, useMemo, useRef} from 'react';
import {SpectrumDialogClose, SpectrumDialogProps, SpectrumDialogTriggerProps} from '@react-types/dialog';
import {useIsMobileDevice} from '@react-spectrum/utils';
import {useOverlayTrigger} from '@react-aria/overlays';
Expand Down Expand Up @@ -57,7 +57,7 @@ function DialogTrigger(props: SpectrumDialogTriggerProps) {
let onExiting = () => isExiting.current = true;
let onExited = () => isExiting.current = false;


useEffect(() => {
return () => {
if ((wasOpen.current || isExiting.current) && type !== 'popover' && type !== 'tray' && process.env.NODE_ENV !== 'production') {
Expand Down Expand Up @@ -184,12 +184,12 @@ interface SpectrumDialogTriggerBase {
}

function DialogTriggerBase({type, state, isDismissable, dialogProps = {}, triggerProps = {}, overlay, trigger}: SpectrumDialogTriggerBase) {
let context = {
let context = useMemo(() => ({
type,
onClose: state.close,
isDismissable,
...dialogProps
};
}), [dialogProps, isDismissable, state.close, type]);

return (
<Fragment>
Expand Down
10 changes: 6 additions & 4 deletions packages/@react-spectrum/form/src/Form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils';
import {filterDOMProps} from '@react-aria/utils';
import {FormValidationContext} from '@react-stately/form';
import {Provider, useProviderProps} from '@react-spectrum/provider';
import React, {useContext} from 'react';
import React, {useContext, useMemo} from 'react';
import {SpectrumFormProps} from '@react-types/form';
import styles from '@adobe/spectrum-css-temp/components/fieldlabel/vars.css';

Expand Down Expand Up @@ -68,12 +68,14 @@ export const Form = React.forwardRef(function Form(props: SpectrumFormProps, ref
let {styleProps} = useStyleProps(otherProps);
let domRef = useDOMRef(ref);

let ctx = {
let ctx = useMemo(() => ({
labelPosition,
labelAlign,
necessityIndicator,
validationBehavior
};
}), [labelAlign, labelPosition, necessityIndicator, validationBehavior]);

const validationContext = useMemo(() => validationErrors || {}, [validationErrors]);

return (
<form
Expand All @@ -100,7 +102,7 @@ export const Form = React.forwardRef(function Form(props: SpectrumFormProps, ref
isReadOnly={isReadOnly}
isRequired={isRequired}
validationState={validationState}>
<FormValidationContext.Provider value={validationErrors || {}}>
<FormValidationContext.Provider value={validationContext}>
{children}
</FormValidationContext.Provider>
</Provider>
Expand Down
26 changes: 25 additions & 1 deletion packages/@react-spectrum/list/src/ListView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,32 @@ export const ListView = React.forwardRef(function ListView<T extends object>(pro

let hasAnyChildren = useMemo(() => [...collection].some(item => item.hasChildNodes), [collection]);

const context = useMemo(() => ({
state,
dragState,
dropState,
dragAndDropHooks,
onAction,
isListDraggable,
isListDroppable,
layout,
loadingState,
renderEmptyState
}), [
dragAndDropHooks,
dragState,
dropState,
isListDraggable,
isListDroppable,
layout,
loadingState,
onAction,
renderEmptyState,
state
]);

return (
<ListViewContext.Provider value={{state, dragState, dropState, dragAndDropHooks, onAction, isListDraggable, isListDroppable, layout, loadingState, renderEmptyState}}>
<ListViewContext.Provider value={context}>
<FocusScope>
<FocusRing focusRingClass={classNames(listStyles, 'focus-ring')}>
<Virtualizer
Expand Down
9 changes: 8 additions & 1 deletion packages/@react-spectrum/listbox/src/ListBoxBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,15 @@ export const ListBoxBase = React.forwardRef(function ListBoxBase<T extends objec
let focusedKey = state.selectionManager.focusedKey;
let persistedKeys = useMemo(() => focusedKey != null ? new Set([focusedKey]) : null, [focusedKey]);

const context = useMemo(() => ({
state,
renderEmptyState,
shouldFocusOnHover,
shouldUseVirtualFocus
}), [renderEmptyState, shouldFocusOnHover, shouldUseVirtualFocus, state]);

return (
<ListBoxContext.Provider value={{state, renderEmptyState, shouldFocusOnHover, shouldUseVirtualFocus}}>
<ListBoxContext.Provider value={context}>
<FocusScope>
<Virtualizer
{...styleProps}
Expand Down
8 changes: 6 additions & 2 deletions packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {FocusScope} from '@react-aria/focus';
import {getInteractionModality} from '@react-aria/interactions';
import helpStyles from '@adobe/spectrum-css-temp/components/contextualhelp/vars.css';
import {Popover} from '@react-spectrum/overlays';
import React, {JSX, KeyboardEventHandler, ReactElement, useEffect, useRef, useState} from 'react';
import React, {JSX, KeyboardEventHandler, ReactElement, useEffect, useMemo, useRef, useState} from 'react';
import ReactDOM from 'react-dom';
import styles from '@adobe/spectrum-css-temp/components/menu/vars.css';
import {SubmenuTriggerContext, useMenuStateContext} from './context';
Expand Down Expand Up @@ -158,9 +158,13 @@ function ContextualHelpTrigger(props: InternalMenuDialogTriggerProps): ReactElem
);
}

const context = useMemo(() => ({
isUnavailable, triggerRef, ...submenuTriggerProps
}), [isUnavailable, submenuTriggerProps]);

return (
<>
<SubmenuTriggerContext.Provider value={{isUnavailable, triggerRef, ...submenuTriggerProps}}>{trigger}</SubmenuTriggerContext.Provider>
<SubmenuTriggerContext.Provider value={context}>{trigger}</SubmenuTriggerContext.Provider>
<SlotProvider slots={slots}>
{submenuTriggerState.isOpen && overlay}
</SlotProvider>
Expand Down
13 changes: 11 additions & 2 deletions packages/@react-spectrum/menu/src/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {MenuContext, MenuStateContext, useMenuStateContext} from './context';
import {MenuItem} from './MenuItem';
import {MenuSection} from './MenuSection';
import {mergeProps, useLayoutEffect, useSlotId, useSyncRef} from '@react-aria/utils';
import React, {KeyboardEventHandler, ReactElement, ReactNode, RefObject, useContext, useEffect, useRef, useState} from 'react';
import React, {KeyboardEventHandler, ReactElement, ReactNode, RefObject, useContext, useEffect, useMemo, useRef, useState} from 'react';
import {RootMenuTriggerState} from '@react-stately/menu';
import {SpectrumMenuProps} from '@react-types/menu';
import styles from '@adobe/spectrum-css-temp/components/menu/vars.css';
Expand Down Expand Up @@ -71,8 +71,17 @@ export const Menu = React.forwardRef(function Menu<T extends object>(props: Spec
hasOpenSubmenu = nextMenuLevel != null;
}

const context = useMemo(() => ({
popoverContainer,
trayContainerRef,
menu: domRef,
submenu: submenuRef,
rootMenuTriggerState,
state
}), [domRef, popoverContainer, rootMenuTriggerState, state]);

return (
<MenuStateContext.Provider value={{popoverContainer, trayContainerRef, menu: domRef, submenu: submenuRef, rootMenuTriggerState, state}}>
<MenuStateContext.Provider value={context}>
<div style={{height: hasOpenSubmenu ? '100%' : undefined}} ref={trayContainerRef} />
<FocusScope>
<TrayHeaderWrapper
Expand Down
Loading