zillow / react-slider

Accessible, CSS agnostic, slider component for React.
https://zillow.github.io/react-slider
MIT License
883 stars 231 forks source link

React Slider works on always one value reference and shares it outside #181

Closed viters closed 4 years ago

viters commented 4 years ago

Description

Instance of state.value is the same throughout component usage and slider does all operations by checking and mutating this array. In the same time, it shares the reference to it with getValue or fired events like onChange.

It may lead to difficult bugs to track. E.g. I am using immerjs, which is making partial updates to app state. If I just pass value from ReactSlider to state - it breaks. I suppose immer is holding the reference to ReactSlider's state.value and tries to update only one value (because the other one stays the same when you grab one handle).

Example: https://codesandbox.io/s/sweet-water-0qxz6?file=/src/App.js

Other prop variants:

onChange={onChange} value={value.slice()} // breaks
onChange={v => onChange(v.slice())} value={value} // breaks
onChange={v => onChange(v.slice())} value={value.slice()} // works

I suppose ReactSlider could either copy value array before exposing it or create a new reference on each mutate.

stonebk commented 4 years ago

Yeah, we probably shouldn't be mutating the value that is passed in, rather make a copy of the array if we are going to mutate it in our internal state.