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: React StrictMode takes not updated state value from first update on second update of strictmode #21956

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

Comments

@Eliav2
Copy link

Eliav2 commented Jul 24, 2021

copy and edited from #21930 (comment)

I will use the next 2 terms

  • update call - execution of the function body(no effects)
  • render cycle - update(s) call and following effects

2 very important notes:

  • Calling state hook from effect(like useEffect or useLayoutEffect) will cause React to schedule another render.
  • Calling state hook from FC body will cause React to schedule another update call.

on StrictMode:
after a small experiment,2 updates,1 render for each render cycle(means the body of the FC executes twice while effect fire once)

code

see the next code , and lets remember react triggers 2 updates(at least) per render on StrictMode, if the first update will trigger another update(via setState for example) it will come before the second update strictmode schedules:

  if (shouldUpdatePosition.current) {
    ...
    const pos = getPosition(...)
    setSt(pos);
    log('pos',pos)
    shouldUpdatePosition.current = false;
  }
log('st',st)

on the first render shouldUpdatePosition.current=true so setSt gets the right position and update call scheduled by React with the right dimensions. the update call would execute and then on StrictMode, the second update would start(and the 3'th update call) and now shouldUpdatePosition.current=false the setSt is not executed, and React takes the value from the first update and not from the previous update call. this is unexpected!

recap, and logs:

for logs explanation, let's say 0 is the wrong(not updated) value and 10 is the correct value

  • update 0
    'pos',10 (set state trigger, another update call is scheduled)
    'st',0
  • update 1
    'st',10
  • update 2 (StrictMode only! the second update call of strict mode!)
    'st',0 (why? not expected! should be 10)

in order to fix this bug and make the result as expected react should take the value from the most recent update call and not the the first update call of the current cycle in strict mode

this is clearly a bug (or limitation) in StrictMode

code sandbox

see logs, this is not expected
https://codesandbox.io/s/react-strict-mode-bug-bx8is?file=/src/index.js

@vkurchatkin
Copy link

This is neither a bug nor a limitation. This block of code violates some important rules:

 if (shouldUpdate.current) {
    setValue(10);
    shouldUpdate.current = false;
  }
  1. You read a mutable value during render (ref value)
  2. You perform side effects in render (calling state setter and mutating the ref)

StrictMode is just doing its job and highlights these issues

@Eliav2
Copy link
Author

Eliav2 commented Jul 25, 2021

can you please provide a reference to the docs that shows this is a rule violation, and alternative approaches?

also, why is this a rule violation? I know what is the order of a component lifecycle and I want to rely on it with a reference to a constant object. why is this wrong?

does Detecting unexpected side effects is the reason? if does, does it applies only to concurrent mode in react?

@Eliav2
Copy link
Author

Eliav2 commented Jul 25, 2021

I want to discuss the example on the docs:

For example, consider the following code:

class TopLevelRoute extends React.Component {
  constructor(props) {
    super(props);

    SharedApplicationState.recordEvent('ExampleComponent');
  }
}

At first glance, this code might not seem problematic. But if SharedApplicationState.recordEvent is not idempotent, then instantiating this component multiple times could lead to invalid application state. This sort of subtle bug might not manifest during development, or it might do so inconsistently and so be overlooked.

this example talks about inconsistently of event that if you would fire more then once will cause to unexpected results, while in my case the there is no inconsistency because I rely on a constant and consistent lifecycle. My app would work if React.StrictMode would take into account effects from the first update call on the second update call of strictmode.

look at the logs examples: it seems like update 3 is unaware to state value of update 2, it just takes the state value of update 1 and therefore just overrides the state value of update 2.
you can call update 1, and then update 2 1000 times, and you will always get the same result. the issue here that is update 3(which is the second call that comes from strict mode) always takes the state value of update 1 without taking into account the updated state value of update 2. and this, is unexpected.

@Eliav2 Eliav2 changed the title Bug: React StrictMode takes not updated state value Bug: React StrictMode takes not updated state value from first update on second update of strictmode Jul 25, 2021
@kosciolek
Copy link

@vkurchatkin

  1. You perform side effects in render (calling state setter and mutating the ref)

Does it mean you cannot do the following?

const [element, setElement] = useState();
<div ref={setElement} />

This is what I currently use, if I want to be notified of element mounts/unmounts.

@Eliav2
Copy link
Author

Eliav2 commented Jul 30, 2021

Your situation is not related to this issue. @kosciolek

@bvaughn
Copy link
Contributor

bvaughn commented Aug 4, 2021

  1. You read a mutable value during render (ref value)
  2. You perform side effects in render (calling state setter and mutating the ref)

Slight clarification: It's okay to call a state setter during render.

Reading or writing a ref value during render is only safe if you are implementing the lazy initialization pattern.

Other types of reading are unsafe as the ref is a mutable source.

Other types of writing are unsafe as they are effectively side effects.

In the future we may warn about this in DEV mode (see #18545) but we haven't decided yet.

@bvaughn
Copy link
Contributor

bvaughn commented Aug 4, 2021

I'm going to close this issue since this isn't a React bug.

@bvaughn bvaughn closed this as completed Aug 4, 2021
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

4 participants