[Slider] Don't error on minimal changes with readonly value #28472
Conversation
Simulating user input that may lead to a change but doesn't because it's too small
|
@material-ui/core: parsed: +Infinity% , gzip: +Infinity% |
There was a problem hiding this comment.
I removed the shortcut since it probably has next to no benefit since we always fired a change behavior regardless. If we want to revisit the performance shortcut, we need to be diligent and measure. Because I doubt there's much benefit when the Slider would re-render anyway if the value is controlled.
I think that we should revert. This logic allows shortcutting the rendering. When you slide, you often call 10 times the same onChange with the same value. This is noticeable, especially when controlled. DX is also benefiting from this logic, when using a console log, you won't get flooded.
| [ | ||
| ['readonly range', Object.freeze([1, 2])], | ||
| ['range', [1, 2]], | ||
| ].forEach(([valueLabel, value]) => { |
There was a problem hiding this comment.
Should we write the tests on the "styled" version? To be more "end-to-end"?
There was a problem hiding this comment.
The unstyled version is already the end.
Does not matter. It's such a negligible impact that no additional line of code is worth it. |
|
Revert the recent attempt by @oliviertassinari to make it work. The change should be done with more diligence. Either |
Agree, we could push it one step further, it would be more intuitive for developers. |
That's not the issue here. The issue is that we have behavior that doesn't match the interface semantics. We fire a change event even though nothing changed. That is a bug. |
Closes #28466
We previously leveraged React bailing out when setting state that's referntially equal. But at the same time we tried to mutate that value (essentially breaking the assumptions that React has about referentially equal values). This causes errors in JavaScript's Strict Mode (
"use strict";notReact.StrictMode) if that value is frozen (which RTK does).I removed the shortcut since it probably has next to no benefit since we always fired a change behavior regardless. If we want to revisit the performance shortcut, we need to be diligent and measure. Because I doubt there's much benefit when the Slider would re-render anyway if the value is controlled.