diff --git a/packages/@react-aria/utils/src/useObjectRef.ts b/packages/@react-aria/utils/src/useObjectRef.ts index 0dc377ba39f..6309f25d8d1 100644 --- a/packages/@react-aria/utils/src/useObjectRef.ts +++ b/packages/@react-aria/utils/src/useObjectRef.ts @@ -10,7 +10,8 @@ * governing permissions and limitations under the License. */ -import {MutableRefObject, useEffect, useRef} from 'react'; +import {MutableRefObject, useRef} from 'react'; +import {useLayoutEffect} from './'; /** * Offers an object ref for a given callback ref or an object ref. Especially @@ -24,7 +25,12 @@ import {MutableRefObject, useEffect, useRef} from 'react'; export function useObjectRef(forwardedRef?: ((instance: T | null) => void) | MutableRefObject | null): MutableRefObject { const objRef = useRef(); - useEffect(() => { + /** + * We're using `useLayoutEffect` here instead of `useEffect` because we want + * to make sure that the `ref` value is up to date before other places in the + * the execution cycle try to read it. + */ + useLayoutEffect(() => { if (!forwardedRef) { return; } diff --git a/packages/@react-aria/utils/test/useObjectRef.test.js b/packages/@react-aria/utils/test/useObjectRef.test.js index ee32fcad2ba..d8c8e7980d2 100644 --- a/packages/@react-aria/utils/test/useObjectRef.test.js +++ b/packages/@react-aria/utils/test/useObjectRef.test.js @@ -10,33 +10,18 @@ * governing permissions and limitations under the License. */ -import React from 'react'; -import {render} from '@testing-library/react'; +import React, {useEffect, useLayoutEffect} from 'react'; +import {render, screen} from '@testing-library/react'; import {renderHook} from '@testing-library/react-hooks'; import {useObjectRef} from '../'; describe('useObjectRef', () => { - it('should return an object ref by default', () => { + it('returns an empty object ref by default', () => { const {result} = renderHook(() => useObjectRef()); - expect(result.current.current).not.toBe(null); - }); - - it('should return an object ref for an object ref', () => { - const ref = React.createRef(); - - const {result} = renderHook(() => useObjectRef(ref)); - - expect(result.current.current).toBe(ref.current); - }); - - it('should return an object ref for a function ref', () => { - let inputElem; - const ref = (el) => inputElem = el; - - const {result} = renderHook(() => useObjectRef(ref)); - - expect(result.current.current).toBe(inputElem); + expect(result.current).toBeDefined(); + expect(result.current).not.toBeNull(); + expect(result.current.current).toBeUndefined(); }); it('should support React.forwardRef for an object ref', () => { @@ -66,4 +51,88 @@ describe('useObjectRef', () => { expect(inputElem.placeholder).toBe('Foo'); }); + + /** + * This describe would completely fail if `useObjectRef` did not account + * for order of execution and rendering, especially when other components + * or Hooks utilize the `useLayoutEffect` Hook. In other words, it guards + * against use-cases where the returned `ref` value may not be up to date. + */ + describe('when considering rendering order', () => { + const LeaderTextField = React.forwardRef((props, forwardedRef) => { + const inputRef = useObjectRef(forwardedRef); + + return ; + }); + + it('takes precedence over useEffect', () => { + const FollowerTextField = React.forwardRef((props, forwardedRef) => { + const inputRef = React.useRef(); + + useEffect(() => { + forwardedRef.current && (inputRef.current.placeholder = forwardedRef.current.placeholder); + }, [forwardedRef]); + + return ; + }); + + const Example = () => { + const outerRef = React.useRef(); + + /** + * Order of the following should not matter. So, even though the first + * component has a "Bar" placeholder, both will end up having the same + * placeholder text "Foo" because `outerRef` was forwarded to + * `LeaderTextField` and got updated by `useObjectRef` before + * `FollowerTextField` executed its `useEffect`. + */ + return ( + <> + + + + ); + }; + + render(); + + expect(screen.getAllByPlaceholderText(/foo/i)).toHaveLength(2); + }); + + it('batches up with useLayoutEffect', () => { + const FollowerTextField = React.forwardRef((props, forwardedRef) => { + const inputRef = React.useRef(); + + useLayoutEffect(() => { + forwardedRef.current && (inputRef.current.placeholder = forwardedRef.current.placeholder); + }, [forwardedRef]); + + return ; + }); + + const Example = () => { + const outerRef = React.useRef(); + + /** + * Order of the following _does_ matter because `FollowerTextField` + * this time has a `useLayoutEffect`, which is synchronous and is + * executed in the order it was called. But, still, both will end + * up having the same placeholder text "Foo" because `outerRef` is + * forwarded to `LeaderTextField`, which, in this test, comes _before_ + * `FollowerTextField`. Hence, `outerRef` gets updated by + * `useObjectRef`, so `FollowerTextField` gets the updated `ref` value. + */ + return ( + <> + + + + ); + }; + + render(); + + expect(screen.getAllByPlaceholderText(/foo/i)).toHaveLength(2); + }); + }); });