Skip to content

Commit

Permalink
Use isomorphic layout effect everywhere (#3722)
Browse files Browse the repository at this point in the history
* use isomorphic layout effect instead of layout effect

* changeset

* fix namespace
  • Loading branch information
mattcosta7 authored Sep 11, 2023
1 parent 500e529 commit 0baa5a7
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 20 deletions.
7 changes: 7 additions & 0 deletions .changeset/two-seals-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/react': patch
---

use isomorphic layout effects only

<!-- Changed components: InlineAutocomplete, MarkdownEditor -->
6 changes: 6 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand Down
5 changes: 3 additions & 2 deletions src/drafts/InlineAutocomplete/InlineAutocomplete.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
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 '.'
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'

Expand Down Expand Up @@ -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.
Expand Down
14 changes: 3 additions & 11 deletions src/drafts/MarkdownEditor/MarkdownEditor.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -268,7 +260,7 @@ const MarkdownEditor = forwardRef<MarkdownEditorHandle, MarkdownEditorProps>(
useResizeObserver(onResize, containerRef)

// workaround for Safari bug where layout is otherwise not recalculated
useLayoutEffect(() => {
useIsomorphicLayoutEffect(() => {
const container = containerRef.current
if (!container) return

Expand Down
5 changes: 3 additions & 2 deletions src/drafts/hooks/useCombobox.ts
Original file line number Diff line number Diff line change
@@ -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<T> = {
/** The underlying `combobox-commit` event. */
Expand Down Expand Up @@ -138,7 +139,7 @@ export const useCombobox = <T>({
[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()) {
Expand Down
5 changes: 3 additions & 2 deletions src/drafts/hooks/useDynamicTextareaHeight.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions src/drafts/hooks/useSafeAsyncCallback.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -27,7 +28,7 @@ export const useSafeAsyncCallback = <A extends unknown[], R>(
allowCallingAfterUnmount = false,
): ((...args: A) => R | CallbackCancelledResult) => {
const trackingRef = useRef(fn)
useLayoutEffect(() => {
useIsomorphicLayoutEffect(() => {
trackingRef.current = fn
}, [fn])

Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useId.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// eslint-disable-next-line import/no-namespace
// 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'
Expand Down
1 change: 1 addition & 0 deletions src/utils/useIsomorphicLayoutEffect.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// eslint-disable-next-line no-restricted-imports
import {useEffect, useLayoutEffect} from 'react'

const useIsomorphicLayoutEffect =
Expand Down

0 comments on commit 0baa5a7

Please sign in to comment.