perf(SelectPanel): reset intermediate selection during render, not in an effect#8025
perf(SelectPanel): reset intermediate selection during render, not in an effect#8025mattcosta7 wants to merge 3 commits into
Conversation
… an effect
Replace the useEffect that reset the single-select modal's intermediate selection with the adjust-state-during-render pattern (tracking previous {open, selected, isSingleSelectModal}). Behavior is identical but avoids the extra post-commit render, and removing the eslint-disable lets the lint rule guard against regressions.
🦋 Changeset detectedLatest commit: c962a6d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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
Bot
added
the
integration-tests: recommended
|
There was a problem hiding this comment.
Pull request overview
This PR updates SelectPanel’s single-select modal behavior to reset the intermediate selection during render (tracking prior inputs) instead of syncing it from a useEffect, aiming to avoid an extra post-commit render and removing the related ESLint disable.
Changes:
- Replace the
useEffect-based intermediate-selection reset with an adjust-state-during-render approach keyed on{open, selected, isSingleSelectModal}changes. - Add a patch changeset describing the performance-oriented internal behavior adjustment.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/SelectPanel/SelectPanel.tsx | Moves intermediate selection reset logic from an effect to render-time adjustment based on previous inputs. |
| .changeset/selectpanel-intermediate-selected-derive.md | Adds a patch changeset documenting the internal behavior/perf change. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
| // Reset the intermediate selected item when the panel is opened/closed or the external | ||
| // selection changes. Adjusting state during render (tracking the previous inputs) rather | ||
| // than syncing it from an effect avoids an extra post-commit render. | ||
| const [intermediateSelectedResetKey, setIntermediateSelectedResetKey] = useState({ | ||
| open, | ||
| selected, | ||
| isSingleSelectModal, | ||
| }) | ||
| if ( | ||
| intermediateSelectedResetKey.open !== open || | ||
| intermediateSelectedResetKey.selected !== selected || | ||
| intermediateSelectedResetKey.isSingleSelectModal !== isSingleSelectModal | ||
| ) { | ||
| setIntermediateSelectedResetKey({open, selected, isSingleSelectModal}) | ||
| setIntermediateSelected(isSingleSelectModal ? selected : undefined) | ||
| }, [isSingleSelectModal, open, selected]) | ||
| } |
|
Integration test results from github/github-ui PR:
All checks passed! |
Overview
SelectPanelreset the single-select modal's intermediate selection from an effect:That's the
set-state-in-effect/no-derived-statepattern and forces an extra post-commit render whenever the panel opens/closes or the selection changes. This replaces it with the adjust-state-during-render pattern (tracking the previous inputs), which React restarts before committing — so the reset is behavior-identical with no extra commit:Removing the
eslint-disablealso lets the lint rule guard against regressions.Changelog
Changed
SelectPanel: the single-select modal's intermediate selection is reset during render instead of in an effect, avoiding an extra re-render on open/close/selection change. No behavior or API change.Rollout strategy
Testing & Reviewing
{open, selected, isSingleSelectModal}reset triggers).SelectPanelsuite (152 tests), which covers the single-select-modal intermediate-selection / cancel / confirm flows, passes unchanged.Merge checklist