react-component / react-component/field-form
getFieldsValue(true) allows for direct mutation of the form store
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 1k
- Forks
- 286
- Avg merge
- 4d 16h
- Merged PRs (30d)
- 1
Description
Repro:
Set up any basic form, and give a field an onChange like this:
() => {
const formData = form.getFieldsValue(true)
delete formData.nameOfField
console.log(form.getFieldsValue(true))
}
Normally, one would expect that this should have no effect, as react/state context should be immutable, but in this case you will find that it will log an object missing the deleting field.
The problem comes from the implementation of getFieldsValue here:
https://github.com/react-component/field-form/blob/d7892873e9eee6e4c618d4289fcdbc9ee240f68f/src/useForm.ts#L271-L273
Which just returns the store object directly.
This should be following React conventions and returning a clone, as it does when a specific list of NamePaths is requested.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at src/useForm.ts around lines 271-273 and reproduce the issue with getFieldsValue(true) and deletion of a field. Confirm the returned value shares mutable state with the form store; done means deleting a field from the returned object no longer changes a later getFieldsValue(true) result, while the existing behavior for specific NamePaths remains intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100