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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
RyanAtViceSoftware opened this issue Apr 20, 2020 · 13 comments

Comments

@RyanAtViceSoftware
Copy link

馃悰 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
Copy link
Collaborator

johnrom commented Apr 21, 2020

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
Copy link
Author

@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
Copy link
Collaborator

johnrom commented Apr 21, 2020

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
Copy link

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
Copy link
Collaborator

johnrom commented Apr 26, 2020

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
Copy link
Author

@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
Copy link
Collaborator

johnrom commented Apr 28, 2020

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
Copy link

alexandermckay commented Nov 5, 2020

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

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

@maddhruv maddhruv closed this as completed Nov 6, 2020
@Powersource
Copy link

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

@johnrom johnrom reopened this Nov 25, 2020
@johnrom
Copy link
Collaborator

johnrom commented Nov 25, 2020

Needs further evaluation

@sa-webb
Copy link

sa-webb commented Dec 21, 2020

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
Copy link

soumyajitdutta commented Feb 19, 2021

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
Copy link

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.

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

9 participants