Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Number increases twice with set-Callback #21963

Closed
Shaegi opened this issue Jul 26, 2021 · 7 comments
Closed

Bug: Number increases twice with set-Callback #21963

Shaegi opened this issue Jul 26, 2021 · 7 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@Shaegi
Copy link

Shaegi commented Jul 26, 2021

When trying to update an Object with a number within, there seems to be a bug where when I first add and then spread I will receive a wrong number.

React version: 17.0.2

Steps To Reproduce

  1. create useState with Object with Number inside
  2. create set-Callback
  3. mutate prevState directly
  4. Spread on return

Link to code example: https://codesandbox.io/s/nice-flower-3783g?file=/src/App.js

The current behavior

Click0 will go from 0 -> 1 -> 3 -> 5 -> 7
Click1 will go from 0->1->2->3
Click2 will go from 0->1->2->3

The expected behavior

"Click0", "Click1" and "Click2" should have the same result.

@Shaegi Shaegi added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jul 26, 2021
@vkurchatkin
Copy link

Well, you shouldn't "mutate prevState directly". That's what causes the problem

@bvaughn
Copy link
Contributor

bvaughn commented Jul 26, 2021

This code should not be doing what it's doing:

          setCount((prev) => {
            prev.count = prev.count + 1;
            return { ...prev };
          });

State updaters should always return a new state, not mutate the old one:

          setCount((prev) => {
            return {
              count: prev.count + 1
            };
          });

@bvaughn
Copy link
Contributor

bvaughn commented Jul 26, 2021

I'm going to close this issue since it seems like a question that's now been answered

@bvaughn bvaughn closed this as completed Jul 26, 2021
@Shaegi
Copy link
Author

Shaegi commented Jul 26, 2021

In all 3 cases a new object will be returned, it seems pretty weird that the order of creating the new object matters aspecially because all updaters are only called once. So how does the +2 happen instead of +1, which ist still not explained. Even tho I know its indeed bad practise to update the state value directly it should be an issue to do so IMO and seems to be unintuitive for new users and just straight up wrong because click0 and click1 will both return a new object with count increased by 1.

@bvaughn
Copy link
Contributor

bvaughn commented Jul 26, 2021

aspecially because all updaters are only called once

This is not true. Strict mode calls updaters twice, like render functions and other render-phase functions in order to help spot side effects.

The mutation side effect in your code is why the value gets over-incremented.

@Shaegi
Copy link
Author

Shaegi commented Jul 26, 2021

Okay, thanks for the explaination. Haven't read concurrent mode up yet, so I didn't know about the second "silent" render.
Even as someone who uses react for the last 3 years this seams quite harsh to understand whats going on.
Still Mystery solved.
I'd still suggest some kind of warning/error beeing thrown in a case like this because it seems like it can be easly overlooked.

@bvaughn
Copy link
Contributor

bvaughn commented Jul 26, 2021

We are working on both React and DevTools to make this (double render) more discoverable and less confusing. (See #21889 and #21890)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

3 participants