formik icon indicating copy to clipboard operation
formik copied to clipboard

updates during validation on unmounted component

Open artola opened this issue 4 years ago • 8 comments

Bug report

Current Behavior

When used the prop validateOnMount={true} the component runs the validation as required (using validate and/or validationSchema produces the same behaviour), but if the component is unmounted asap it performs some state updates (dispatch) even when an unmounted component should not.

Expected behavior

Do not change state once unmounted.

Reproducible example

https://codesandbox.io/s/formik-codesandbox-template-bug-ku21b?file=/index.test.js

See console error:

Warning: An update to Formik inside a test was not wrapped in act(...).

When testing, code that causes React state updates should be wrapped into act(...):

act(() => {
  /* fire events that update state */
});
/* assert on the output */

This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act
    at Formik (https://ku21b.csb.app/node_modules/formik/dist/formik.esm.js:1062:19)
    at MyForm

Suggested solution(s)

At least in both places where it was used useEffect (with validateOnMount as dependency) should be replaced by useLayoutEffect.

https://github.com/formium/formik/blob/b9cc2536a1edb9f2d69c4cd20ecf4fa0f8059ade/packages/formik/src/Formik.tsx#L335 https://github.com/formium/formik/blob/b9cc2536a1edb9f2d69c4cd20ecf4fa0f8059ade/packages/formik/src/Formik.tsx#L412

With this change, it works as expected for the example provided. It is not clear to me if some other places also should change to useLayoutEffect, specially because of React v17 different handling of them:

https://github.com/formium/formik/blob/b9cc2536a1edb9f2d69c4cd20ecf4fa0f8059ade/packages/formik/src/Formik.tsx#L165

  React.useEffect(() => {
    isMounted.current = true;

    return () => {
      isMounted.current = false;
    };
  }, []);

Additional context

React v17

Your environment

Software Version(s)
Formik 2.2.9
React 17.0.2
@testing-library/react 12.1.2

artola avatar Nov 27 '21 14:11 artola

This might be fixed in #3231 just because the underlying state manager useSubscription checks for unmounts, I think.

johnrom avatar Nov 29 '21 19:11 johnrom

@johnrom It might be that v3 version fix it, but it is not released and we should have a quick-fix for v2 time. @jaredpalmer Should we replace in v2 the above mentioned useEffect (may some other also) by useIsomorphicLayoutEffect because of React v17?

artola avatar Nov 29 '21 20:11 artola

Is there any eta on v3? Seems like it would fix a bunch of people's problems and enahnce existing use-cases so it's a shame that it's kind of being left in limbo for so long now. 🤔

mikoz93 avatar Dec 01 '21 17:12 mikoz93

Just as an FYI for anyone who has seen this bug, I've discovered that if you put the render inside an act, the warnings will go away.

So you can replace

render(<ComponentContainingFormik/>)

with:

await act(async () => render(<ComponentContainingFormik/>))

And the warning go away. At least in my case :smile:

wwahammy avatar Dec 03 '21 19:12 wwahammy

@wwahammy With your "solution" the warning is not printed, but the code is still running. I found that it is related to Promise.all used in the validation. If we do not unmount or cleanup inside the same test case, because even if RTL adds the cleanup to afterEach, it seems not enough.

See: https://github.com/testing-library/react-testing-library/issues/999

I hope the @gaearon could help us to understand it.

artola avatar Dec 03 '21 20:12 artola

Thanks for clarifying @artola!

wwahammy avatar Dec 03 '21 22:12 wwahammy

I’m on a vacation. If there’s some problem or question about React please file an issue on React issue tracker.

gaearon avatar Dec 04 '21 06:12 gaearon

I am using another workaround, might help someone:

declare module 'react-dom/test-utils' {
  export function act<T>(callback: () => Promise<T>): Promise<T>;
}
  it('should render', async () => {
    const {getByTestId} = await act(async () =>
      render(
        <Foo />,
      ),
    );

artola avatar May 05 '22 13:05 artola