perf(FormControl): derive validation message visibility during render by mattcosta7 · Pull Request #8023 · primer/react

mattcosta7 · 2026-06-22T14:05:05Z

Overview

ValidationAnimationContainer (used by FormControl to animate validation messages) tracked whether to render its children in state and synced it from an effect:

useEffect(() => { if (show) setRender(true) }, [show])

That's the set-state-in-effect / no-derived-state pattern, and it deferred the enter animation by one render. This adjusts the state during render instead, so the content mounts in the same commit:

const [shouldRender, setShouldRender] = useState(show)
if (show && !shouldRender) setShouldRender(true)          // enter, in render
const onAnimationEnd = () => { if (!show) setShouldRender(false) } // exit unchanged

Behavior is identical (the exit animation still plays via onAnimationEnd), but there's no extra post-commit render, and removing the eslint-disable lets the lint rule guard against regressions.

Changelog

Changed

FormControl validation messages now render in the same commit instead of one render later (internal ValidationAnimationContainer refactor). No public API change.

Rollout strategy

Patch release
Minor release
Major release
None

Testing & Reviewing

Added a render-count regression test (via createRenderCounter) asserting show produces no extra update commit.
The FormControl suite passes unchanged.

Merge checklist

Replace the effect that set ValidationAnimationContainer's shouldRender with an adjust-during-render update, removing the extra post-commit render when the validation message appears. Adds a render-count regression test.

changeset-bot · 2026-06-22T14:05:13Z

🦋 Changeset detected

Latest commit: 1a50c22

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2026-06-22T14:05:56Z

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. If this doesn't work, you can also use the original workflow here. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

primer · 2026-06-22T14:20:50Z

🤖 Lint issues have been automatically fixed and committed to this PR.

…verage check The new ValidationAnimationContainer render-count test triggers the classname-coverage check, but the internal animation wrapper does not forward a className prop, so implementsClassName does not apply. Add it to the ignore list.

Copilot

Pull request overview

This PR refactors the internal ValidationAnimationContainer used by FormControl-adjacent validation UI to avoid an effect-driven “derived state” update, aiming to mount validation content in the same commit when show becomes true.

Changes:

Refactored ValidationAnimationContainer to set shouldRender when show becomes true during render (removing the effect + eslint-disable).
Added a dedicated unit test suite, including a render-count regression assertion.
Added a patch changeset and updated the classname-test script ignore list for the new test file.

Show a summary per file

File	Description
`script/check-classname-tests.mjs`	Ignores the new internal component test from the “implementsClassName” enforcement script.
`packages/react/src/internal/components/ValidationAnimationContainer.tsx`	Removes effect-driven derived state and derives “enter” visibility during render.
`packages/react/src/internal/components/ValidationAnimationContainer.test.tsx`	Adds initial render/transition tests and a render-count regression test.
`.changeset/validation-animation-derive-in-render.md`	Adds a patch changeset entry for the behavior/perf improvement.

Copilot's findings

Files reviewed: 4/4 changed files
Comments generated: 4

mattcosta7 · 2026-06-23T14:38:55Z

 const ValidationAnimationContainer: React.FC<React.PropsWithChildren<Props>> = ({show, children}) => {
  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.


@copilot do we neded to make sure show is only checking this when it was changed? we may need to track previous state and if previous!==show && show && !shouldRender

No previous-state tracking needed. shouldRender can only be false in exactly two situations:

Initial render — useState(show) initialised it as false because show started false. For show && !shouldRender to fire here, show must now be true, so it did change.

After onAnimationEnd — that callback calls setRender(false) only when !show (exit animation complete). For show && !shouldRender to fire after that, show must have flipped back to true.

In both cases !shouldRender is already the precise guard: it's true if and only if show transitioned from false → true. Adding previous !== show would be redundant — there's no scenario where show is continuously true yet shouldRender is false.

+    expect(counter.updateCount).toBe(1)
+  })


+'@primer/react': patch
+---
+
+FormControl: Show the validation message without an extra re-render by deriving its mounted state during render instead of in an effect.


+  // Internal animation wrapper that does not forward a className prop
+  'packages/react/src/internal/components/ValidationAnimationContainer.test.tsx',


github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jun 22, 2026

mattcosta7 deployed to github-pages June 22, 2026 14:18 — with GitHub Actions View deployment

github-actions Bot temporarily deployed to storybook-preview-8023 June 22, 2026 14:21 Inactive

mattcosta7 deployed to github-pages June 22, 2026 15:45 — with GitHub Actions View deployment

github-actions Bot temporarily deployed to storybook-preview-8023 June 22, 2026 15:45 Inactive

mattcosta7 self-assigned this Jun 23, 2026

mattcosta7 marked this pull request as ready for review June 23, 2026 11:58

mattcosta7 requested a review from a team as a code owner June 23, 2026 11:58

mattcosta7 requested review from Copilot and siddharthkp June 23, 2026 11:58

Merge branch 'main' into fix/validation-animation-derive-in-render

bfa9772

Copilot started reviewing on behalf of mattcosta7 June 23, 2026 11:58 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

mattcosta7 deployed to github-pages June 23, 2026 12:09 — with GitHub Actions View deployment

github-actions Bot temporarily deployed to storybook-preview-8023 June 23, 2026 12:10 Inactive

Copilot started work on behalf of mattcosta7 June 23, 2026 14:39 View session

Copilot finished work on behalf of mattcosta7 June 23, 2026 14:44

joshblack approved these changes Jun 23, 2026

View reviewed changes

Forward props/className, tighten render test, terse changeset

1a50c22

mattcosta7 deployed to github-pages June 24, 2026 01:31 — with GitHub Actions View deployment

github-actions Bot deployed to storybook-preview-8023 June 24, 2026 01:32 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(FormControl): derive validation message visibility during render#8023

perf(FormControl): derive validation message visibility during render#8023
mattcosta7 wants to merge 4 commits into
mainfrom
fix/validation-animation-derive-in-render

mattcosta7 commented Jun 22, 2026

Uh oh!

changeset-bot Bot commented Jun 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 22, 2026

Uh oh!

primer Bot commented Jun 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mattcosta7 Jun 23, 2026

Uh oh!

Copilot AI Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		// Internal animation wrapper that does not forward a className prop
		'packages/react/src/internal/components/ValidationAnimationContainer.test.tsx',

Conversation

mattcosta7 commented Jun 22, 2026

Overview

Changelog

Changed

Rollout strategy

Testing & Reviewing

Merge checklist

Uh oh!

changeset-bot Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions Bot commented Jun 22, 2026

⚠️ Action required

Uh oh!

primer Bot commented Jun 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

mattcosta7 Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

changeset-bot Bot commented Jun 22, 2026 •

edited

Loading