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

React.StrictMode causes setState to fire twice #12856

Closed
larabr opened this issue May 18, 2018 · 27 comments
Closed

React.StrictMode causes setState to fire twice #12856

larabr opened this issue May 18, 2018 · 27 comments

Comments

@larabr
Copy link

larabr commented May 18, 2018

Do you want to request a feature or report a bug? Bug

What is the current behavior?
When wrapping a with React.StrictMode, setState is fired twice

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:
Here is the jsFiddle
By clicking on a title and checking the console, you should see that the setState is called twice (its callback, however, is called only once).

What is the expected behavior?
Not sure, it might be related to #12094, then the behaviour might be intended. But when using the previous state to set the new one, then the component breaks if setState is fired twice (for instance, toggle components don't work).

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React 16.3

@nmain
Copy link

nmain commented May 18, 2018

The prevState => nextState callback is fired twice, but I don't see the behavior you're talking about with toggle components not working. If you add some more logging or rendering to your example, you'll see that the toggle in it does in fact work correctly:

https://jsfiddle.net/oq58bf9k/

@gaearon
Copy link
Collaborator

gaearon commented May 18, 2018

It is expected that setState updaters will run twice in strict mode in development. This helps ensure the code doesn't rely on them running a single time (which wouldn't be the case if an async render was aborted and alter restarted). If your setState updaters are pure functions (as they should be) then this shouldn't affect the logic of your application.

@gaearon gaearon closed this as completed May 18, 2018
@franklixuefei
Copy link

franklixuefei commented May 31, 2018

@gaearon Consider this example

this.setState(({counter}) => {
  return { counter: counter + 1 };
});

say this piece of code will be triggered by clicking on a button. If setState updater function can be called multiple times under StrictMode, then how do I guarantee that my counter only increments by 1 per click?

Actually, this is a real example in my project, where I have a button, and setState was triggered after I clicked on the button. I noticed that the updater function in my setState was called twice, resulting in inconsistency.

@aweary
Copy link
Contributor

aweary commented May 31, 2018

@franklixuefei the updater should be called twice with the same state. For example, if counter is 0 it will be called with 0 twice, returning 1 in both cases.

Also I believe only one of the invocations actually cares about the value returned. So React isn't processing each state update twice, it's just calling the function twice to help surface issues related to side effects in a state updater and other methods that should be pure.

@franklixuefei
Copy link

franklixuefei commented May 31, 2018

@aweary Thanks for answering my question! However, this is not the case. Take a look at the below real-life example from my project:

  private handleItemRemove(index: number) {
    const { onItemsChange } = this.props;
    this.setState(
      ({ selectedItems }) => {
        console.log('updater function called', selectedItems);
        // remove the item at index
        if (selectedItems.length > 0) {
        // remove one item at the index
          selectedItems.splice(index, 1);
        }
        return { selectedItems: [...selectedItems] };
      },
      () => {
        // tslint:disable-next-line:no-unused-expression
        this.inputNode && this.inputNode.focus();
        // tslint:disable-next-line:no-unused-expression
        onItemsChange && onItemsChange(this.state.selectedItems);
      }
    );
  }

where selectedItems is just an array of objects. I noticed that in this example, the updater function was called twice (can be seen from the console log), and the second time the updater function was called, selectedItems contains one fewer element than the first time. Trust me, I was banging my head on this bug last night...

@iamdustan
Copy link
Contributor

splice mutates the array so this looks like the type of impurity that strict mode is intended to catch 😄

@franklixuefei
Copy link

franklixuefei commented May 31, 2018

@iamdustan I noticed that too, but if you look closely, I created a new array at the end:

return { selectedItems: [...selectedItems] };

Oh wait... You are right! It modified the original array even though I returned a new array!

I'm so surprised that I didn't even think of it at first. What a shame.

Now I realized how great React.StrictMode is.

Thanks @iamdustan and @aweary

@kumar303
Copy link

Similarly, constructor() is called twice with the same props. If you're dispatching a loading action that triggers API fetching and more action dispatching via something like saga then it makes development pretty confusing. You might see flashes of UI changes as the component enters its loading state multiple times.

I'm not sure how helpful this is for day to day development. I guess it's just a downside to relying on side effects with Saga?

@tonix-tuft
Copy link

I am experiencing the same "issue" (setState updater function called twice) even if I do not use React.StrictMode (I am using React 16.8.6). Is this behaviour enabled by default on React 16.8.6?

@LoveenDyall
Copy link

Surely execution of setState (and its count of execution) is a functional req and should be identical across development and production modes?

Only difference between dev and prod should be data or non-functional reqs IMO.

@gaearon
Copy link
Collaborator

gaearon commented Apr 13, 2020

The whole purpose of StrictMode is to find cases where the user-written functions that are supposed to be pure are, in fact, not pure. If they are pure, then there is no observable difference between development and production modes.

f they are not pure, Strict Mode helps find that issue. This is to prepare for the future, where React doesn't give any guarantees about the number of times they would be invoked, even in production. You can opt out of Strict Mode if you don't want this behavior, but it is the very purpose of Strict Mode.

@LoveenDyall
Copy link

Executing twice to detect an intermittent error in a potentially 'unpure' function is arbitrary. They could argue you need 10 executions so I'm struggling to see why this is a justification for Strict Mode at the package level, when it should and could be a user-defined mode.

@gaearon
Copy link
Collaborator

gaearon commented Apr 14, 2020

Executing twice to detect an intermittent error in a potentially 'unpure' function is arbitrary.

Not an error, but a mutation or a side effect. Yes, it is arbitrary, but we've found it works really well in practice. In fact, you can see an example of this right above in the comments: #12856 (comment). A "proper" solution to this would involve having some way to track effects on the language level. There are languages that do this (e.g. Koka) so it's conceivable that eventually JS type systems might be able to express some version of this. In the meantime, we use what works in practice.

They could argue you need 10 executions so I'm struggling to see why this is a justification for Strict Mode at the package level, when it should and could be a user-defined mode.

I'm not sure what you mean by either "justification at the package level" or a "user-defined mode". Strict Mode is a part of React precisely because user code has no control over how React calls its functions. So it's React that needs to execute these functions twice (if you can get onboard with this heuristic). You're welcome to remove StrictMode from your app if that confuses you.

@tonix-tuft
Copy link

The whole purpose of StrictMode is to find cases where the user-written functions that are supposed to be pure are, in fact, not pure. If they are pure, then there is no observable difference between development and production modes.

f they are not pure, Strict Mode helps find that issue. This is to prepare for the future, where React doesn't give any guarantees about the number of times they would be invoked, even in production. You can opt out of Strict Mode if you don't want this behavior, but it is the very purpose of Strict Mode.

@gaearon Is Strict Mode enabled by default as of React 16.13.1?

Thanks

@akhileshkcoder
Copy link

Same issue here.

handleChange(id){
    this.setState(state => {
      const newtodos = state.todos.map(todo => {
        if(todo.id === id){
          todo.completed = !todo.completed
        }
        return todo
      })
      return {todos: newtodos}
    })
  }

The handleChange function is called for handling the checkbox onChange event. Due to strict mode, it doesn't change the value of the completed. Because it is called twice it goes back to the previous state. I am using the completed to check or uncheck the checkbox. So is this not a pure function?

@tonix-tuft
Copy link

tonix-tuft commented Oct 15, 2020

@akhileshkcoder In your case, you are mutating the nth todo when todo.id === id, so, even though newtodos is a new array when you return the next state at return {todos: newtodos}, your setState callback function is not pure.

Try this instead:

handleChange(id){
    this.setState(state => {
      const newtodos = state.todos.map(todo => {
        if(todo.id === id){
          return {
              ...todo,
              completed: !todo.completed
          };
        }
        return todo
      })
      return {todos: newtodos}
    })
  }

This way the setState's callback is idempotent and even if React calls it more than once, it will lead to the correct state.

@akhileshkcoder
Copy link

@tonix-tuft Cool!. It worked. Thank you.

srand1 pushed a commit to srand1/ro-pocket that referenced this issue Nov 14, 2021
Note:
It seems `done` on line 82 is executed once (log on line 88), but the callback
of `setMsgs` is executed multiple times (log on line 92). It is expected to
execute only once, or line 56 would fail because `custom` is already parsed.

More strangely, uncommenting `try-catch` on line 54,57-59 seems to change this
multi-execute behavior/bug mysteriously. `catch` seems never executed (log on
line 58) and `setMsgs` callback seems only executes once (log on line 92).

Theorically it should be related to inplace mutation of `obj.msgs`/`msg.custom`,
but the behavior above looks more like a React codegen issue. Tracing up to the
multi-execute of callback is hard.

However, with `console.trace` or `obj.debug` (a mutating counter), or with
`debugger` or breakpoint, we can see that it actually execute 3 times without
`try-catch` and 2 times with it, (rather than 2 vs 1). For some reason
`console.log` does not work on the 2nd execution (1st and 3rd ok) but
`console.trace` works all the time. Similarly, the `catch` on line 58 is
executed but its `console.log` is muted because it is during 2nd run.

This makes a little bit more sense. With `try-catch`, the 2nd execution is no-op
and nothing gets mutated. So it works as expected. Without it, the 2nd execution
throws and somehow React decides to run it 3rd time, which finally make React
unhappy.

It is left unknown why `console.log` is less useful than `console.trace`. And
whether/how React's multi-execution of callback is related to mutation of object
captured by the closure. But anyways, the action should be a descriptive object
and reducer should be pure (or at least idempotent here).

React.StrictMode calls the callback twice intentionally.
facebook/react#12856 (comment)
Note that parsing and reversing was outside `setMsgs` in `HEAD^`, so it executes
only once as expected. For performance, we may extract sanitization unrelated to
`msgsPrev` (eg. !resend and parsing) outside of `setMsgs`.

`console.log` was overwritten as `function disabledLog() {}` by React during the
2nd call to hide its side effects.
@nvmnghia
Copy link

nvmnghia commented Feb 15, 2022

Does setter function returned from useState() have the same functionality? In my pet project (everything up-to-date), this doesn't seem to be the case as I can still side effects in the updater function (passed to the setter function) and it doesn't run twice.

@gaearon
Copy link
Collaborator

gaearon commented Feb 15, 2022

If you run your project in Strict Mode, the updater function (the one you pass to setState) should fire twice. (However, if you test it with console logs, you may be running into the fact that React 17 suppresses logs from second render — which we're changing in 18. You can save let log = console.log outside to verify.)

Your updater function should not contain side effects.

@loulou1994
Copy link

loulou1994 commented May 12, 2022

I'm facing the same issue where the setState is triggered twice and update the state accordingly. does the following snippet contain any side effect : setPlayersPick(prevState => ({ ...prevState, showResult: !prevState.showResult })) ? Thanks !

@gaearon
Copy link
Collaborator

gaearon commented May 12, 2022

No, this code seems fine.

@loulou1994
Copy link

@gaearon Yes? But still the state is updated twice though.

@rolaru
Copy link

rolaru commented Mar 3, 2023

Hi guys! Very interesting feature to detect state mutation. It surprised me when I found out about it but I'm not 100% convinced by it. I have the code below which renders a tree file/directory structure:

import { useState, useEffect, useCallback } from 'react';

import DirectoryTree from './components/directory-tree/directory-tree';

import './App.css';

function App({ fileTree }) {
  const [tree, setTree] = useState(fileTree);

  const updateTree = useCallback(() => {
    console.log('second-render');

    setTree(tree => ({
      ...tree,
      children: [
        ...tree.children,
        {
          id: 'd6c86f11-d02f-46c1-b55a-fb6c38e1eb54',
          name: 'TEST'
        }
      ]
    }));
  }, []);

  useEffect(() => {
    setTimeout(() => updateTree(), 5000);
  }, [updateTree])

  return (
    <div className="App">
      <DirectoryTree tree={tree}></DirectoryTree>
    </div>
  );
}

export default App;

Here I set the tree initial state and then, after 5 seconds, I try to update it by adding a TEST file. The problem is that in React.StrictMode adds the file twice and I can't determine in the code where I have "mutated" the state. I tried to create a clean, pure nested state at each level that I was changing.

@gaearon , @tonix-tuft , could you please advise?

@tonix-tuft
Copy link

tonix-tuft commented Mar 3, 2023

Hi, @rolaru.

Your setState updater function is pure and is fine. The issue you are experiencing is due to the new Strict Mode behaviors of React 18.

In React 18, when you are in strict mode (this happens only in development), the component is mounted twice:

mounted -> instantly unmounted -> instantly remounted

This is done so that you can ensure that your effects are not sensible to this mounting/unmounting happening multiple times during the lifetime of your application in the browser's tab.

The purpose of this new behaviour is to allow React to preserve a component's state even after that component has been unmounted in order to be able to instantly restore it later if that component is needed again.

React 17:

* React mounts the component.
  * Layout effects are created.
  * Effects are created.

React 18 with this new strict mode behaviour:

* React mounts the component.
  * Layout effects are created.
  * Effects are created.
* React simulates unmounting the component.
  * Layout effects are destroyed.
  * Effects are destroyed.
* React simulates mounting the component with the previous state. <--- This step creates your timeout again
  * Layout effects are created.
  * Effects are created.

Being aware of this, the fix to your code is straightforward: just call clearTimeout in the effect's destroy function so that only the second timeout is kept after the App component is immediately destroyed and re-mounted during the initial render.

Here is your example with the issue:

https://codesandbox.io/s/react-18-directorytree-setstate-same-update-happening-twice-issue-i84n6g

And here is the fix:

https://codesandbox.io/s/react-18-directorytree-setstate-same-update-happening-twice-fix-zqd7k3

@rolaru
Copy link

rolaru commented Mar 3, 2023

Hi, @rolaru.

Your setState updater function is pure and is fine. The issue you are experiencing is due to the new Strict Mode behaviors of React 18.
...

Thank you very much for the explanation! I now understand that by this extra measure you are trying to educate React developers to properly use lifecycle hooks and also do proper cleanup after themselves.

@tonix-tuft
Copy link

tonix-tuft commented Mar 4, 2023

@gaearon Yes? But still the state is updated twice though.

Hey @loulou1994 👋, was the issue you were facing which you mentioned in your last comment similar to the one of @rolaru? Were you using React 18? I guess you were.

@saksham2026
Copy link

`useEffect(() => {
const menu_btn = document.querySelector('.Hamburger');
const mobile_menu = document.querySelector('.mobile-nav')
menu_btn.addEventListener('click', function () {
menu_btn.classList.toggle('is-active');
mobile_menu.classList.toggle('is-active')
});

}, []);`
This is my code, in this menu_btn is clicked 2 times, even though i clicked it once. Hence the net effect of toggling is none. What is the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests