From 061e452441cd21c752eb795774b021e116dbace7 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 8 Sep 2023 21:05:13 +0000 Subject: [PATCH 1/3] use isomorphic layout effect instead of layout effect --- .eslintrc.js | 6 ++++++ .../InlineAutocomplete/InlineAutocomplete.test.tsx | 5 +++-- src/drafts/MarkdownEditor/MarkdownEditor.tsx | 14 +++----------- src/drafts/hooks/useCombobox.ts | 5 +++-- src/drafts/hooks/useDynamicTextareaHeight.ts | 5 +++-- src/drafts/hooks/useSafeAsyncCallback.ts | 5 +++-- src/hooks/useId.ts | 3 +-- src/utils/useIsomorphicLayoutEffect.ts | 1 + 8 files changed, 23 insertions(+), 21 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index c9356e66c54..b8784d93e39 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -143,6 +143,12 @@ module.exports = { importNames: ['useSSRSafeId'], message: 'Please use the `useId` hook from `src/hooks/useId.ts` instead', }, + { + name: 'react', + importNames: ['useLayoutEffect'], + message: + 'Please use the `useIsomorphicLayoutEffect` hook from `src/hooks/useIsomorphicLayoutEffect.ts` instead', + }, ], patterns: [ { diff --git a/src/drafts/InlineAutocomplete/InlineAutocomplete.test.tsx b/src/drafts/InlineAutocomplete/InlineAutocomplete.test.tsx index 5b090e5eb5d..d7c26c48908 100644 --- a/src/drafts/InlineAutocomplete/InlineAutocomplete.test.tsx +++ b/src/drafts/InlineAutocomplete/InlineAutocomplete.test.tsx @@ -1,4 +1,4 @@ -import React, {useLayoutEffect, useState} from 'react' +import React, {useState} from 'react' import {fireEvent, render, within} from '@testing-library/react' import userEvent from '@testing-library/user-event' import InlineAutocomplete, {ShowSuggestionsEvent, Suggestions, Trigger} from '.' @@ -6,6 +6,7 @@ import FormControl from '../../FormControl' import {ActionList} from '../../ActionList' import Textarea from '../../Textarea' import ThemeProvider from '../../ThemeProvider' +import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' const label = 'Inline Autocomplete' @@ -82,7 +83,7 @@ const UncontrolledInlineAutocomplete = ({ } } - useLayoutEffect(() => { + useIsomorphicLayoutEffect(() => { // combobox-nav attempts to filter out 'hidden' options by checking if the option has an // offsetHeight or width > 0. In JSDom, all elements have offsetHeight = offsetWidth = 0, // so we need to override at least one to make the class recognize that any options exist. diff --git a/src/drafts/MarkdownEditor/MarkdownEditor.tsx b/src/drafts/MarkdownEditor/MarkdownEditor.tsx index df8b2bd6bc7..50cb614c7e7 100644 --- a/src/drafts/MarkdownEditor/MarkdownEditor.tsx +++ b/src/drafts/MarkdownEditor/MarkdownEditor.tsx @@ -1,13 +1,4 @@ -import React, { - forwardRef, - useCallback, - useEffect, - useImperativeHandle, - useLayoutEffect, - useMemo, - useRef, - useState, -} from 'react' +import React, {forwardRef, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react' import Box from '../../Box' import VisuallyHidden from '../../_VisuallyHidden' import {useId} from '../../hooks/useId' @@ -36,6 +27,7 @@ import {Emoji} from './suggestions/_useEmojiSuggestions' import {Mentionable} from './suggestions/_useMentionSuggestions' import {Reference} from './suggestions/_useReferenceSuggestions' import {isModifierKey} from './utils' +import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' export type MarkdownEditorProps = SxProp & { /** Current value of the editor as a multiline markdown string. */ @@ -268,7 +260,7 @@ const MarkdownEditor = forwardRef( useResizeObserver(onResize, containerRef) // workaround for Safari bug where layout is otherwise not recalculated - useLayoutEffect(() => { + useIsomorphicLayoutEffect(() => { const container = containerRef.current if (!container) return diff --git a/src/drafts/hooks/useCombobox.ts b/src/drafts/hooks/useCombobox.ts index c821f069cf5..3bb10ba4ec2 100644 --- a/src/drafts/hooks/useCombobox.ts +++ b/src/drafts/hooks/useCombobox.ts @@ -1,6 +1,7 @@ import Combobox from '@github/combobox-nav' -import {useCallback, useEffect, useLayoutEffect, useRef, useState} from 'react' +import {useCallback, useEffect, useRef, useState} from 'react' import {useId} from '../../hooks/useId' +import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' export type ComboboxCommitEvent = { /** The underlying `combobox-commit` event. */ @@ -138,7 +139,7 @@ export const useCombobox = ({ [onCommit, list], ) - useLayoutEffect(() => { + useIsomorphicLayoutEffect(() => { const optionElements = getOptionElements() // Ensure each option has a unique ID (required by the Combobox class), but respect user provided IDs for (const [i, option] of optionElements.entries()) { diff --git a/src/drafts/hooks/useDynamicTextareaHeight.ts b/src/drafts/hooks/useDynamicTextareaHeight.ts index b6a1b2c44e0..f6ce3a55745 100644 --- a/src/drafts/hooks/useDynamicTextareaHeight.ts +++ b/src/drafts/hooks/useDynamicTextareaHeight.ts @@ -1,7 +1,8 @@ -import {RefObject, useCallback, useEffect, useLayoutEffect, useState} from 'react' +import {RefObject, useCallback, useEffect, useState} from 'react' import {SxProp} from '../../sx' import {getCharacterCoordinates} from '../utils/character-coordinates' +import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' type UseDynamicTextareaHeightSettings = { disabled?: boolean @@ -63,7 +64,7 @@ export const useDynamicTextareaHeight = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [minHeightLines, maxHeightLines, value, elementRef, disabled]) - useLayoutEffect(refreshHeight, [refreshHeight]) + useIsomorphicLayoutEffect(refreshHeight, [refreshHeight]) // With Slots, initial render of the component is delayed and so the initial layout effect can occur // before the target element has actually been calculated in the DOM. But if we only use regular effects, diff --git a/src/drafts/hooks/useSafeAsyncCallback.ts b/src/drafts/hooks/useSafeAsyncCallback.ts index b8f9f1aaa61..78b741dd7c1 100644 --- a/src/drafts/hooks/useSafeAsyncCallback.ts +++ b/src/drafts/hooks/useSafeAsyncCallback.ts @@ -1,4 +1,5 @@ -import {useCallback, useEffect, useLayoutEffect, useRef} from 'react' +import {useCallback, useEffect, useRef} from 'react' +import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' export const callbackCancelledResult = Symbol('callbackCancelledResult') export type CallbackCancelledResult = typeof callbackCancelledResult @@ -27,7 +28,7 @@ export const useSafeAsyncCallback = ( allowCallingAfterUnmount = false, ): ((...args: A) => R | CallbackCancelledResult) => { const trackingRef = useRef(fn) - useLayoutEffect(() => { + useIsomorphicLayoutEffect(() => { trackingRef.current = fn }, [fn]) diff --git a/src/hooks/useId.ts b/src/hooks/useId.ts index 06e5fcf8d24..8c8f843c1b4 100644 --- a/src/hooks/useId.ts +++ b/src/hooks/useId.ts @@ -1,5 +1,4 @@ -// eslint-disable-next-line import/no-namespace -import * as React from 'react' +import React from 'react' // eslint-disable-next-line no-restricted-imports import {useSSRSafeId} from '@react-aria/ssr' diff --git a/src/utils/useIsomorphicLayoutEffect.ts b/src/utils/useIsomorphicLayoutEffect.ts index b22e79d0da6..acd14253541 100644 --- a/src/utils/useIsomorphicLayoutEffect.ts +++ b/src/utils/useIsomorphicLayoutEffect.ts @@ -1,3 +1,4 @@ +// eslint-disable-next-line no-restricted-imports import {useEffect, useLayoutEffect} from 'react' const useIsomorphicLayoutEffect = From 9f7935149201b33f4d8dda9e9644ea0007c83053 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 8 Sep 2023 21:07:21 +0000 Subject: [PATCH 2/3] changeset --- .changeset/two-seals-prove.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/two-seals-prove.md diff --git a/.changeset/two-seals-prove.md b/.changeset/two-seals-prove.md new file mode 100644 index 00000000000..5ca8c009f35 --- /dev/null +++ b/.changeset/two-seals-prove.md @@ -0,0 +1,7 @@ +--- +'@primer/react': patch +--- + +use isomorphic layout effects only + + From a38d6e5838ebee2337eb7a0bfd4f31fad3cac6ec Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 8 Sep 2023 21:09:28 +0000 Subject: [PATCH 3/3] fix namespace --- src/hooks/useId.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hooks/useId.ts b/src/hooks/useId.ts index 8c8f843c1b4..b951b256b02 100644 --- a/src/hooks/useId.ts +++ b/src/hooks/useId.ts @@ -1,4 +1,5 @@ -import React from 'react' +// eslint-disable-next-line no-restricted-imports, import/no-namespace +import * as React from 'react' // eslint-disable-next-line no-restricted-imports import {useSSRSafeId} from '@react-aria/ssr'