diff --git a/.changeset/validation-animation-derive-in-render.md b/.changeset/validation-animation-derive-in-render.md new file mode 100644 index 00000000000..c74bd167c4f --- /dev/null +++ b/.changeset/validation-animation-derive-in-render.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +FormControl: Avoid an extra re-render when showing a validation message. diff --git a/packages/react/src/internal/components/ValidationAnimationContainer.test.tsx b/packages/react/src/internal/components/ValidationAnimationContainer.test.tsx new file mode 100644 index 00000000000..bb7f2cec9d9 --- /dev/null +++ b/packages/react/src/internal/components/ValidationAnimationContainer.test.tsx @@ -0,0 +1,50 @@ +import {render} from '@testing-library/react' +import {describe, it, expect} from 'vitest' +import ValidationAnimationContainer from './ValidationAnimationContainer' +import {createRenderCounter} from '../../utils/testing/profiler' +import {implementsClassName} from '../../utils/testing' + +describe('ValidationAnimationContainer', () => { + implementsClassName(props => ) + + it('renders children when show is true', () => { + const {getByText} = render(content) + expect(getByText('content')).toBeInTheDocument() + }) + + it('does not render children when show is false initially', () => { + const {queryByText} = render(content) + expect(queryByText('content')).not.toBeInTheDocument() + }) + + it('renders children after show transitions to true', () => { + const {queryByText, rerender} = render( + content, + ) + expect(queryByText('content')).not.toBeInTheDocument() + + rerender(content) + expect(queryByText('content')).toBeInTheDocument() + }) + + it('commits once (no cascade) when show transitions to true', () => { + // The previous implementation set `shouldRender` from an effect, forcing an + // extra commit after the prop change. Deriving it during render keeps it to one. + const [Wrap, counter] = createRenderCounter() + const {rerender} = render( + + content + , + ) + counter.reset() + + rerender( + + content + , + ) + + expect(counter.updateCount).toBe(1) + expect(counter.nestedUpdateCount).toBe(0) + }) +}) diff --git a/packages/react/src/internal/components/ValidationAnimationContainer.tsx b/packages/react/src/internal/components/ValidationAnimationContainer.tsx index 637502770f2..884ee15d2a7 100644 --- a/packages/react/src/internal/components/ValidationAnimationContainer.tsx +++ b/packages/react/src/internal/components/ValidationAnimationContainer.tsx @@ -1,25 +1,26 @@ import type {HTMLProps} from 'react' import type React from 'react' -import {useEffect, useState} from 'react' +import {useState} from 'react' import classes from './ValidationAnimationContainer.module.css' interface Props extends HTMLProps { show?: boolean } -const ValidationAnimationContainer: React.FC> = ({show, children}) => { +const ValidationAnimationContainer: React.FC> = ({show, children, style, ...rest}) => { const [shouldRender, setRender] = useState(show) - useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect, react-you-might-not-need-an-effect/no-derived-state, react-you-might-not-need-an-effect/no-event-handler - if (show) setRender(true) - }, [show]) + // Start rendering as soon as `show` becomes true. Adjusting state during render + // (instead of from an effect) avoids the extra post-commit render the effect caused. + if (show && !shouldRender) { + setRender(true) + } const onAnimationEnd = () => { if (!show) setRender(false) } return shouldRender ? ( -
+
{children}