From be529d2ff3e68cd8456619f7684b8ae55e5a6780 Mon Sep 17 00:00:00 2001 From: Mallory Allen Date: Fri, 4 Jan 2019 17:05:04 -0500 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20better=20behaviour=20for=20=20initialValue=20changes=20(#446)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ✨ customizable state derivation for form state * 🗒️ fix FAQ --- packages/react-form-state/CHANGELOG.md | 10 +- packages/react-form-state/docs/FAQ.md | 49 ++- packages/react-form-state/src/FormState.ts | 82 +++-- .../src/tests/FormState.test.tsx | 319 ++++++++++++++---- 4 files changed, 379 insertions(+), 81 deletions(-) diff --git a/packages/react-form-state/CHANGELOG.md b/packages/react-form-state/CHANGELOG.md index 0520f1d457..ca74512976 100644 --- a/packages/react-form-state/CHANGELOG.md +++ b/packages/react-form-state/CHANGELOG.md @@ -4,15 +4,21 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). +## Unreleased + +### Added + +- You can control how `` reacts to changes in the initialValue prop using onInitialValueChanged. + ## [0.5] -## Added +### Added - `` supports `getChildKey` to provide custom `key`s for it's children. [#387](https://github.com/Shopify/quilt/pull/387) ## [0.4.1] -## Fixed +### Fixed - `` no longer breaks on name generation. diff --git a/packages/react-form-state/docs/FAQ.md b/packages/react-form-state/docs/FAQ.md index 820863b35b..2a1fdf4c35 100644 --- a/packages/react-form-state/docs/FAQ.md +++ b/packages/react-form-state/docs/FAQ.md @@ -17,7 +17,7 @@ As such the main difference in our solution is the explicit, declarative api. Fo ## I want to invoke all my validators whenever I want, how can I do this? -You can do this by setting a `ref` on your ``, and calling `validateForm` on the instance passed in. +You can do this by setting a [`ref`](https://reactjs.org/docs/refs-and-the-dom.html#creating-refs) on your ``, and calling `validateForm` on the instance passed in. ```typescript // use `createRef` and validate imperatively later @@ -38,6 +38,53 @@ class MyComponent extends React.Component { If you need to do something immediately on mount you could also use old fashioned [callback refs](https://reactjs.org/docs/refs-and-the-dom.html#callback-refs). +## My form keeps resetting for no reason! / My form is resetting whenever I change an input! + +By default `` resets whenever any value in your `initialValues` changes. If you are basing your initial values on existing state, this lets it update when your state changes (usually this would be the result of submitting). + +If this is happening on each rerender, it is likely that you are generating your `initialValues` in some way that is different each time. This can happen when you construct `Date` objects, `UUID`s, or other dynamic values inline. You can solve this by `memoize`ing your initial value creation or creating dates and other dynamic data only once outside of your component's `render` method. + +```typescript +// Bad! +function MyForm() { + return ( + + {({fields}) => /* markup*/ } + + ); +} + + +// Good! +const today = new Date(); + +function MyForm() { + return ( + + {({fields}) => /* markup*/ } + + ); +} +``` + +## Can I have more control over what happens when initialValues change? + +You can control how `` reacts to changes in the `initialValue` prop using `onInitialValueChanged`. This prop has three options: + +- (default) `reset-all`: Reset the entire form when `initialValues` changes. +- `reset-where-changed`: Reset only the changed field objects when `initialValues` changes. +- `ignore`: Ignore changes to the `initialValues` prop. This option makes `` behave like a [fully controlled component](https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html#recommendation-fully-controlled-component). You will generally want to accompany this option with a [`key`](https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html#recommendation-fully-uncontrolled-component-with-a-key) or [`ref`](https://reactjs.org/docs/refs-and-the-dom.html#creating-refs). + ## More questions Have a question that you think should be included in our FAQ? Please help us by creating an [issue](https://github.com/Shopify/quilt/issues/new?template=ENHANCEMENT.md) or opening a pull request. diff --git a/packages/react-form-state/src/FormState.ts b/packages/react-form-state/src/FormState.ts index 55c164abd9..405c4c0c68 100644 --- a/packages/react-form-state/src/FormState.ts +++ b/packages/react-form-state/src/FormState.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-case-declarations */ import * as React from 'react'; import isEqual from 'lodash/isEqual'; import isArray from 'lodash/isArray'; @@ -52,6 +53,7 @@ interface Props { validators?: Partial>; onSubmit?: SubmitHandler; validateOnSubmit?: boolean; + onInitialValuesChange?: 'reset-all' | 'reset-where-changed' | 'ignore'; children(form: FormDetails): React.ReactNode; } @@ -68,21 +70,25 @@ export default class FormState< static List = List; static Nested = Nested; - static getDerivedStateFromProps(newProps: Props, oldState?: State) { - const newInitialValues = newProps.initialValues; + static getDerivedStateFromProps(newProps: Props, oldState: State) { + const {initialValues, onInitialValuesChange} = newProps; - if (oldState == null) { - return createFormState(newInitialValues); - } + switch (onInitialValuesChange) { + case 'ignore': + return null; + case 'reset-where-changed': + return reconcileFormState(initialValues, oldState); + case 'reset-all': + default: + const oldInitialValues = initialValuesFromFields(oldState.fields); + const valuesMatch = isEqual(oldInitialValues, initialValues); - const oldInitialValues = initialValuesFromFields(oldState.fields); - const shouldReinitialize = !isEqual(oldInitialValues, newInitialValues); + if (valuesMatch) { + return null; + } - if (shouldReinitialize) { - return createFormState(newInitialValues); + return createFormState(initialValues); } - - return null; } state = createFormState(this.props.initialValues); @@ -127,6 +133,16 @@ export default class FormState< }); } + @bind() + public reset() { + return new Promise(resolve => { + this.setState( + (_state, props) => createFormState(props.initialValues), + () => resolve(), + ); + }); + } + private get dirty() { return this.state.dirtyFields.length > 0; } @@ -184,22 +200,18 @@ export default class FormState< } } - const errors = await onSubmit(formData); + const errors = (await onSubmit(formData)) || []; if (!this.mounted) { return; } - if (errors) { + if (errors.length > 0) { this.updateRemoteErrors(errors); + this.setState({submitting: false}); + } else { + this.setState({submitting: false, errors}); } - - this.setState({submitting: false}); - } - - @bind() - private reset() { - this.setState((_state, props) => createFormState(props.initialValues)); } @memoize() @@ -373,6 +385,36 @@ export default class FormState< } } +function reconcileFormState( + values: Fields, + oldState: State, +): State { + const {fields: oldFields} = oldState; + const dirtyFields = new Set(oldState.dirtyFields); + + const fields: FieldStates = mapObject(values, (value, key) => { + const oldField = oldFields[key]; + + if (value === oldField.initialValue) { + return oldField; + } + + dirtyFields.delete(key); + + return { + value, + initialValue: value, + dirty: false, + }; + }); + + return { + ...oldState, + dirtyFields: Array.from(dirtyFields), + fields, + }; +} + function createFormState(values: Fields): State { const fields: FieldStates = mapObject(values, value => { return { diff --git a/packages/react-form-state/src/tests/FormState.test.tsx b/packages/react-form-state/src/tests/FormState.test.tsx index a9e6b86d28..8929b4b1eb 100644 --- a/packages/react-form-state/src/tests/FormState.test.tsx +++ b/packages/react-form-state/src/tests/FormState.test.tsx @@ -41,76 +41,240 @@ describe('', () => { }, }); }); + }); - it('resets field objects values to new initialValues when prop is updated', () => { - const renderPropSpy = jest.fn(() => null); - const product = faker.commerce.productName(); - - const form = mount( - {renderPropSpy}, - ); - - const formDetails = lastCallArgs(renderPropSpy); - formDetails.fields.product.onChange(faker.commerce.productName()); - - const otherProduct = faker.commerce.productName(); - form.setProps({initialValues: {product: otherProduct}}); + describe('onInitialValuesChange', () => { + describe('default', () => { + it('resets all field objects values to their initialValues when the prop is updated', () => { + const renderPropSpy = jest.fn(() => null); + const originalValues = { + product: faker.commerce.productName(), + price: faker.commerce.price(), + }; + + const form = mount( + {renderPropSpy}, + ); + + const formDetails = lastCallArgs(renderPropSpy); + formDetails.fields.product.onChange(faker.commerce.productName()); + formDetails.fields.price.onChange(faker.commerce.price()); + + const otherProduct = faker.commerce.productName(); + form.setProps({ + initialValues: {price: originalValues.price, product: otherProduct}, + }); + + const {fields} = lastCallArgs(renderPropSpy); + expect(fields).toMatchObject({ + product: { + value: otherProduct, + initialValue: otherProduct, + dirty: false, + }, + price: { + value: originalValues.price, + initialValue: originalValues.price, + dirty: false, + }, + }); + }); - const {fields} = lastCallArgs(renderPropSpy); - expect(fields).toMatchObject({ - product: { - value: otherProduct, - initialValue: otherProduct, + it('resets errors and dirty when the initialValues prop is updated', async () => { + const renderPropSpy = jest.fn(() => null); + const originalValues = { + product: faker.commerce.productName(), + price: faker.commerce.price(), + }; + + const form = mount( + Promise.resolve([{message: 'bad'}])} + initialValues={originalValues} + > + {renderPropSpy} + , + ); + + const formDetails = lastCallArgs(renderPropSpy); + formDetails.fields.price.onChange(faker.commerce.price()); + formDetails.fields.price.onChange(faker.commerce.price()); + + const {submit} = lastCallArgs(renderPropSpy); + await submit(); + + const otherProduct = faker.commerce.productName(); + form.setProps({ + initialValues: {price: originalValues.price, product: otherProduct}, + }); + + const state = lastCallArgs(renderPropSpy); + expect(state).toMatchObject({ + errors: [], dirty: false, - }, + }); }); - }); - it('does not reset field objects values when non-initialValues props are updated', () => { - const renderPropSpy = jest.fn(() => null); - const product = faker.commerce.productName(); + it('does not reset field objects values when non-initialValues props are updated', () => { + const renderPropSpy = jest.fn(() => null); + const product = faker.commerce.productName(); - const form = mount( - {renderPropSpy}, - ); + const form = mount( + {renderPropSpy}, + ); - const formDetails = lastCallArgs(renderPropSpy); - const otherProduct = faker.commerce.productName(); - formDetails.fields.product.onChange(otherProduct); + const formDetails = lastCallArgs(renderPropSpy); + const otherProduct = faker.commerce.productName(); + formDetails.fields.product.onChange(otherProduct); - form.setProps({validators: {}}); + form.setProps({validators: {}}); - const {fields} = lastCallArgs(renderPropSpy); - expect(fields).toMatchObject({ - product: { - value: otherProduct, - initialValue: product, - dirty: true, - }, + const {fields} = lastCallArgs(renderPropSpy); + expect(fields).toMatchObject({ + product: { + value: otherProduct, + initialValue: product, + dirty: true, + }, + }); }); }); - }); - describe('reset()', () => { - it('resets all fields to their initial values', () => { - const renderPropSpy = jest.fn(() => null); - const product = faker.commerce.productName(); + describe('reset-where-changed', () => { + it('resets only field objects with changed initialValues when the prop is updated', () => { + const renderPropSpy = jest.fn(() => null); + const originalValues = { + product: faker.commerce.productName(), + price: faker.commerce.price(), + }; + + const form = mount( + + {renderPropSpy} + , + ); + + const formDetails = lastCallArgs(renderPropSpy); + formDetails.fields.product.onChange(faker.commerce.productName()); + const changedPrice = faker.commerce.price(); + formDetails.fields.price.onChange(changedPrice); + + const otherProduct = faker.commerce.productName(); + form.setProps({ + initialValues: {price: originalValues.price, product: otherProduct}, + }); + + const {fields} = lastCallArgs(renderPropSpy); + expect(fields).toMatchObject({ + product: { + value: otherProduct, + initialValue: otherProduct, + dirty: false, + }, + price: { + value: changedPrice, + initialValue: originalValues.price, + dirty: true, + }, + }); + }); - mount({renderPropSpy}); + it('does not reset errors when the initialValues prop is updated', async () => { + const renderPropSpy = jest.fn(() => null); + const originalValues = { + product: faker.commerce.productName(), + price: faker.commerce.price(), + }; + + const submitErrors = [ + {message: faker.lorem.sentences()}, + {message: faker.lorem.sentences()}, + ]; + + function onSubmit() { + return Promise.resolve(submitErrors); + } + + const form = mount( + + {renderPropSpy} + , + ); + + const formDetails = lastCallArgs(renderPropSpy); + formDetails.fields.price.onChange(faker.commerce.price()); + formDetails.fields.price.onChange(faker.commerce.price()); + + const {submit} = lastCallArgs(renderPropSpy); + await submit(); + + const otherProduct = faker.commerce.productName(); + + form.setProps({ + initialValues: {price: originalValues.price, product: otherProduct}, + }); + + const state = lastCallArgs(renderPropSpy); + expect(state.errors).toBe(submitErrors); + }); + }); - const formDetails = lastCallArgs(renderPropSpy); - formDetails.fields.product.onChange(faker.commerce.productName()); + describe('ignore', () => { + it('does not reset field objects when the initialValues prop is updated', () => { + const renderPropSpy = jest.fn(() => null); + const product = faker.commerce.productName(); + + const form = mount( + + {renderPropSpy} + , + ); + + const formDetails = lastCallArgs(renderPropSpy); + const changedProduct = faker.commerce.productName(); + formDetails.fields.product.onChange(changedProduct); + + form.setProps({initialValues: {product: faker.commerce.productName()}}); + + const {fields} = lastCallArgs(renderPropSpy); + expect(fields).toMatchObject({ + product: { + value: changedProduct, + initialValue: product, + dirty: true, + }, + }); + }); - const {reset} = lastCallArgs(renderPropSpy); - reset(); + it('does not reset field objects values when non-initialValues props are updated', () => { + const renderPropSpy = jest.fn(() => null); + const product = faker.commerce.productName(); - const {fields} = lastCallArgs(renderPropSpy); - expect(fields).toMatchObject({ - product: { - value: product, - initialValue: product, - dirty: false, - }, + const form = mount( + {renderPropSpy}, + ); + + const formDetails = lastCallArgs(renderPropSpy); + const otherProduct = faker.commerce.productName(); + formDetails.fields.product.onChange(otherProduct); + + form.setProps({validators: {}}); + + const {fields} = lastCallArgs(renderPropSpy); + expect(fields).toMatchObject({ + product: { + value: otherProduct, + initialValue: product, + dirty: true, + }, + }); }); }); }); @@ -530,7 +694,8 @@ describe('', () => { expect.objectContaining(formData), ); }); - it('when onSubmit() returns a promise, re-renders with submitting true while waiting for it to resolve/reject', () => { + + it('re-renders with submitting true while waiting for the onSubmit promise to resolve/reject', () => { const renderPropSpy = jest.fn(() => null); const product = faker.commerce.productName(); @@ -551,7 +716,7 @@ describe('', () => { expect(submitting).toBe(true); }); - it('when onSubmit() returns a promise, re-renders with submitting false when promise resolves', async () => { + it('re-renders with submitting false when onSubmit promise resolves', async () => { const renderPropSpy = jest.fn(() => null); const product = faker.commerce.productName(); @@ -572,7 +737,7 @@ describe('', () => { expect(submitting).toBe(false); }); - it('updates errors when errors are returned from onSubmit', async () => { + it('updates errors when errors are returned from onSubmit promise', async () => { const renderPropSpy = jest.fn(() => null); const product = faker.commerce.productName(); @@ -598,7 +763,7 @@ describe('', () => { expect(errors).toEqual(errors); }); - it('propagates submit errors down to fields when the field array matches', async () => { + it('propagates submit errors down to fields when the field array matches an existing field', async () => { const renderPropSpy = jest.fn(() => null); const message = faker.lorem.sentences(); @@ -629,6 +794,44 @@ describe('', () => { expect(fields.product.error).toBe(message); }); + it('clears submit errors when a subsequent submit is successful', async () => { + const renderPropSpy = jest.fn(() => null); + + const submitErrors = [ + { + message: faker.lorem.sentences(), + }, + ]; + + let shouldSucceed = false; + function onSubmit() { + if (shouldSucceed) { + return []; + } + + return Promise.resolve(submitErrors); + } + + mount( + + {renderPropSpy} + , + ); + + const failingState = lastCallArgs(renderPropSpy); + await failingState.submit(); + + shouldSucceed = true; + const succeedingState = lastCallArgs(renderPropSpy); + await succeedingState.submit(); + + const {errors} = lastCallArgs(renderPropSpy); + expect(errors).toHaveLength(0); + }); + it('clears submit errors on fields when onChange is called', async () => { const renderPropSpy = jest.fn(() => null); const message = faker.lorem.sentences();