Slider: Unnecessary onChange event fired by Range

[gpinhey]

In componentWillReceiveProps:
https://github.com/react-component/slider/blob/e7af5aa6e252a16cc97d9627b28e19e58b519bd2/src/Range.jsx#L52-L73
After <Range/> uses setState to update bounds, it then uses the cached old value of bounds to determine whether that value is outside of the range. If value and min/max are changed at the same time, and the previous value is outside of the new min/max, then <Range/> will fire an onChange handler which provides the current value clipped to the new min/max. This will fire onChange even if there is no actual change, such as when the current value is already within the new min/max.

[imprashast]

Even onAfterChange for range is fired twice. One when you drag the slider and one after you lose the focus

[datner]

I made a codesandbox to replicate this issue (not that it's all that complicated)

This is not a huge deal but Its annoying haha

[datner]

also @gpinhey componentWillRecieveProps is being deprecated and now considered legacy (even attaining an UNSAFE_ prefix ) after "abuse" of the exact type you are attempting in your code. I highly recommend you reconsider how you structure the interaction between this component and its parent, but if you cannot avoid updating the state from props (I've been there too many times, I understand) use the new static getDerivedStateFromProps that was made for this!

[gpinhey]

Interesting, perhaps I'll try another PR which restructures the <Range/> to not use the old lifecycles. I had heard of the deprecated React lifecycle methods but I didn't recognize that this code was already using one.

I didn't think that my consumer code (using this project as an npm package) encountering this problem was particularly strange, it was using the standard 'controlled component' pattern where values from a Redux store were used to set up a <Range/> which then had its onChange handler connected to actions which updated the values in the Redux store. This is why I was confused; when something else in my consumer code updated the Redux store to change how the <Range/> displayed, the <Range/> fired an extra onChange handler which forced some values back to what was captured in its local component state, because the new values didn't match the bounds stored in local component state.

That said, my existing PR was an attempt to fix only this issue without changing (or even understanding) any of the code around it. The real problem this stretch of code is trying to solve is the case where a user provides props which don't make sense.

[Keksike]

@afc163 this should not be closed, issue is still there

[jonasgroendahl]

The issue is still there indeed