[slider] Move state to ReactStore - #5592
Draft
flaviendelangle wants to merge 5 commits into
Draft
Conversation
flaviendelangle
requested review from
aarongarciah,
atomiks,
colmtuite,
jjenzz and
michaldudak
as code owners
August 28, 2026 11:57
commit: |
Bundle size
PerformanceTotal duration: 1,113.01 ms +42.50 ms(+4.0%) | Renders: 76 (+0) | Paint: 1,818.32 ms +86.75 ms(+5.0%)
11 tests within noise — details Metric alarms
Check out the code infra dashboard for more information about this PR. |
flaviendelangle
added
component: slider
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
flaviendelangle
marked this pull request as draft
August 28, 2026 12:14
- Keep the two most recent `values` entries compared with `Object.is` so layout-effect store writes no longer force an extra commit per controlled value change, and `NaN` values no longer cause an update loop - Only write the internal value when uncontrolled - Flatten interaction state, move refs into the store context, and drop the `onValueCommitted` pass-through - Restore the explanatory comments and document the observer ordering - Add regression tests for the cache, controlled rejection, field touched state, and event target name Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Call `store.x` and `store.context.x` at the use site instead of destructuring - Drop `readonly` from the store methods to match the other stores - Keep only the comments the other stores carry Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`useControlledProp` kept the last controlled value in the store after the prop was removed, so `prop ?? internal` selectors kept reporting a value that was no longer rendered and commands diverged from the screen. Clear the key so the internal value takes over, mirroring `useControlled`'s fallback. Also fix the inverted switch warning and log it once per switch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slider currently keeps value state, interaction state, refs, and lifecycle commands across
Slider.Rootand one large memoized context. This PR moves that lifecycle into aSliderStorebased onReactStore.The change:
useControlledPropand callback props withuseContextCallbackvaluesarray through a memoized per-store selector instead of storing a second copyNo public API, DOM, or behavior change is intended.
Tradeoff
The standalone Slider entry now includes the
ReactStoremachinery, which produces a noticeable relative increase for consumers importing only Slider. The full library already includesReactStorethrough other components, so its increase is much smaller.@base-ui/react@base-ui/react/sliderThis trades 1.6 kB gzip for standalone Slider consumers for one owner of the component lifecycle and alignment with the newer store-based components. The whole-library cost is 512 B gzip.
Keeping root props such as
disabledoutside the synchronized store state is deliberate.useSyncedValueswrites in a layout effect, after descendants render. Storing those props only in the store would let descendant callbacks observe the previous render during the same commit.Verification
pnpm test:jsdom Slider --no-watch(337 passed, 82 skipped)pnpm test:chromium Slider --no-watch(417 passed, 2 skipped)pnpm typescriptpnpm eslintpnpm prettierpnpm stylelintpnpm extract-error-codespnpm -F @base-ui/react buildpnpm size:snapshotI have followed the PR section of the contributing guide.