feat: implement isChanged flags #125
Conversation
| controller?: FormController | ||
| ): FormHandle<Values, Err> => { | ||
| const { state, methods } = useFormtsContext<Values, Err>(controller); | ||
|
|
||
| // TODO: create cache for form handle atoms to avoid memory leaks and redundant computations |
There was a problem hiding this comment.
will be dealt with in next PR
| value, | ||
| changed, |
There was a problem hiding this comment.
we can compute changed here based on value, without the need to introduce a new Atom out of .fuse (line 57), by simply comparing value and initialValue in the combinator body
| changed, | |
| changed: !deepEqual(value, initialValue) |
There was a problem hiding this comment.
I'd keep it - there is a small difference - as a separate atom deepEqual will only run on value changes, but when put in the combinator fn it would run on change of any dependency (and it is somewhat heavy operation). I think we will need to remove FieldDependenciesAtom and move error and validating state computations here in the future as part #127 (so it will make bigger difference)
| Atom.fuse( | ||
| (...fieldsChanged) => fieldsChanged.some(Boolean), | ||
| ...values(Schema).map(field => { | ||
| const fieldLens = impl(field).__lens; | ||
| const initialValue = fieldLens.get(state.initialValues); | ||
| const fieldAtom = Atom.entangle(state.values, fieldLens); | ||
| return Atom.fuse( | ||
| fieldValue => !deepEqual(fieldValue, initialValue), | ||
| fieldAtom | ||
| ); | ||
| }) | ||
| ), |
There was a problem hiding this comment.
wouldn't deepEqual on state.values and state.initialValues be enough?
| Atom.fuse( | |
| (...fieldsChanged) => fieldsChanged.some(Boolean), | |
| ...values(Schema).map(field => { | |
| const fieldLens = impl(field).__lens; | |
| const initialValue = fieldLens.get(state.initialValues); | |
| const fieldAtom = Atom.entangle(state.values, fieldLens); | |
| return Atom.fuse( | |
| fieldValue => !deepEqual(fieldValue, initialValue), | |
| fieldAtom | |
| ); | |
| }) | |
| ), | |
| Atom.fuse(values => !deepEqual(values, state.initialValues), state.values), |
There was a problem hiding this comment.
yes, this is a bit of an optimisation - we don't throw the entire form values into deepEqual every time any field changes, but only compare the field that did change. If the entire from is nested in object field then it does not matter, but I thought it could be worth it in some cases.
closes #117
published as
0.2.21-beta.1demo: https://codesandbox.io/s/pizza-form-v2-czt6cw?file=/src/order-pizza-form.tsx (see updated pizza form example!)