-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 switching results in merged formData #231
Comments
I didn't test your patch (maybe a jsfiddle would be more convenient), but at first glance it seems to me that the Form should just use what's provided as a formData object, meaning it's the responsibility of the developer to not mutate it before it's passed as a prop to the component. Reminds me the conversation in #181. |
I have not mutated the formData that I pass in. I render a form (specifying it's formData), I then render a different form (specifying it's formData also). I then rerender the original form (again specifying the unmutated original formData). If you then modify one of the fields and submit, the formData that is submitted has data in it from the other form. The only way this could be the case is if it is somehow being saved inside the Form component class. In my example, I store formData per form, so there is no possibility that the intermediate form mutated the original/final form's formData. |
I just pushed another commit, logging the explicitly passed in formData when rendering each form. It is unmutated.
|
As a jsfiddle: https://jsfiddle.net/f2y3fq7L/5/ |
Thanks for the fiddle, it really helped. There's indeed an issue, seems like we're keeping a reference to defaults. Definitely a bug. |
I guess it is probably caused by copying props to state. Sometime unavoidable, but perhaps not in this case? https://facebook.github.io/react/tips/props-in-getInitialState-as-anti-pattern.html |
Not sure if this is related, but I'm seeing similar behavior on the playground site. To reproduce:
Then you'll see "age" pop back up in the formData. |
While most components are stateless, the Using something like redux would basically move the problem elsewhere, adding more code and levels of indirection while we would still have to manage field state. Note: this might be an option eventually, but right now I'm reluctant at introducing more complexity to a somewhat already complex library (forms are complex anyway). I'm working on fixing the bug atm anyway :) |
Please provide feedback on #234. |
Released in v0.33.2 |
I'm not familiar with the process for publishing to npm, but I noticed it hasn't updated on npm yet. Possibly there is just a delay involved? |
Oops, forgot to run the publish command; should be published now. |
Prerequisites
Description
When switching between forms, displaying one form, then another, then back to the original, the submitted formData will be merged between the two forms.
Steps to Reproduce
I made a minimal example here: https://github.com/dpwrussell/react-jsonschema-form/tree/broken
Just clone that, npm install, npm start. There are two hyperlinks in the top-right corner to switch between the two forms.
Expected behavior
formData would be exclusively of the current 'Simple' form.
Actual behavior
formData includes data from 'Large' form.
Version
master branch and released versions
The text was updated successfully, but these errors were encountered: