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 8 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: 50046a3 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 |
|
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
…mediate-selected-derive
…nputs change Decouples the reset-key advance from the value-difference check so it can't go stale and clobber a user-driven selection on the next render (single-select modal). Adds a regression test.
|
Integration test results from github/github-ui PR: |
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