From 072c22a30a62d8446182dfc08a075c20d41e587d Mon Sep 17 00:00:00 2001 From: Cody Olsen Date: Tue, 29 Oct 2024 15:13:49 +0100 Subject: [PATCH] feat: use react compiler --- .eslintrc.js | 2 +- .github/workflows/main.yml | 2 +- .storybook/main.ts | 4 +- package.config.ts | 20 +++++ package.json | 3 +- pnpm-lock.yaml | 18 ++--- .../components/autocomplete/autocomplete.tsx | 73 +++++++++++-------- src/core/components/dialog/dialog.tsx | 6 +- src/core/components/menu/menu.tsx | 4 +- src/core/components/menu/menuGroup.tsx | 3 +- src/core/components/menu/menuItem.tsx | 6 +- src/core/components/menu/useMenuController.ts | 39 +++++----- src/core/components/tree/treeItem.tsx | 26 +++---- src/core/hooks/useArrayProp.ts | 23 +++--- src/core/hooks/useMatchMedia.ts | 34 +++------ src/core/primitives/avatar/avatarStack.tsx | 7 +- src/core/primitives/popover/popover.tsx | 48 ++++++------ src/core/primitives/tooltip/tooltip.tsx | 11 ++- src/core/theme/themeProvider.tsx | 10 +-- src/core/utils/elementQuery/elementQuery.tsx | 3 +- src/core/utils/portal/portalProvider.tsx | 10 +-- 21 files changed, 190 insertions(+), 162 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 491976d9e..ab8d19e0c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -90,7 +90,7 @@ module.exports = { 'react/prop-types': 'off', 'react-hooks/exhaustive-deps': 'error', // Checks effect dependencies 'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks - 'react-compiler/react-compiler': 'warn', // Set to error once existing warnings are fixed + 'react-compiler/react-compiler': 'error', 'react/no-unescaped-entities': 'off', 'no-restricted-imports': [ 'error', diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 07b392309..e46a99e75 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -38,7 +38,7 @@ jobs: - run: pnpm install - name: Register Problem Matcher for ESLint that handles -f compact and shows warnings and errors inline on PRs run: echo "::add-matcher::.github/eslint-compact.json" - - run: "pnpm lint -f compact --rule 'no-warning-comments: [off]' --max-warnings 12" + - run: "pnpm lint -f compact --rule 'no-warning-comments: [off]' --max-warnings 0" test: needs: [build] diff --git a/.storybook/main.ts b/.storybook/main.ts index 9dba041a7..adf3a670b 100644 --- a/.storybook/main.ts +++ b/.storybook/main.ts @@ -25,7 +25,9 @@ const config: StorybookConfig = { return mergeConfig(config, { plugins: [ viteReact({ - babel: {plugins: [['babel-plugin-react-compiler', {target: '18'}]]}, + babel: { + plugins: [['babel-plugin-react-compiler', {target: '18'}]], + }, }), tsconfigPaths(), ], diff --git a/package.config.ts b/package.config.ts index 5f9bb0b2f..cd657229e 100644 --- a/package.config.ts +++ b/package.config.ts @@ -14,4 +14,24 @@ export default defineConfig({ noImplicitBrowsersList: 'off', }, tsconfig: 'tsconfig.dist.json', + babel: {reactCompiler: true}, + reactCompilerOptions: { + target: '18', + logger: { + logEvent(filename, event) { + /* eslint-disable no-console */ + if (event.kind === 'CompileError') { + console.group(`[${filename}] ${event.kind}`) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const {reason, description, severity, loc, suggestions} = event.detail as any + + console.error(`[${severity}] ${reason}`) + console.log(`${filename}:${loc.start?.line}:${loc.start?.column} ${description}`) + console.log(suggestions) + + console.groupEnd() + } + }, + }, + }, }) diff --git a/package.json b/package.json index befb5f753..4dfc533d5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@sanity/ui", - "version": "2.8.13", + "version": "2.9.0-canary.1", "keywords": [ "sanity", "ui", @@ -110,6 +110,7 @@ "@sanity/icons": "^3.4.0", "csstype": "^3.1.3", "framer-motion": "11.0.8", + "react-compiler-runtime": "19.0.0-beta-6fc168f-20241025", "react-refractor": "^2.2.0", "use-effect-event": "^1.0.2" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 476c463fd..ac0ee35b5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -26,6 +26,9 @@ importers: framer-motion: specifier: 11.0.8 version: 11.0.8(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + react-compiler-runtime: + specifier: 19.0.0-beta-6fc168f-20241025 + version: 19.0.0-beta-6fc168f-20241025(react@18.3.1) react-refractor: specifier: ^2.2.0 version: 2.2.0(react@18.3.1) @@ -65,7 +68,7 @@ importers: version: 5.0.0(semantic-release@24.0.0(typescript@5.5.3)) '@sanity/ui-workshop': specifier: ^2.0.16 - version: 2.0.16(@sanity/icons@3.4.0(react@18.3.1))(@sanity/ui@2.8.11(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1)))(@types/node@20.12.7)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1))(terser@5.30.3) + version: 2.0.16(@sanity/icons@3.4.0(react@18.3.1))(@sanity/ui@2.8.12(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1)))(@types/node@20.12.7)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1))(terser@5.30.3) '@storybook/addon-a11y': specifier: ^8.3.6 version: 8.3.6(storybook@8.3.6) @@ -225,9 +228,6 @@ importers: react: specifier: ^18.3.1 version: 18.3.1 - react-compiler-runtime: - specifier: beta - version: 19.0.0-beta-6fc168f-20241025(react@18.3.1) react-dom: specifier: ^18.3.1 version: 18.3.1(react@18.3.1) @@ -1967,8 +1967,8 @@ packages: react-dom: ^18 styled-components: ^5.2 || ^6 - '@sanity/ui@2.8.11': - resolution: {integrity: sha512-5PAWadS6bXBnV+dADChQATcrHNlsuKZa67PxdPxSO3v3TDVzm4GXYSpSvEz9GuOKCotyK17vze8WSCEVuY7OYg==} + '@sanity/ui@2.8.12': + resolution: {integrity: sha512-SnmHh1kQavLqoGnZB37KoWkMQWDyhJWfT6Aa3BJtPqY8RN59eXc9tnZXpjGyDIsjqqhdq/r4aHQlZgHBW9g3uA==} engines: {node: '>=14.0.0'} peerDependencies: react: ^18 @@ -9603,10 +9603,10 @@ snapshots: transitivePeerDependencies: - supports-color - '@sanity/ui-workshop@2.0.16(@sanity/icons@3.4.0(react@18.3.1))(@sanity/ui@2.8.11(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1)))(@types/node@20.12.7)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1))(terser@5.30.3)': + '@sanity/ui-workshop@2.0.16(@sanity/icons@3.4.0(react@18.3.1))(@sanity/ui@2.8.12(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1)))(@types/node@20.12.7)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1))(terser@5.30.3)': dependencies: '@sanity/icons': 3.4.0(react@18.3.1) - '@sanity/ui': 2.8.11(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1)) + '@sanity/ui': 2.8.12(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1)) '@vitejs/plugin-react': 4.3.3(vite@5.4.2(@types/node@20.12.7)(terser@5.30.3)) axe-core: 4.10.2 cac: 6.7.14 @@ -9637,7 +9637,7 @@ snapshots: - supports-color - terser - '@sanity/ui@2.8.11(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1))': + '@sanity/ui@2.8.12(react-dom@18.3.1(react@18.3.1))(react-is@18.3.1)(react@18.3.1)(styled-components@6.1.13(react-dom@18.3.1(react@18.3.1))(react@18.3.1))': dependencies: '@floating-ui/react-dom': 2.1.2(react-dom@18.3.1(react@18.3.1))(react@18.3.1) '@sanity/color': 3.0.6 diff --git a/src/core/components/autocomplete/autocomplete.tsx b/src/core/components/autocomplete/autocomplete.tsx index 9677cc5d9..94b121d1b 100644 --- a/src/core/components/autocomplete/autocomplete.tsx +++ b/src/core/components/autocomplete/autocomplete.tsx @@ -17,6 +17,7 @@ import { useMemo, useReducer, useRef, + useState, } from 'react' import {EMPTY_ARRAY, EMPTY_RECORD} from '../../constants' import {_hasFocus, _raf, focusFirstDescendant} from '../../helpers' @@ -184,21 +185,22 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< typeof filterOptionProp === 'function' ? filterOptionProp : DEFAULT_FILTER_OPTION // Element refs - const rootElementRef = useRef(null) - const resultsPopoverElementRef = useRef(null) - const inputElementRef = useRef(null) + const [rootElement, setRootElement] = useState(null) + const [resultsPopoverElement, setResultsPopoverElement] = useState(null) + const [inputElement, setInputElement] = useState(null) const listBoxElementRef = useRef(null) // Value refs const listFocusedRef = useRef(false) const valueRef = useRef(value) const valuePropRef = useRef(valueProp) - const popoverMouseWithinRef = useRef(false) + const [popoverMouseWithin, setPopoverMouseWithin] = useState(false) // Forward ref to parent useImperativeHandle( forwardedRef, - () => inputElementRef.current, + () => inputElement, + [inputElement], ) const listBoxId = `${id}-listbox` @@ -222,13 +224,13 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< // NOTE: This is a workaround for a bug that may happen in Chrome (clicking the scrollbar // closes the results in certain situations): // - Do not handle blur if the mouse is within the popover - if (popoverMouseWithinRef.current) { + if (popoverMouseWithin) { return } const elements: HTMLElement[] = (relatedElements || []).concat( - rootElementRef.current ? [rootElementRef.current] : [], - resultsPopoverElementRef.current ? [resultsPopoverElementRef.current] : [], + rootElement ? [rootElement] : [], + resultsPopoverElement ? [resultsPopoverElement] : [], ) let focusInside = false @@ -244,13 +246,20 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< if (focusInside === false) { dispatch({type: 'root/blur'}) - popoverMouseWithinRef.current = false + setPopoverMouseWithin(false) if (onQueryChange) onQueryChange(null) if (onBlur) onBlur(event) } }, 0) }, - [onBlur, onQueryChange, relatedElements], + [ + onBlur, + onQueryChange, + popoverMouseWithin, + relatedElements, + resultsPopoverElement, + rootElement, + ], ) const handleRootFocus = useCallback((event: FocusEvent) => { @@ -269,7 +278,7 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< (v: string) => { dispatch({type: 'value/change', value: v}) - popoverMouseWithinRef.current = false + setPopoverMouseWithin(false) if (onSelect) onSelect(v) @@ -278,9 +287,9 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< if (onChange) onChange(v) if (onQueryChange) onQueryChange(null) - inputElementRef.current?.focus() + inputElement?.focus() }, - [onChange, onSelect, onQueryChange], + [onSelect, onChange, onQueryChange, inputElement], ) const handleRootKeyDown = useCallback( @@ -324,9 +333,9 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< if (event.key === 'Escape') { dispatch({type: 'root/escape'}) - popoverMouseWithinRef.current = false + setPopoverMouseWithin(false) if (onQueryChange) onQueryChange(null) - inputElementRef.current?.focus() + inputElement?.focus() return } @@ -338,12 +347,12 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< (listEl === target || listEl?.contains(target)) && !AUTOCOMPLETE_LISTBOX_IGNORE_KEYS.includes(event.key) ) { - inputElementRef.current?.focus() + inputElement?.focus() return } }, - [activeValue, filteredOptions, filteredOptionsLen, onQueryChange], + [activeValue, filteredOptions, filteredOptionsLen, inputElement, onQueryChange], ) const handleInputChange = useCallback( @@ -377,11 +386,11 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< ) const handlePopoverMouseEnter = useCallback(() => { - popoverMouseWithinRef.current = true + setPopoverMouseWithin(true) }, []) const handlePopoverMouseLeave = useCallback(() => { - popoverMouseWithinRef.current = false + setPopoverMouseWithin(false) }, []) const handleClearButtonClick = useCallback(() => { @@ -389,8 +398,8 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< valueRef.current = '' if (onChange) onChange('') if (onQueryChange) onQueryChange(null) - inputElementRef.current?.focus() - }, [onChange, onQueryChange]) + inputElement?.focus() + }, [inputElement, onChange, onQueryChange]) const handleClearButtonFocus = useCallback(() => { dispatch({type: 'input/focus'}) @@ -482,9 +491,9 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< if (openButtonProps.onClick) openButtonProps.onClick(event) - _raf(() => inputElementRef.current?.focus()) + _raf(() => inputElement?.focus()) }, - [openButtonProps, dispatchOpen], + [dispatchOpen, openButtonProps, inputElement], ) const openButtonNode = useMemo( @@ -554,7 +563,7 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< prefix={prefix} radius={radius} readOnly={readOnly} - ref={inputElementRef} + ref={setInputElement} role="combobox" spellCheck={false} suffix={suffix || openButtonNode} @@ -566,10 +575,10 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< (event: KeyboardEvent) => { // If the focus is currently in the list, move focus to the input element if (event.key === 'Tab') { - if (listFocused) inputElementRef.current?.focus() + if (listFocused) inputElement?.focus() } }, - [listFocused], + [inputElement, listFocused], ) const content = useMemo(() => { @@ -635,11 +644,11 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< { content, hidden: !expanded, - inputElement: inputElementRef.current, + inputElement, onMouseEnter: handlePopoverMouseEnter, onMouseLeave: handlePopoverMouseLeave, }, - resultsPopoverElementRef, + {current: resultsPopoverElement}, ) } @@ -661,8 +670,8 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< placement={AUTOCOMPLETE_POPOVER_PLACEMENT} portal radius={radius} - ref={resultsPopoverElementRef} - referenceElement={inputElementRef.current} + ref={setResultsPopoverElement} + referenceElement={inputElement} {...popover} /> ) @@ -672,9 +681,11 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< filteredOptionsLen, handlePopoverMouseEnter, handlePopoverMouseLeave, + inputElement, popover, radius, renderPopover, + resultsPopoverElement, ]) return ( @@ -683,7 +694,7 @@ const InnerAutocomplete = forwardRef(function InnerAutocomplete< onBlur={handleRootBlur} onFocus={handleRootFocus} onKeyDown={handleRootKeyDown} - ref={rootElementRef} + ref={setRootElement} > {input} {results} diff --git a/src/core/components/dialog/dialog.tsx b/src/core/components/dialog/dialog.tsx index 0ed6ab35b..d93fdc88b 100644 --- a/src/core/components/dialog/dialog.tsx +++ b/src/core/components/dialog/dialog.tsx @@ -305,13 +305,15 @@ export const Dialog = forwardRef(function Dialog( onFocus, padding: paddingProp = 3, portal: portalProp, - position: positionProp = dialog.position || 'fixed', + position: _positionProp, scheme, width: widthProp = 0, - zOffset: zOffsetProp = dialog.zOffset || layer.dialog.zOffset, + zOffset: _zOffsetProp, animate: _animate = false, ...restProps } = props + const positionProp = _positionProp ?? (dialog.position || 'fixed') + const zOffsetProp = _zOffsetProp ?? (dialog.zOffset || layer.dialog.zOffset) const prefersReducedMotion = usePrefersReducedMotion() const animate = prefersReducedMotion ? false : _animate const portal = usePortal() diff --git a/src/core/components/menu/menu.tsx b/src/core/components/menu/menu.tsx index f13dbc79e..def9b01b7 100644 --- a/src/core/components/menu/menu.tsx +++ b/src/core/components/menu/menu.tsx @@ -57,10 +57,12 @@ export const Menu = forwardRef(function Menu( originElement, padding = 1, registerElement, - shouldFocus = (props.focusFirst && 'first') || (props.focusLast && 'last') || null, + shouldFocus: _shouldFocus, space = 1, ...restProps } = props + const shouldFocus = + _shouldFocus ?? ((props.focusFirst && 'first') || (props.focusLast && 'last') || null) const ref = useRef(null) diff --git a/src/core/components/menu/menuGroup.tsx b/src/core/components/menu/menuGroup.tsx index ed76aeb81..99abd4258 100644 --- a/src/core/components/menu/menuGroup.tsx +++ b/src/core/components/menu/menuGroup.tsx @@ -53,9 +53,10 @@ export function MenuGroup( onClickOutside, onEscape, onItemClick, - onItemMouseEnter = menu.onMouseEnter, + onItemMouseEnter: _onItemMouseEnter, registerElement, } = menu + const onItemMouseEnter = _onItemMouseEnter ?? menu.onMouseEnter const [rootElement, setRootElement] = useState(null) const [open, setOpen] = useState(false) const [shouldFocus, setShouldFocus] = useState<'first' | 'last' | null>(null) diff --git a/src/core/components/menu/menuItem.tsx b/src/core/components/menu/menuItem.tsx index accf543f1..d1abcf69e 100644 --- a/src/core/components/menu/menuItem.tsx +++ b/src/core/components/menu/menuItem.tsx @@ -72,9 +72,11 @@ export const MenuItem = forwardRef(function MenuItem( activeElement, mount, onItemClick, - onItemMouseEnter = menu.onMouseEnter, - onItemMouseLeave = menu.onMouseLeave, + onItemMouseEnter: _onItemMouseEnter, + onItemMouseLeave: _onItemMouseLeave, } = menu + const onItemMouseEnter = _onItemMouseEnter ?? menu.onMouseEnter + const onItemMouseLeave = _onItemMouseLeave ?? menu.onMouseLeave const [rootElement, setRootElement] = useState(null) const active = Boolean(activeElement) && activeElement === rootElement const ref = useRef(null) diff --git a/src/core/components/menu/useMenuController.ts b/src/core/components/menu/useMenuController.ts index dd4abbbcb..92d446030 100644 --- a/src/core/components/menu/useMenuController.ts +++ b/src/core/components/menu/useMenuController.ts @@ -36,31 +36,28 @@ export function useMenuController(props: { activeIndexRef.current = nextActiveIndex }, []) - const mount = useCallback( - (element: HTMLElement | null, selected?: boolean): (() => void) => { - if (!element) return () => undefined + const mount = (element: HTMLElement | null, selected?: boolean): (() => void) => { + if (!element) return () => undefined - if (elementsRef.current.indexOf(element) === -1) { - elementsRef.current.push(element) - _sortElements(rootElementRef.current, elementsRef.current) - } + if (elementsRef.current.indexOf(element) === -1) { + elementsRef.current.push(element) + _sortElements(rootElementRef.current, elementsRef.current) + } - if (selected) { - const selectedIndex = elementsRef.current.indexOf(element) + if (selected) { + const selectedIndex = elementsRef.current.indexOf(element) - setActiveIndex(selectedIndex) - } + setActiveIndex(selectedIndex) + } - return () => { - const idx = elementsRef.current.indexOf(element) + return () => { + const idx = elementsRef.current.indexOf(element) - if (idx > -1) { - elementsRef.current.splice(idx, 1) - } + if (idx > -1) { + elementsRef.current.splice(idx, 1) } - }, - [rootElementRef, setActiveIndex], - ) + } + } const handleKeyDown = useCallback( (event: React.KeyboardEvent) => { @@ -170,7 +167,7 @@ export function useMenuController(props: { [setActiveIndex], ) - const handleItemMouseLeave = useCallback(() => { + const handleItemMouseLeave = () => { // Set the active index to -2 to deactivate all menu items // when the user moves the mouse away from the menu item. // We avoid using -1 because it would focus the first menu item, @@ -178,7 +175,7 @@ export function useMenuController(props: { // between two menu items or a menu divider. setActiveIndex(-2) rootElementRef.current?.focus() - }, [rootElementRef, setActiveIndex]) + } // Set focus on the currently active element useEffect(() => { diff --git a/src/core/components/tree/treeItem.tsx b/src/core/components/tree/treeItem.tsx index a80635071..725c9f305 100644 --- a/src/core/components/tree/treeItem.tsx +++ b/src/core/components/tree/treeItem.tsx @@ -1,6 +1,6 @@ import {ToggleArrowRightIcon} from '@sanity/icons' import {ThemeFontWeightKey} from '@sanity/ui/theme' -import {memo, useCallback, useEffect, useId, useMemo, useRef} from 'react' +import {memo, useCallback, useEffect, useId, useMemo, useRef, useState} from 'react' import {styled} from 'styled-components' import {Box, BoxProps, Flex, Text} from '../../primitives' import { @@ -64,7 +64,7 @@ export const TreeItem = memo(function TreeItem( weight, ...restProps } = props - const rootRef = useRef(null) + const [rootElement, setRootElement] = useState(null) const treeitemRef = useRef(null) const tree = useTree() const {path, registerItem, setExpanded, setFocusedElement} = tree @@ -73,9 +73,9 @@ export const TreeItem = memo(function TreeItem( const itemPath = useMemo(() => path.concat([id || '']), [id, path]) const itemKey = itemPath.join('/') const itemState = tree.state[itemKey] - const focused = tree.focusedElement === rootRef.current + const focused = tree.focusedElement === rootElement const expanded = itemState?.expanded === undefined ? expandedProp : itemState?.expanded || false - const tabIndex = tree.focusedElement && tree.focusedElement === rootRef.current ? 0 : -1 + const tabIndex = tree.focusedElement && tree.focusedElement === rootElement ? 0 : -1 const contextValue = useMemo( () => ({...tree, level: tree.level + 1, path: itemPath}), [itemPath, tree], @@ -94,28 +94,28 @@ export const TreeItem = memo(function TreeItem( ) { event.stopPropagation() setExpanded(itemKey, !expanded) - setFocusedElement(rootRef.current) + setFocusedElement(rootElement) } }, - [expanded, itemKey, onClick, setExpanded, setFocusedElement], + [expanded, itemKey, onClick, rootElement, setExpanded, setFocusedElement], ) const handleKeyDown = useCallback( (event: React.KeyboardEvent) => { if (focused && event.key === 'Enter') { - const el = treeitemRef.current || rootRef.current + const el = treeitemRef.current || rootElement el?.click() } }, - [focused], + [focused, rootElement], ) useEffect(() => { - if (!rootRef.current) return + if (!rootElement) return - return registerItem(rootRef.current, itemPath.join('/'), expanded, selected) - }, [expanded, itemPath, registerItem, selected]) + return registerItem(rootElement, itemPath.join('/'), expanded, selected) + }, [expanded, itemPath, registerItem, rootElement, selected]) const content = ( @@ -154,7 +154,7 @@ export const TreeItem = memo(function TreeItem( data-ui="TreeItem" {...restProps} onClick={handleClick} - ref={rootRef} + ref={setRootElement} role="none" > diff --git a/src/core/hooks/useArrayProp.ts b/src/core/hooks/useArrayProp.ts index f5d4371ae..fe6855d1e 100644 --- a/src/core/hooks/useArrayProp.ts +++ b/src/core/hooks/useArrayProp.ts @@ -1,4 +1,4 @@ -import {useMemo} from 'react' +import {useState} from 'react' import {_getArrayProp} from '../styles' /** @beta */ @@ -12,14 +12,19 @@ export function useArrayProp( val: T | T[] | undefined, defaultVal?: T[], ): T[] { - // JSON.stringify is fast, but it's not faster than useMemo's referencial equality check - const __perf_hash__ = useMemo(() => JSON.stringify(val ?? defaultVal), [defaultVal, val]) + const [[cachedVal, cachedHash], setCache] = useState<[T[], string]>(() => [ + _getArrayProp(val, defaultVal), + JSON.stringify(val ?? defaultVal), + ]) - return useMemo( - () => _getArrayProp(val, defaultVal), + const hash = JSON.stringify(val ?? defaultVal) - // Improve performance: Keep object identify for a given hash of the value - // eslint-disable-next-line react-hooks/exhaustive-deps - [__perf_hash__], - ) + if (hash !== cachedHash) { + // If the cached hash has changed, update the cache right away. + // Calling setState during render is fine in this case, and preferred over a useEffect loop + // https://19.react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes + setCache([_getArrayProp(val, defaultVal), hash]) + } + + return cachedVal } diff --git a/src/core/hooks/useMatchMedia.ts b/src/core/hooks/useMatchMedia.ts index c155698ef..f13926c98 100644 --- a/src/core/hooks/useMatchMedia.ts +++ b/src/core/hooks/useMatchMedia.ts @@ -11,34 +11,24 @@ export function useMatchMedia( mediaQueryString: `(${string})`, getServerSnapshot?: () => boolean, ): boolean { + /** + * `subscribe` and `getSnapshot` are only called on the client and both need access to the same `matchMedia` instance + * we don't want to eagerly instantiate it to ensure it's only created when actually used + */ + const cachedMatchMedia = useMemo( + () => (typeof window === 'undefined' ? null : window.matchMedia(mediaQueryString)), + [mediaQueryString], + ) const {subscribe, getSnapshot} = useMemo(() => { - /** - * `subscribe` and `getSnapshot` are only called on the client and both need access to the same `matchMedia` instance - * we don't want to eagerly instantiate it to ensure it's only created when actually used - */ - let MEDIA_QUERY_CACHE: MediaQueryList | undefined - - const getMatchMedia = (): MediaQueryList => { - if (!MEDIA_QUERY_CACHE) { - // As this function is only called during `subscribe` and `getSnapshot`, we can assume that the - // the `window` global is available and we're in a browser environment - MEDIA_QUERY_CACHE = window.matchMedia(mediaQueryString) - } - - return MEDIA_QUERY_CACHE - } - return { subscribe: (onStoreChange: () => void): (() => void) => { - const matchMedia = getMatchMedia() - - matchMedia.addEventListener('change', onStoreChange) + cachedMatchMedia!.addEventListener('change', onStoreChange) - return () => matchMedia.removeEventListener('change', onStoreChange) + return () => cachedMatchMedia!.removeEventListener('change', onStoreChange) }, - getSnapshot: () => getMatchMedia().matches, + getSnapshot: () => cachedMatchMedia!.matches, } - }, [mediaQueryString]) + }, [cachedMatchMedia]) useDebugValue(mediaQueryString) diff --git a/src/core/primitives/avatar/avatarStack.tsx b/src/core/primitives/avatar/avatarStack.tsx index a0db0cc60..dada7effa 100644 --- a/src/core/primitives/avatar/avatarStack.tsx +++ b/src/core/primitives/avatar/avatarStack.tsx @@ -1,5 +1,5 @@ import {getTheme_v2} from '@sanity/ui/theme' -import {Children, cloneElement, forwardRef, isValidElement, useMemo} from 'react' +import {Children, cloneElement, forwardRef, isValidElement} from 'react' import {styled, css} from 'styled-components' import {EMPTY_RECORD} from '../../constants' import {useArrayProp} from '../../hooks' @@ -65,10 +65,7 @@ export const AvatarStack = forwardRef(function AvatarStack( size: sizeProp = 1, ...restProps } = props - const children = useMemo( - () => Children.toArray(childrenProp).filter(isValidElement), - [childrenProp], - ) + const children: React.ReactElement[] = Children.toArray(childrenProp).filter(isValidElement) const maxLength = Math.max(maxLengthProp, 0) const size = useArrayProp(sizeProp) diff --git a/src/core/primitives/popover/popover.tsx b/src/core/primitives/popover/popover.tsx index 64bbbc65d..89212bebd 100644 --- a/src/core/primitives/popover/popover.tsx +++ b/src/core/primitives/popover/popover.tsx @@ -21,8 +21,10 @@ import { useCallback, useEffect, useImperativeHandle, + useLayoutEffect, useMemo, useRef, + useState, } from 'react' import {useArrayProp, useElementSize, useMediaIndex, usePrefersReducedMotion} from '../../hooks' import {origin} from '../../middleware/origin' @@ -126,15 +128,14 @@ export const Popover = memo( __unstable_margins: margins = DEFAULT_POPOVER_MARGINS, animate: _animate = false, arrow: arrowProp = false, - boundaryElement = boundaryElementContext.element, + boundaryElement: _boundaryElement, children: childProp, constrainSize = false, content, disabled, - fallbackPlacements = props.fallbackPlacements ?? - DEFAULT_FALLBACK_PLACEMENTS[props.placement ?? 'bottom'], + fallbackPlacements: _fallbackPlacements, matchReferenceWidth, - floatingBoundary = props.boundaryElement ?? boundaryElementContext.element, + floatingBoundary: _floatingBoundary, // eslint-disable-next-line @typescript-eslint/no-unused-vars onActivate, open, @@ -144,16 +145,23 @@ export const Popover = memo( portal, preventOverflow = true, radius: radiusProp = 3, - referenceBoundary = props.boundaryElement ?? boundaryElementContext.element, + referenceBoundary: _referenceBoundary, referenceElement, scheme, shadow: shadowProp = 3, tone = 'inherit', width: widthProp = 'auto', - zOffset: zOffsetProp = layer.popover.zOffset, + zOffset: _zOffsetProp, updateRef, ...restProps } = props + const boundaryElement = _boundaryElement ?? boundaryElementContext.element + const fallbackPlacements = + _fallbackPlacements ?? DEFAULT_FALLBACK_PLACEMENTS[props.placement ?? 'bottom'] + const floatingBoundary = _floatingBoundary ?? boundaryElement ?? boundaryElementContext.element + const referenceBoundary = + _referenceBoundary ?? boundaryElement ?? boundaryElementContext.element + const zOffsetProp = _zOffsetProp ?? layer.popover.zOffset const prefersReducedMotion = usePrefersReducedMotion() const animate = prefersReducedMotion ? false : _animate const boundarySize = useElementSize(boundaryElement)?.border @@ -202,7 +210,7 @@ export const Popover = memo( const referenceWidthRef = useRef() // Force apply width & max width to floating element - useEffect(() => { + useLayoutEffect(() => { const floatingElement = ref.current if (!open || !floatingElement) return @@ -210,7 +218,7 @@ export const Popover = memo( const referenceWidth = referenceWidthRef.current if (matchReferenceWidth) { - if (referenceWidth !== undefined) { + if (referenceWidthRef.current !== undefined) { floatingElement.style.width = `${referenceWidth}px` } } else if (width !== undefined) { @@ -350,36 +358,24 @@ export const Popover = memo( [refs], ) + const [childElement, setChildElement] = useState(null) const setReference = useCallback( (node: HTMLElement | null) => { refs.setReference(node) - - const childRef = getElementRef(childProp as any) - - if (typeof childRef === 'function') { - childRef(node) - } else if (childRef) { - childRef.current = node - } + setChildElement(node) }, - [childProp, refs], + [refs], ) + useImperativeHandle(getElementRef(childProp as any), () => childElement, [childElement]) + const child = useMemo(() => { if (!childProp || referenceElement) return null return cloneElement(childProp, {ref: setReference}) }, [childProp, referenceElement, setReference]) - useEffect(() => { - if (updateRef) { - if (typeof updateRef === 'function') { - updateRef(update) - } else if (updateRef) { - updateRef.current = update - } - } - }, [update, updateRef]) + useImperativeHandle(updateRef, () => update, [update]) useEffect(() => { if (child) return diff --git a/src/core/primitives/tooltip/tooltip.tsx b/src/core/primitives/tooltip/tooltip.tsx index 7f3a916f0..c8c00383a 100644 --- a/src/core/primitives/tooltip/tooltip.tsx +++ b/src/core/primitives/tooltip/tooltip.tsx @@ -97,22 +97,25 @@ export const Tooltip = forwardRef(function Tooltip( const { animate: _animate = false, arrow: arrowProp = false, - boundaryElement = boundaryElementContext?.element, + boundaryElement: _boundaryElement, children: childProp, content, disabled, - fallbackPlacements: fallbackPlacementsProp = props.fallbackPlacements ?? - DEFAULT_FALLBACK_PLACEMENTS[props.placement ?? 'bottom'], + fallbackPlacements: _fallbackPlacementsProp, padding = 2, placement: placementProp = 'bottom', portal: portalProp, radius = 2, scheme, shadow = 2, - zOffset = layer.tooltip.zOffset, + zOffset: _zOffset, delay, ...restProps } = props + const boundaryElement = _boundaryElement ?? boundaryElementContext?.element + const fallbackPlacementsProp = + _fallbackPlacementsProp ?? DEFAULT_FALLBACK_PLACEMENTS[props.placement ?? 'bottom'] + const zOffset = _zOffset ?? layer.tooltip.zOffset const prefersReducedMotion = usePrefersReducedMotion() const animate = prefersReducedMotion ? false : _animate const fallbackPlacements = useArrayProp(fallbackPlacementsProp) diff --git a/src/core/theme/themeProvider.tsx b/src/core/theme/themeProvider.tsx index 3864b1adc..ccda05dac 100644 --- a/src/core/theme/themeProvider.tsx +++ b/src/core/theme/themeProvider.tsx @@ -25,12 +25,10 @@ export interface ThemeProviderProps { */ export function ThemeProvider(props: ThemeProviderProps): React.ReactElement { const parentTheme = useContext(ThemeContext) - const { - children, - scheme = parentTheme?.scheme || 'light', - theme: rootTheme = parentTheme?.theme || null, - tone = parentTheme?.tone || 'default', - } = props + const {children} = props + const scheme = props.scheme ?? (parentTheme?.scheme || 'light') + const rootTheme = props.theme ?? (parentTheme?.theme || null) + const tone = props.tone ?? (parentTheme?.tone || 'default') const themeContext: ThemeContextValue | null = useMemo(() => { if (!rootTheme) return null diff --git a/src/core/utils/elementQuery/elementQuery.tsx b/src/core/utils/elementQuery/elementQuery.tsx index 3c555efb3..88b2eee15 100644 --- a/src/core/utils/elementQuery/elementQuery.tsx +++ b/src/core/utils/elementQuery/elementQuery.tsx @@ -21,7 +21,8 @@ export const ElementQuery = forwardRef(function ElementQuery( forwardedRef: React.ForwardedRef, ) { const theme = useTheme_v2() - const {children, media = theme.media, ...restProps} = props + const {children, media: _media, ...restProps} = props + const media = _media ?? theme.media const ref = useRef(null) const [element, setElement] = useState(null) diff --git a/src/core/utils/portal/portalProvider.tsx b/src/core/utils/portal/portalProvider.tsx index 41918f7d2..3da51f198 100644 --- a/src/core/utils/portal/portalProvider.tsx +++ b/src/core/utils/portal/portalProvider.tsx @@ -1,4 +1,4 @@ -import {useMemo, useRef, useSyncExternalStore} from 'react' +import {useMemo, useState, useSyncExternalStore} from 'react' import {PortalContext} from './portalContext' import {PortalContextValue} from './types' @@ -51,13 +51,13 @@ const emptySubscribe = () => () => {} * equality comparison (eg by identity), and only goes one level deep. */ function useUnique(value: ValueType): ValueType { - const valueRef = useRef(value) + const [cachedValue, setCachedValue] = useState(value) - if (!_isEqual(valueRef.current, value)) { - valueRef.current = value + if (!_isEqual(cachedValue, value)) { + setCachedValue(value) } - return valueRef.current + return cachedValue } function _isEqual(objA: Comparable, objB: Comparable): boolean {