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

Warning: Cannot update a component from inside the function body of a different component. #751

Closed
ChrisChiasson opened this issue Mar 1, 2020 · 49 comments · Fixed by #766

Comments

@ChrisChiasson
Copy link

ChrisChiasson commented Mar 1, 2020

Are you submitting a bug report or a feature request?

bug report

What is the current behavior?

After npm upgrading to the latest React, I am getting the following warning in my console from the React Developer tools Chrome extension:
"Warning: Cannot update a component from inside the function body of a different component."

The component trace just points to a line with a boring/standard use of Field in JSX. Quickly going through the app I'm working on, this warning happens everywhere, on every use of Field in the app. For example, the warnings point to lines like the 2nd to last line (RadioField = ...) of this file (please excuse the coding style here). Again, there is nothing special about this code; it's happening on every Form that has components that use Field.

import React from "react";
import { Field } from "react-final-form";

const Radio = ({label,input,children,className}) => {
 const { checked } = input;
  return (
    <label className={"input-radio "+ (checked? "is-checked ": "")+
     className}>
      {children && children}
      <input
        {...input}
        className="form-radio"
      />
      <var title={label}>{label}</var>
    </label>
  );
};

Radio.defaultProps = { label:"", meta:{}, input:{}, children:[] };

const RadioField = props => <Field {...props} component={Radio} type="radio"/>;

export default RadioField;

What is the expected behavior?

No warning

Sandbox Link

I'll make one if you need it, but I think the link below in Other Information succinctly explains what is happening and a hooks based workaround that react-final-form can implement.

What's your environment?

ChromeOS 79
React 16.13.0 released Feb 26th, 2020
Final Form 4.18.7
React Final Form 6.3.5

Other information

See here: https://reactjs.org/blog/2020/02/26/react-v16.13.0.html#warnings-for-some-updates-during-render

@jsalis
Copy link

jsalis commented Mar 2, 2020

I'm also seeing this warning after updating to React 16.13.0. For me, it only seems to occur when a field is conditionally rendered.

Sandbox link: https://codesandbox.io/s/empty-forest-6m0ri?fontsize=14&hidenavigation=1&theme=dark

@dsplank
Copy link

dsplank commented Mar 5, 2020

A good example of this happening is the Wizard Form Example. Any time the Previous or Next buttons are pressed, just about every Field from all pages throws this warning.

@laukaichung
Copy link

I'm also seeing this warning after updating to React 16.13.0. For me, it only seems to occur when a field is conditionally rendered.

Sandbox link: https://codesandbox.io/s/empty-forest-6m0ri?fontsize=14&hidenavigation=1&theme=dark

Yes, I got this error for a select box that is dynamically rendered.

@MartinL83
Copy link

Same error here. Using final-form-array produces this error as well.

@MartinL83
Copy link

Not a fix. But i realized that when i added the subscription prop to the form:
( subscription={{ submitting: true, pristine: true }} in my case)
The errors disappeared.

@jgoux
Copy link

jgoux commented Mar 11, 2020

A lot of popular libraries are affected by this warning : facebook/react#18178
It's a warning party in my console right now as I also use urql which is affected as well. 😂

@DASPRiD
Copy link

DASPRiD commented Mar 17, 2020

I did some analysis on this issue and found the source of the problem, at least for conditionally rendered fields, but this might also be the cause in other cases:

const [state, setState] = React.useState<FieldState>((): FieldState => {
let initialState: FieldState = {}
// temporarily disable destroyOnUnregister
const destroyOnUnregister = form.destroyOnUnregister
form.destroyOnUnregister = false
register(state => {
initialState = state
})()
// return destroyOnUnregister to its original value
form.destroyOnUnregister = destroyOnUnregister
return initialState
})

In here, the field is synchronously registered, which causes a synchronous update to the form state, which in turn synchronously triggers setState() on the Form:

So what it boils down to is that the mounting of a Field directly calls setState() on the Form. This is apparently not a problem in some cases, but in others. I'll have to do some additional analysis to figure how how to rewrite that code to use useEffect() instead. If anyone else already has a good idea, just reply, I'll keep a close eye on this thread.

@DASPRiD
Copy link

DASPRiD commented Mar 17, 2020

So, after looking (and trying to understand what the heck all the code is doing), my personal conclusion is this:

  • Instead of useField() registering the field in useState() initially, it should only be registered in the useEffect() bellow it (that is already done).
  • While the field is not registered, useField() would yield null until registration occurred.
  • Field would have to check for the returned FieldRenderProps to be null, and not render until it received a non-null value.

Upside of this idea: very easy to implement (just a couple of lines have to be changed), almost no side-effects.
Downside: it changes the useField() API, which is a public one, so this would affect projects using it directly instead of the Field component.

Personally, I see no other way to fix this problem, I'd love to hear the thoughts of one of the maintainers about this issue. If they'd agree with my conclusion, I'd go ahead and write a pull request, or they can implement that fix.

@yann-combarnous
Copy link

@erikras, any take on below approach?

So, after looking (and trying to understand what the heck all the code is doing), my personal conclusion is this:

  • Instead of useField() registering the field in useState() initially, it should only be registered in the useEffect() bellow it (that is already done).
  • While the field is not registered, useField() would yield null until registration occurred.
  • Field would have to check for the returned FieldRenderProps to be null, and not render until it received a non-null value.

Upside of this idea: very easy to implement (just a couple of lines have to be changed), almost no side-effects.
Downside: it changes the useField() API, which is a public one, so this would affect projects using it directly instead of the Field component.

Personally, I see no other way to fix this problem, I'd love to hear the thoughts of one of the maintainers about this issue. If they'd agree with my conclusion, I'd go ahead and write a pull request, or they can implement that fix.

@DASPRiD
Copy link

DASPRiD commented Mar 23, 2020

@yann-combarnous, @erikras Any update on this?

@yann-combarnous
Copy link

@yann-combarnous, @erikras Any update on this?

I am not a maintainer, just an interested party like you :).

@lfantone
Copy link

I spent a whole day looking for a solution for this issue and so far I manage to silent this error by pausing the form validations with form.pauseValidation(). In our used case this error only happens when I have a field with a validator function, I don't know why but final form is validating the initial value of the field which in most cases will be a nil value.

Adding something like do the trick:

pauseValidation();
useEffect(() => resumeValidation(), [resumeValidation]);

But, final-form is overriding the isValidationPause flag internally in some cases.

@DASPRiD
Copy link

DASPRiD commented Mar 27, 2020

That seems more like a workaround instead of actually solving the problem (like my recommendation would). Still waiting on a word from the maintainer though :)

@erikras
Copy link
Member

erikras commented Mar 30, 2020

Hi guys. Sorry to be so late on this. I got laid off at work and have had two children locked in the house with me for weeks.

Upside of this idea: very easy to implement (just a couple of lines have to be changed), almost no side-effects.

A lot have work has been done to ensure that things look right "on first render", which is why things are the way they are. This is going to be a major breaking change. I just tried it and 75% of the unit tests failed.

Downside: it changes the useField() API, which is a public one, so this would affect projects using it directly instead of the Field component.

Yep.

This needs more investigation.

@callmeberzerker
Copy link

No worries, everything will be fine @erikras . 🍻

@ChrisChiasson
Copy link
Author

ChrisChiasson commented Mar 30, 2020 via email

@erikras
Copy link
Member

erikras commented Mar 30, 2020

Yeah, I'm gonna get a job quickly; have several offers already, but it still adds additional stress to the already elevated global level.

Companies restructure and my position disappeared out from under me. Wasn't to do with my performance or skill level.

@DASPRiD
Copy link

DASPRiD commented Mar 30, 2020

Another idea, but that one's kinda hacky, which is why I didn't propose it: Have the setState() in the Form be called via window.requestAnimationFrame().

@erikras
Copy link
Member

erikras commented Mar 30, 2020

I have some good news for you guys....

@lookfirst
Copy link
Contributor

@erikras Just looking at the code and it is a tad bit hard to follow for me... would you mind explaining why notifyFieldListeners() has an optional 'name' passed into it and the usage there? It is called quite a bit and seems to only get name passed in when silent is true now? Not sure what 'safeFields' is.

@erikras
Copy link
Member

erikras commented Mar 30, 2020

Published fix in v6.4.0 and final-form@4.19.0.

Summary: useField() was doing a quick subscribe-and-unsubscribe of the field on first render to get the initial state. React v16.13.x doesn't like this, as it was calling setState() in the containing <Form/> component. So I've added a silent flag to form.registerField() which allows this register-unregister trick to happen without any ripples going out to other subscribers. This solves the problem.

would you mind explaining why notifyFieldListeners() has an optional name passed into it and the usage there?

That means "only notify listeners to this one field". It doesn't seem to have any other usages, so it's vestigial from an earlier time, but it's just what was needed in this case.

It is called quite a bit and seems to only get name passed in when silent is true now?

Yes. The rest of the times, it really does need to notify all the other fields, as they can depend on each other.

Not sure what safeFields is.

A copy of the fields structure must be made to iterate through, since some of the things that get called can result in fields being unregistered.

@lookfirst
Copy link
Contributor

@erikras Thank you so much for the clarification, that helps a lot!

@shinjini-saha
Copy link

@erikras I noticed that the silent trick does not work for fields with validators since during the unsubscribe, line 902 still runs https://github.com/final-form/final-form/blob/64225dd3589020297c5e23209630ac31afd46c5c/src/FinalForm.js#L902

@lfantone
Copy link

I'm still having issues with the new version, also using fields with validation.

@fernard
Copy link

fernard commented Apr 1, 2020

Same here, i've upgraded to the newest versions of final-form and the issue still remains.

@yann-combarnous
Copy link

React Final Form Arrays just uses useField from this library. If you upgrade this library, I don't see why it wouldn't work.

Published an example sandbox, see #768 . As mentioned by other users above, this seems to be linked to validators.

@erikras
Copy link
Member

erikras commented Apr 1, 2020

Published fix in final-form@4.19.1. It fixes that sandbox, @yann-combarnous.

@yann-combarnous
Copy link

yann-combarnous commented Apr 2, 2020

Published fix in final-form@4.19.1. It fixes that sandbox, @yann-combarnous.

Thanks @erikras . Still getting one scenario where this error shows up, it seems. Seems to happen on submit when form.registerField is used. Replacing it with OnChange from react-final-form-listeners also seems to not trigger the error.

Will try and narrow down the specific pattern, have made a short try today at a CodeSandbox, with no luck so far. Stack trace:

Screenshot 2020-04-02 at 11 05 30

@atuttle
Copy link

atuttle commented Apr 2, 2020

I have seen a reduction in these errors in my app, but they are not completely eliminated.

I have a form that shows some assumed values and gives the user the option to edit them by clicking on a link to make the form fields appear. Previously, that click would throw a couple of these errors (I think 2 or 3? odd since there are 5 fields). Those errors no longer appear. However, that form also includes a checkbox that, when toggled a certain way, adds an additional field. Adding that additional field is still throwing the error.

I'm not a frequent user of CodeSandbox, but hopefully I've done this right. The sandbox from @jsalis demonstrated my issue (checkbox to make field appear) so I forked it and updated the final-form version to 4.19.1, and the error still happens there: https://codesandbox.io/s/react-16130-final-form-warning-tnid2

@callmeberzerker
Copy link

@atuttle yep I have the very same error with checkbox adding a new Field. :)

@yann-combarnous
Copy link

I'm not a frequent user of CodeSandbox, but hopefully I've done this right. The sandbox from @jsalis demonstrated my issue (checkbox to make field appear) so I forked it and updated the final-form version to 4.19.1, and the error still happens there: https://codesandbox.io/s/react-16130-final-form-warning-tnid2

You also need to upgrade react-final-form to 6.4.0.

@atuttle
Copy link

atuttle commented Apr 6, 2020

@yann-combarnous

You also need to upgrade react-final-form to 6.4.0.

That fixed it for me, thanks! (@callmeberzerker fyi)

@jwld
Copy link

jwld commented Apr 12, 2020

I'm seeing this when using useField in conjunction with a select field built with downshift. Using the latest version of both packages.

I've solved it temporarily by wrapping the offending function in setTimeout:

setTimeout(() => input.onChange(state.selectedItem.value))

Larger section of code for context:

import { useSelect } from 'downshift'

const onStateChange = state => {
  if (state.selectedItem) {
    // setTimeout quiets react warning
    setTimeout(() => input.onChange(state.selectedItem.value))
  }
}

const {
  isOpen,
  selectedItem,
  getToggleButtonProps,
  getLabelProps,
  getMenuProps,
  highlightedIndex,
  getItemProps
} = useSelect({
  items,
  itemToString: item => (item ? item.label : ''),
  onStateChange,
  selectedItem: items.find(item => item.value === input.value)
})

@umairshahid436
Copy link

I have created a generic dropdown menu component using (Material-Ui Autocomplete), so I can use where I need. But I am facing a warning,

"Warning: Cannot update a component (ProductSubCategory) while rendering a different component (ForwardRef(Autocomplete)). To locate the bad setState() call inside ForwardRef(Autocomplete)"

Note

I followed different links but couldn't get the proper answer I have a slightly different scenario. I have created a generic dropdown menu component and use inside other components where needed.

Here is code of dropdown menu component
`

const AsyncDropDownMenu: React.FC<IAsyncDropDownMenu> = memo(
 ({
title,
id = "asynchronous-dropdown-menu",
loadData,
onSelect,
fieldError,
}) => {
 const [open, setOpen] = React.useState(false);
const [options, setOptions] = React.useState<IOptionTypes[]>([]);
const loading = open && options.length === 0; 
const dispatch = useDispatch();   

useEffect(() => {
  let active = true;
  if (!loading) {
    return undefined;
  }
  (async () => {
    const data = await dispatch(loadData());
    if (active) {
      setOptions(data);
    }
  })();    

  return () => {
    active = false;
  };
}, [loading]);    

useEffect(() => {
  if (!open) {
    setOptions([]);
  }
}, [open]);    

return (
  <Autocomplete
    id={id}
    open={open}
    onOpen={() => {
      setOpen(true);
    }}
    onClose={() => {
      setOpen(false);
    }}
    onChange={(event, value) => seletOption(value)}
    getOptionSelected={(option, value) => {
      onSelect(value);              // When I call onSelect then I got warning, which I mentioned above.
      return option.name === value.name;
    }}
    getOptionLabel={(option) => option.name}
    options={options}
    loading={loading}
    renderInput={(params) => (
      <TextField
        {...params}
        label={title}
        helperText={fieldError.helperText}
        error={fieldError.error}
        InputProps={{
          ...params.InputProps,
          endAdornment: (
            <>
              {loading ? (
                <CircularProgress color="inherit" size={20} />
              ) : null}
              {params.InputProps.endAdornment}
            </>
          ),
        }}
      />
    )}
  />
);
}
);
export { AsyncDropDownMenu };`

Here is the code of Component in which I am using dropdown menu component

`

const ProductSubCategory: React.FC = () => {
const classes = CommonStyle();
const subCatList=[];
const dispatch = useDispatch();

//#region error states and function
 const [selectedOption, setSelectedOption] = React.useState({
      id: 0,
      name: "",
  });

 const [categoryError, setCategoryError] = React.useState({
    error: false,
    label: "",
    helperText: "",
    validateInput: false,
  });
 const onError = (action: Function, message: string = "Required field") => {
   action({
         error: true,
        label: "required",
        helperText: message,
        validateInput: true,
   });
 };

 //#endregion

 const onselect = (value: any) => {
 if (categoryError.validateInput) {
  setCategoryError({
     error: false,
     label: "",
     helperText: "",
     validateInput: false,
    });
  }
  setSelectedOption(value);
};

//#region grid columns
  const columns = [
  {
    title: "Category",
    render: (rowData: ITemp) => {
     return rowData.categories ? rowData.categories.name : "";
   },
 field: "category",
  lookup: {},
   editComponent: (props: any) => (
     <AsyncDropDownMenu
       title="Category"
       loadData={loadCategories}
       onSelect={onselect}
       fieldError={categoryError}
     ></AsyncDropDownMenu>
   ),
 },   
];

//#endregion

//#region validate input data
const onValidate = (
  data: any,
  reject: Function,
  resolve: Function,
  action: Function,
 isUpdate: boolean,
 oldData: any
) => {
  if (Object.keys(data).length === 0 && selectedOption.id === 0) {
    onError(setNameError);
    onError(setCategoryError);
    reject();
  } else if (selectedOption.id === 0) {
    onError(setCategoryError);
    reject();
  } else {

    if (isUpdate) {
      if (data !== oldData) dispatch(action(data, reject, resolve));
      else reject();
    } else {
      dispatch(action(data, reject, resolve));
    }
  }
};
//#endregion
 return (
        <div>
                   <CustomMatrialTable
                      title="Product Sub Categories"
                     isLoading={isLoading}
                     col={columns}
                     data={subCatList}
                   ></CustomMatrialTable>
             </div>
           );
          };
      export { ProductSubCategory };

`

@kopax
Copy link

kopax commented Jul 6, 2020

I have the same issue with ff 4.19.1 and rff 6.5.0. I have not found a way to read the form state from outside my component safely. Is there a workaround?

@tonydehnke
Copy link

Looks like I am seeing the same issue with:

    "final-form": "4.20.1",
    "final-form-arrays": "3.0.2",
    "react-final-form": "^6.5.1",
    "react-final-form-arrays": "3.1.
Warning: Cannot update a component (`Editor`) while rendering a different component (`FormSpy`). To locate the bad setState() call inside `FormSpy`, follow the stack trace as described in https://fb.me/setstate-in-render
    in FormSpy (created by Editor)
    in fieldset (created by ReactFinalForm)
    in form (created by ReactFinalForm)
    in ReactFinalForm (created by DetailsForm)
    in DetailsForm (created by Editor)
    in div (created by TabPanel)
    in TabPanel (created by Editor)
    in div (created by TabPanels)
    in DescendantProvider (created by TabPanels)
    in TabPanels
    in TabPanels (created by Editor)
    in div (created by Tabs)
    in DescendantProvider (created by Tabs)
    in Tabs (created by Editor)
    in Editor (created by Context.Consumer)
    in Route (created by V12Scheduler)
    in V12Scheduler (created by App)
    in Router (created by HashRouter)
    in HashRouter (created by App)
    in ThemeProvider (created by ThemeProvider)
    in ThemeProvider (created by App)
    in App```

@mynamesleon
Copy link

mynamesleon commented Sep 9, 2020

I've been encountering this too with field validation updating the form.

Package versions:

"final-form": "^4.20.1",
"final-form-arrays": "^3.0.2",
"react-final-form": "^6.5.1",
"react-final-form-arrays": "^3.1.2",

Console error:

Warning: Cannot update a component (`ForwardRef(Field)`) while rendering a different component (`ForwardRef(Field)`).

For the time being, I've had to resort to the workaround suggested by @lfantone - feels almost hacky, but works wonderfully.

pauseValidation();
useEffect(() => resumeValidation(), [resumeValidation]);

@Kazragone
Copy link

I've been encountering this too with field validation updating the form.

Package versions:

"final-form": "^4.20.1",
"final-form-arrays": "^3.0.2",
"react-final-form": "^6.5.1",
"react-final-form-arrays": "^3.1.2",

Console error:

Warning: Cannot update a component (`ForwardRef(Field)`) while rendering a different component (`ForwardRef(Field)`).

For the time being, I've had to resort to the workaround suggested by @lfantone - feels almost hacky, but works wonderfully.

pauseValidation();
useEffect(() => resumeValidation(), [resumeValidation]);

where did u use that useeffect?? i am using form with render props

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

Successfully merging a pull request may close this issue.