jaredpalmer / formik

Build forms in React, without the tears 😭
https://formik.org
Apache License 2.0
34.02k stars 2.79k forks source link

Warning: Can't perform a React state update on an unmounted component #2430

Open RyanAtViceSoftware opened 4 years ago

RyanAtViceSoftware commented 4 years ago

🐛 Bug report

Current Behavior

I'm getting this error

index.js:1 Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function. in Formik (at SignIn.js:24)

For this component

import React from 'react'
import { Redirect, useLocation } from 'react-router-dom'
import Form from 'react-bootstrap/Form'
import Button from 'react-bootstrap/Button'
import { Formik } from 'formik'
import { useSelector, useDispatch } from 'react-redux'
import BusyIndicator from '../widgets/busyIndicator'
import { selectIsAuthenticated, signIn } from './userContext'

const SignIn = () => {
    const location = useLocation()

    console.log(location)

    const isAuthenticated = useSelector(selectIsAuthenticated)
    const dispatch = useDispatch()

    return (
        <>
            {isAuthenticated ? (
                <Redirect to={location.state.from} />
            ) : (
                <BusyIndicator>
                    <Formik
                        initialValues={{ email: '', password: '' }}
                        onSubmit={(values, { setSubmitting, resetForm }) => {
                            // When button submits form and form is in the process of submitting, submit button is disabled
                            setSubmitting(true)

                            // Simulate submitting to database, shows us values submitted, resets form
                            dispatch(signIn(values)).then(() => {
                                resetForm()
                                setSubmitting(false) // <=== warning happens here
                            })
                        }}
                    >
                        {({
                            values,
                            // errors,
                            // touched,
                            handleChange,
                            handleBlur,
                            handleSubmit,
                            isSubmitting,
                        }) => (
                            <Form onSubmit={handleSubmit}>
                                <Form.Group controlId='formBasicEmail'>
                                    <Form.Label>Email address</Form.Label>
                                    <Form.Control
                                        name='email'
                                        type='email'
                                        placeholder='Enter email'
                                        value={values.email}
                                        onChange={handleChange}
                                        onBlur={handleBlur}
                                    />
                                    {/* <Form.Text className='text-muted'>
                            We'll never share your email with anyone else.
                        </Form.Text> */}
                                </Form.Group>

                                <Form.Group controlId='formBasicPassword'>
                                    <Form.Label>Password</Form.Label>
                                    <Form.Control
                                        name='password'
                                        type='password'
                                        placeholder='Password'
                                        value={values.password}
                                        onChange={handleChange}
                                        onBlur={handleBlur}
                                    />
                                </Form.Group>
                                <Button variant='primary' type='submit' disabled={isSubmitting}>
                                    Submit
                                </Button>
                            </Form>
                        )}
                    </Formik>
                </BusyIndicator>
            )}
        </>
    )
}

export default SignIn

Expected behavior

To not get a warning

Reproducible example

https://github.com/vicesoftware/react-redux-hooks-boilerplate/tree/formik-warning-demo

  1. clone repo
  2. cd webapp
  3. npm install
  4. npm start
  5. open dev tools in your browser
  6. open http://localhost:3000/signin
  7. log in with ryan@vicesoftware.com and password

see warning

Suggested solution(s)

Additional context

Your environment

Software Version(s)
Formik 2.1.4
React 16.13.1
TypeScript
Browser chrome
npm/Yarn 6.14.3
Operating System mac os
johnrom commented 4 years ago

It looks like the warning is occurring because by the time you call signIn, isAuthenticated is going to start returning true, so you're going to start rendering <Redirect /> instead of your <BusyIndicator />; thus, since Formik is no longer rendering, it was already unmounted, and can no longer be updated. Basically, you don't need to do anything in the then() because by the time it is called, Formik is gone and there's nothing to reset. You can either remove the then() callback, or hide formik via css instead of removing it from the render.

RyanAtViceSoftware commented 4 years ago

@johnrom I maybe approaching this wrong but how would I handle and error coming back from the server then? Seems like my current design needs to be rethought.

johnrom commented 4 years ago

I'm not sure the signature of your sign In method, but if it returns a condition that results in Formik continuing to render, you can continue to use your callback with that check.

signIn().then(result => {
  if (!result.isAuthenticated) {
    setSubmitting(false);
    // etc
  } 
});
Philipp91 commented 4 years ago

The warning maybe possible to work around in this particular example, but within a more complex application setup, I think it's reasonable to do:

@withFormik({
    handleSubmit: async (values, {props, setSubmitting}) => {
        try {
            await props.callSomeServerAction(values);
        } finally {
            setSubmitting(false);
        }
    },
})
class TheComponent extends React.Component {...}

Maybe this issue is specific to class components and doesn't happen with functional components. Maybe it's wrong to do setSubmitting(false) in a finally block?

If one of the impacts of callSomeServerAction() is to unmount the component (from the "outside"), then it's quite difficult to learn about this here and to skip the setSubmitting(false) call.

Or continuing the example above, imagine that result.isAuthenticated is a really complex condition that (1) is already evaluated elsewhere higher in the stack and (2) involves other values present (only) there. It would be nice/convenient if the form didn't have to re-evaluate this and could safely call setSubmitting().

johnrom commented 4 years ago

I wonder if it would be easier for Formik to check whether it is still mounted within setSubmitting. There should technically be no issue here. The warning is mostly for situations where code continues depending on unmounted components.

Once this callback is completed, references to the component should be garbage collected properly and thus there shouldn't be a memory leak - but I don't have a repro in front of me to confirm that. If that's the case, there's no problem calling setSubmitting after the component unmounts and we should make the check within Formik if possible.

RyanAtViceSoftware commented 4 years ago

@johnrom I think that's a good suggestion.

Here's the repo that has the issue: https://github.com/vicesoftware/react-redux-hooks-boilerplate

It's a boilerplate so it's not over complex and weighted down with business logic. It's well documented and quick and easy to run. It's the signin component that has the issue.

johnrom commented 4 years ago

It's not ideal, but you can check if the future state of redux is isAuthenticated like this:

const MyForm = () => {
  const store = useStore();
  return <Formik 
    onSubmit={() => signIn.then(() => {
      if (!store.getState().authentication.isAuthenticated) {
        resetForm();
        setSubmitting(false);
      }
    })}
  />;
}

I'm still on the fence about whether to make this check in Formik.

alexandermckay commented 4 years ago

My solution is just to check if location.pathname has not changed.

if (location.pathname === '/sign-up') setSubmitting(false)

Powersource commented 4 years ago

Is that the proper solution or just a workaround around a formik bug? I.e. should this actually be closed?

johnrom commented 4 years ago

Needs further evaluation

sa-webb commented 3 years ago

I encountered this issue when a field error was returned from my server because of how I was rendering my loading spinner. My errors were trying to return from the server and my Formik component was unmounted. i.e. I do not believe this is a Formik specific issue.

My code..

const SignIn = () => {
   const [{fetching}, signIn] = useSignInMutation();

   // THIS WAS CAUSING MY ERROR
   if(fetching) {
      return <LoadingSpinner />
   }
   ....
   return (
     // RESOLVED BY CALLING MY LOADING SPINNER HERE
     {isSubmitting && <LoadingSpinner />}
   )

}

Note* I was not having this issue with a successful login.

soumyajitdutta commented 3 years ago

I solved my issue with the following solution, it worked for me. Place the 'isAuthenticated' just outside of your

not outside of , then Formik component will not be unmounted until 'isAuthenticated' is true, hence solving the warning issue.

Below is your possible solution.

{isAuthenticated ? (
              <Redirect to={location.state.from} />
            ) : (
<Form onSubmit={handleSubmit}>
...etc.
fastfedora commented 3 years ago

I encountered this bug in a different scenario where I couldn't simply check a store state.

FWIW, I think it's reasonable for Formik to handle this, since the usage of setState within actions.setSubmitting is an implementation detail. Having to manage Formik's internal state code outside of Formik is non-intuitive.

That said, I was able to fix this in my code using this method of using hooks to see if my wrapper component was mounted before calling setSubmitting.

Ideally this check would be made within Formik; it was a minor fix for me, since I have a wrapper Form component that all my forms use. But anyone who is using Formik without a wrapper component will have to have separate code in each form to perform this check.