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

Form's onChange callback does not provide new formData if Form is rerendered immediately after custom widget calls props.onChange #513

Closed
2 tasks done
hodavidhara opened this issue Mar 15, 2017 · 9 comments
Labels

Comments

@hodavidhara
Copy link

Prerequisites

  • I have read the documentation;
  • In the case of a bug report, I understand that providing a SSCCE example is tremendously useful to the maintainers.

Description

I am seeing an issue where you receive old data if the Form component is rerendered after your custom widget has called props.onChange. This sounds like it would be a race condition, or at least some edge case that rarely ever happens, but it is consistently reproducible if a component that wraps Form calls setState in the onBlur callback. Note that this isn't a case of getting one change old data, but the formData never being updated so you are stuck with the default value unless it is changed through some external mechanism.

Steps to Reproduce

Go to https://jsfiddle.net/3e95vf8b/3/ and see instructions there.

Expected behavior

I would expect to always get the current formData in the onChange callback.

Actual behavior

I get stale formData in the onChange callback.

Version

0.43.0

Additional Debugging Notes

I did some digging on my own and think I understand why this is happening. I've put these findings in comments in the jsfiddle, but will try to detail them here as well. No matter the order that our custom widget component calls the onChange or onBlur events, the Form wrapping component's (call it FormWrapper) handleBlur always executes before handleChange because react-jsonschema-form's Form component calls setState asynchronously for onChange events and only calls its props.onChange event after that setState has resolved, whereas Form calls its props.onBlur method immediately. Then, the setState in FormWrapper's handleBlur function causes a rerender with old formData because FormWrapper hasn't yet been notified of the change. That causes Form::componentWillReceiveProps to fire and update the state of the Form, overwriting the changes to the formData with the old values. Then the callback for the original async setState executes and props.onChange is fired with this.state which has the old formData.

The only workaround I've found was to wrap the setState call that happens in FormWrapper's handleBlur method in a setTimeout to ensure it only fires after we get the latest change, but that doesn't feel great.

@n1k0
Copy link
Collaborator

n1k0 commented Mar 17, 2017

I just quickly played with your jsfiddle and couldn't figure out why you wouldn't call onChange while updating internal state in your custom widget. Tried and it worked https://jsfiddle.net/n1k0/wqjzvsvh/2/

Disclaimer: didn't get too much into the details there.

@hodavidhara
Copy link
Author

I tried to explain it in the comment above the component. Our real custom component is more complex than the one in the fiddle, I was just trying to simplify the example as much as possible. The real component I'm having a problem with is a pill container (or labels, or tags). which is used for array types where the items are strings. We are only calling onChange when a pill is created or removed by the user, and one of the times a pill is created is when the user blurs out of the input with text still in it.

It's similar to something like this: https://www.npmjs.com/package/react-tag-input

@glasserc
Copy link
Contributor

Sorry for the delay! This is a complicated one.

I thought this was related to #446, but I see you're already passing safeRenderCompletion=true.

After reading your example carefully, and experimenting a little bit, I agree with your analysis.

I also noticed that it seems like when componentWillReceiveProps is called, the state hasn't changed yet. (I added a console.log to Form.js at the beginning of getStateFromProps to show the old state, and it shows the previous/default value.) This means we can't just insert a hack to rely on this.state in getStateFromProps, because that state hasn't been updated yet.

Once we finish in getStateFromProps, then it calls setState with a formData, and I think React is merging these two setState calls, which wipes out the change that would be made by the first setState. As a hack, I tried putting a check in componentWillReceiveProps to see if the old props are the same as the new props -- but they aren't, because formContext is being set to {root_name: true}. Couldn't updating formContext change the defaults, which could change the formData? If so, I think the behavior might be correct, strange as it is. Otherwise I'm not sure what to do -- we probably only need to reset formData if certain fields change -- schema and formData (of course).

Thinking about this deeper, you've managed to construct a scenario where a setState happens, and then before the setState is complete, a componentWillReceiveProps happens. Because of this, componentWillReceiveProps can't trust this.state, so computing things like errors is going to totally break in a way that I can't think of how to fix. Maybe the easiest way to address this is to prevent setState from being batched with componentWillReceiveProps? One possible way to do that would be to not update formContext in handleBlur -- what does formContext mean for you and why does it need to be updated as part of the blur? Could you do it as part of onChange, as in https://jsfiddle.net/9ntrq2mf/?

@n1k0
Copy link
Collaborator

n1k0 commented Apr 21, 2017

I think React is merging these two setState calls, which wipes out the change that would be made by the first setState

That reminds me some discussions I've seen on Twitter about always using the callback form of setState, eg.

this.setState((state) => {
  return {...state, foo: "updated"};
})

The reasonning was that this way updates are queued and processed in order, avoiding race conditions and inconsistent resulting state. We may be on something here, might be worth exploring further in this direction I think.

@charlysotelo
Copy link

charlysotelo commented Dec 19, 2019

I'm facing this issue now as well. Would be nice if it could be solved. I will update if I come up with a workaround

EDIT:

I fixed this issue by doing several things:

  1. store formData in the toplevel's component state
  2. store this.setState (or a function wrapping it) in the toplevel's component state.
  3. instead of chaining up the this.props.onChange, call this.setState() using the (prevState) => {} syntax as so:

this.setState((state) => { // Modify state.formData here. // // Use props.id, or props.idSchema['$id'] to get a path to your component // return state; });

@epicfaace epicfaace added the bug label Jan 16, 2020
@AlexMachin1997
Copy link

This sounds like an issue I'm having.

I'm updating a custom widgets and then calling the onChange, then the Forms onChange event shows the updated data but then it's wiped from the form data.

Are there any workarounds for now ?

@S4NT14G0
Copy link

I'm having similar issues related to the form onChange being called even if it's not explicitly called in the custom autocomplete widget I'm working on.

I'm writing an autocomplete text field custom widget that uses Autosuggest under the hood. However, each time I type into the field the parent form onChange gets called and causes my widget to re-render. My plan was to let them type and show a list of potential selections. If they select an autocomplete suggestion I would call props.onChange(selection.name) or if they lose focus from the text field we would use the current value and call props.onChange(currentSearchValue). This doesn't seem possible if the form onChange is being called first since it reloads my custom widget. Not sure if there's a better approach for autocomplete.

@timruiterkamp
Copy link

I'm having similar issues related to the form onChange being called even if it's not explicitly called in the custom autocomplete widget I'm working on.

I'm writing an autocomplete text field custom widget that uses Autosuggest under the hood. However, each time I type into the field the parent form onChange gets called and causes my widget to re-render. My plan was to let them type and show a list of potential selections. If they select an autocomplete suggestion I would call props.onChange(selection.name) or if they lose focus from the text field we would use the current value and call props.onChange(currentSearchValue). This doesn't seem possible if the form onChange is being called first since it reloads my custom widget. Not sure if there's a better approach for autocomplete.

Hi @S4NT14G0, did you find any solution on this? We are running into the same issue right now where the whole form keeps re-rendering because somehow the onChange is firing with every react component update. I am currently trying to work through react context to keep the data steady and not wiping everything when the onChange is firing. But this is not really best practice and is quite buggy due to formData updates also re-rendering everything.

@heath-freenome
Copy link
Member

heath-freenome commented Aug 28, 2022

Fixed in v5 beta via #3052, see the 5.x migration guide

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

No branches or pull requests

9 participants