Comments (15)
Thanks @bumpah for the sandboxes! That really helps to track down the issue.
Something weird is happening... If you change the react version down to 16.3 it works. From react version 16.4 upwards then the behavior is what you describe.
from react-rating.
React 16.4 Bugfix for getDerivedStateFromProps might be the reason why it is working in React 16.3 and not from 16.4 and above.
Too look into:
from react-rating.
I believe here is a cultrip on this.props.onChange
-call
handleClick(value, e) {
const newValue = this.translateDisplayValueToValue(value);
this.props.onClick(newValue);
// Avoid calling setState if not necessary. Micro optimisation.
if (this.state.value !== newValue) {
// If we have a new value trigger onChange callback.
this.setState({
value: newValue
}, () => this.props.onChange(this.state.value)); // this.state.value here does not change
}
}
Inspected this a little bit and you'll notice that this.state.value
does not get updated as expected and always provides value of a initialRating -value (initial state so to say).
from react-rating.
Yep. I have just taken a quick look and yes. It is because the getDerivedStateFromProps
is called before the setState
callback. There, in the getDerivedStateFromProps
, the state.value
is set to initialRating
because, yes, the initialRating
is likely to be different than the recently clicked/set value
. So, basically, we are always setting initialRating
as the current value 😓.
static getDerivedStateFromProps(props, prevState) {
const { initialRating } = props;
return (initialRating !== prevState.value)
? { value: initialRating }
: null;
}
Roughly this is the sequence of calls:
- handleClick
- getDerivedStateFromProps
- setState callback
In the meantime a solution is found I have deprecated the 2.0.1 version.
I have published the version 2.0.2 that is just a copy of the 2.0.0 to try to cut the bleeding 😉.
from react-rating.
What is the main objective you want to achieve by using getDerivedStateFromProps
-function to set initialRating this way?
If you remove that function, component works as I would describe "as you'll expect".
if you want only set initialRating
once when component mounts that is already handled by in component constructor function,
this.state = {
value: props.initialRating
};
If you which to achieve something more complex with this that im not currently aware I can collaborate with this issue if given some details.
from react-rating.
What is the main objective you want to achieve by using getDerivedStateFromProps-function to set initialRating this way?
It is a long story 😉. In short, it is an attempt to replace/get rid of the deprecated componentWillReceiveProps
while keeping the same controlled/uncontrolled component behavior.
In #129 we just tried to replace this deprecated life cycle method with the, I thought equivalent, getDerivedStateFromProps
method. But, as you can see they are not actually equivalent.
After discussing this replacement in PR #129 with @tstirrat15 and reading You Probably Don't Need Derived State and Always be ready to render posts, we agreed that trying to keep this double behavior (controlled and uncontrolled) brings an unnecessary complexity.
A new issue #134 was created with the goal of refactoring it to meet the currently recommended best practice.
Hope it gives you some context.
And of course, I would be really glad of any kind of collaboration 😃
from react-rating.
Forgot to actually revert the offending code 😞. The good reverted version is 2.0.3. Apologies.
from react-rating.
After wrangling with this for quite a while, I finally concluded that this must be a bug in react-rating and then a search brought me here. I'm on v2.0.5, so I'm a bit confused whether this is still an issue or sorted in 2.0.3?
from react-rating.
@zehawki Yep. It should have been fixed on v2.0.3 or greater. The affected versions should only be v2.0.1 and v2.0.2. So v2.0.5 should be fine. Sorry for the confusion it could have caused.
from react-rating.
Well the reason I posted is that I see the issue is there in 2.0.5...
from react-rating.
Then it should be another reason causing the same issue.
I just forked the offending version from the OP:
v2.0.1 => value in onChange
event is always same as initialRating
value https://codesandbox.io/s/react-rating-201-9p75n
with the version you say:
v2.0.5 => value in onChange
event is as expected https://codesandbox.io/s/react-rating-205-te4ik
and it seems fixed. I mean, the value is changed.
Could you use a fork of this codesandox and try to reproduce your issue?
from react-rating.
Please see this starting point: https://codesandbox.io/s/react-rating-205-forked-n5wps?file=/src/index.js. As soon as there is a real onchange handler, things behave strangely. In this case, it takes 2 clicks to register.
Next: https://codesandbox.io/s/react-rating-205-forked-ffkzz?file=/src/index.js: 1st click resets to initial rating and 2nd click registers the new rating.
from react-rating.
I think I know what's the problem... It is the evil initialRating
name hitting again 😓.
When you are handling the state yourself (like useState
does), then initialRating
becomes the actual value. I know. The name is misleading and is one of the things we should improve in next versions.
You have to provide the value
otherwise the rating will get stuck with the initiaRating
. Actually, this, the uncontrolled way, is the way this component should aim to and get rid of the confusing controlled one.
function App() {
const [value, setValue] = useState(11);
useEffect(() => {
console.log(value);
});
return (
<div className="App">
<ReactRating
start={6}
stop={16}
initialRating={value}
onChange={setValue}
/>
</div>
);
}
Check https://codesandbox.io/s/react-rating-205-forked-qcz09
For more information:
Sometimes initialRating property name leads to confusion. It is not the first time having problems with such a name (#84 or #108). It is used as the initial value for the uncontrolled version and the value for the controlled version. I mean, when used as an uncontrolled component the state value is initialized to the initialRating, but, when it is used as a controlled component the state value matches to the initialRating.
from react-rating.
I close this issue because new versions (>= v2.0.3) solve the original bug.
from react-rating.
I feel there's a fair amount of misunderstanding of React going on here. Whether the component is controlled or uncontrolled, it still has to send its value to the outside world else whats the point. The only way it will send its value outside is via an onClick or onChange. And therefore, there is no condition in which one would not use an onChange. And merely because onChange is called, it does NOT mean that: When you are handling the state yourself (like useState does), then initialRating becomes the actual value. I know
Put differently: just because onChange is called, does not mean that the state of this component is being handed from outside, ie controlled. It would be controlled if a props like "value" was send back into this component and actually used. You would see in the code that I've provided, I'm using this component in uncontrolled mode.
Now to correctly get uncontrolled mode working, with an initialValue, all you have to do is this:
- Get rid of both UNSAFE_componentWillReceiveProps functions. These are not needed.
- In componentDidUpdate of class Rating, add:
if (this.props.value !== prevProps.value)
this.setState({ displayValue: this.props.value })
initialRating is just fine, and will continue to work as its intended, ie starting value
from react-rating.
Related Issues (20)
- Initial Rating returns null only renders once
- Error during installation on jenkins HOT 2
- emptySymbol is not hidden when fullSymbol is displayed HOT 9
- Cannot read property 'type' of undefined
- Method comments are switched HOT 1
- Misalignment HOT 4
- Sorry HOT 1
- All touches, even scroll/swipe gestures trigger "onClick" HOT 8
- Warning: Using UNSAFE_componentWillReceiveProps in strict mode is HOT 10
- Warning: Using UNSAFE_componentWillReceiveProps in strict mode is not recommended and may indicate bugs in your code. See https://reactjs.org/link/unsafe-component-lifecycles for details. HOT 5
- Feature request: Lighter DOM Size HOT 1
- Doesn't work with react 17 HOT 1
- Selection requires double click if onClick method provided HOT 3
- How to add the validation for the rating component HOT 1
- Add option to remove default style
- mixed symbol but react component HOT 1
- Package should be marked as deprecated and a replacement fork nominated HOT 2
- React 18: TS2786: 'Rating' cannot be used as a JSX component. HOT 9
- `@types` dependencies should be in `devDependencies`
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from react-rating.