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: Bad state updates with Promise.resolve and useReducer #24650

Closed
gffuma opened this issue Jun 1, 2022 · 17 comments
Closed

Bug: Bad state updates with Promise.resolve and useReducer #24650

gffuma opened this issue Jun 1, 2022 · 17 comments
Labels
Resolution: Stale Automatically closed due to inactivity Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@gffuma
Copy link
Contributor

gffuma commented Jun 1, 2022

React version: 18.1.0

Steps To Reproduce

Code Example

import React, { useEffect, useReducer } from 'react'

function reducer(state, action) {
  switch (action.type) {
    case 'CHANGE':
      return {
        ...state,
        values: {
          ...state.values,
          [action.field]: action.value,
        }
      }
    case 'SUBMIT':
      return {
        ...state,
        submit: !state.submit,
      }
    default:
      return state
  }

}

function App() {
  const [state, dispatch] = useReducer(reducer, {
    submit: false,
    values: {
      foo: 'hello'
    }
  })

  useEffect(() => {
    dispatch({ type: 'SUBMIT' })
    Promise.resolve().then(() => {
      dispatch({ type: 'SUBMIT' })
    })
  }, [state.values])

  return (
    <div className="App">
      <input
        type="text"
        value={state.values.foo}
        onChange={(e) => {
          dispatch({ type: 'CHANGE', field: 'foo', value: e.target.value })
        }}
      />
    </div>
  )
}

export default App

I think is the same bug of #24625
But i fill this issue because this scenario breaks a lot of library out of there i found this bugs using https://github.com/jaredpalmer/formik
If you type fast or you slowdown your cpu using dev tools you see the page freeze and you will see:
Warning: Maximum update depth exceeded.

The current behavior

Not seeing Warning: Maximum update depth exceeded and freeze

The expected behavior

See Warning: Maximum update depth exceeded and freeze

@gffuma gffuma added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jun 1, 2022
@Dremora
Copy link

Dremora commented Jun 1, 2022

See also #24649. I've actually discovered it because of Maximum update depth exceeded warning, although I decided not to replicate it in my example. But the root cause is likely the same.

@Baribj
Copy link

Baribj commented Jun 16, 2022

I think the expected behavior should be no error at all.

This is an interesting scenario because if you replace the dependency array with [state.values.foo], the issue doesn't happen anymore.

This leads me to think useEffect is seeing changes in state.values object with each re-render, which cause it to fire, which updates the state and cause a re-render ..... etc ultimately getting into an infinite loop.

The question is, why does the issue happen when the dependency array has an object but doesn't happen with
primitive variables ?

@gffuma
Copy link
Contributor Author

gffuma commented Jun 16, 2022

@ritwaldev

This is an interesting scenario because if you replace the dependency array with [state.values.foo], the issue doesn't happen anymore.

I think because a literal value "has more chance" to look equal, on useEffect perspective, even if updates are applied out of order on the other hand values instance changes at every keystroke, so if it's out of sync the instance looks always different.

@antonvialibri1
Copy link

Do you think this issue could be related to the following one?

jaredpalmer/formik#3602

@gffuma
Copy link
Contributor Author

gffuma commented Jul 12, 2022

@antonvialibri1 Yes i found this issue while using formik submit in useEffect.

@antonvialibri1
Copy link

@gffuma Yeah, hopefully Formik maintainers will address this issue soon.

@gffuma
Copy link
Contributor Author

gffuma commented Aug 2, 2022

@antonvialibri1 in my opinion this is an issue with React, or at least, this behaviour should be documented.

@antonvialibri1
Copy link

antonvialibri1 commented Aug 3, 2022

@gffuma My assumption is that it's related to Automatic Batching which was introduced with React 18.

I tried debugging Formik's code to understand what happens. If you look at my issue here you will that for this Formik code:

image

image

works like this with React 17:

  • Formik's setFieldValue is called;
  • dispatch SET_FIELD_VALUE
  • Reducer for SET_FIELD_VALUE is executed (no batching)
  • validateFormWithHightPriority is then executed ✅

Whereas, with React 18:

  • Formik's setFieldValue is called;
  • dispatch SET_FIELD_VALUE
  • validateFormWithHightPriority is executed ❗(I guess it's because of batching, React doesn't execute the state reducer for SET_FIELD_VALUE right away as it used to do in React 17)
  • Reducer for SET_ISVALIDATING is executed
  • Reducer for SET_FIELD_VALUE is never executed for some reason ⚠️.

It's a tiny implementation detail, but I think it potentially affect other use cases and therefore should be treated as a React 18 breaking change, even though they mention that upgrading from version 17 to version 18 should happen seamlessly.

The Formik team could address the issue and come up with a workaround, but I agree with you, the main issue is with React 18.

@matths
Copy link

matths commented Nov 22, 2022

@jaredpalmer this is a show stopper to me, any chance for a comment or advice?

@matths
Copy link

matths commented Nov 23, 2022

@antonvialibri1 how did you solve your problem from August then?
Downgrade React, patch/fork Formik, other workaround?
Or does a switch to the legacy root api (reactwg/react-18#5) work out, until Formik is updated for the better?

@antonvialibri1
Copy link

Hi @matths,

in my case the issue was due to a useEffect calling helpers.setValue():

const SomeComponent = (props) => {
  const [field, meta, helpers] = useField(props);
  
  const [value, setValue] = useState(field.value || 'initial');
  
  useEffect(
    () => {
      if (field.value !== value) {
        helpers.setValue(value);
      }
    },
    // For React 18, I needed to remove `field.value` and `helpers` from the deps array
    // as an infinite loop was caused by `helpers.setValue(value);` when using React 18.
    // I've created an issue on the Formik repo on GitHub that's worth tracking: https://github.com/jaredpalmer/formik/issues/3602
    //
    // This might actually be a bug of `useReducer` that was introduced in React 18: https://github.com/facebook/react/issues/24650
    //
    // eslint-disable-next-line react-hooks/exhaustive-deps
    [value /*, field.value, helpers */]
  );
  
  // ...
}

The temporary solution was to comment out field.value and helpers from the useEffect's deps, as field.value may change and needs to be synced with the value/setValue state returned by the useState hook and helpers is not stable across re-renders.

I hope this issue is addressed by Formik's maintaners at some point. To me it feels like a React breaking change though.

Hope this helps!

@matths
Copy link

matths commented Nov 23, 2022

Hi @antonvialibri1,

thank you for your quick reply, but I don't get it.
My use case is a react-select dropdown with a couple of options given from outside.
Whenever options has a length of 1 only, it should set the field value to that single option if this is not already the case.
I am fully aware this would trigger a re-rendering, but only one, right?
With React 18 the field value is not changed, so it runs into an infinite loop.

const SelectFormField = ({options}) => {

  const [field] = useField(props);
  const {setFieldValue} = useFormikContext();

  useEffect(() => {
    if (options.length === 1 && field.value !== options[0]) {
      console.log("DEBUG: ", field.value, options[0]);
      setFieldValue(field.name, options[0]);
    }
  }, [options, field.name, field.value, setFieldValue]);

  return (
    <Select
      {...field}
      options={options}
    />
  );
};

I am able to wrap the setFieldValue into a flushSync to prevent this, but it produces another warning, because flushSync shall not be used in useEffect.

Leaving out field.value and setFieldValue from the deps of useEffect produces even more warnings.

And it doesn't matter if I use setValue from the useField hook or setFieldValue from the useFormikContext hook.

Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
@Janpot
Copy link

Janpot commented Apr 10, 2024

I'll bump it, given the fact that we ran into this beginning last month and are on latest versions.

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 11, 2024
Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Jul 10, 2024
Copy link

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
@maulik1729
Copy link

maulik1729 commented Oct 14, 2024

still facing this issue with react 18.2.0

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

No branches or pull requests

7 participants