diff --git a/packages/react-form/src/hooks/form.ts b/packages/react-form/src/hooks/form.ts index 417870303e..9765c41bf1 100755 --- a/packages/react-form/src/hooks/form.ts +++ b/packages/react-form/src/hooks/form.ts @@ -2,10 +2,12 @@ import {useCallback, useMemo} from 'react'; import {useLazyRef} from '@shopify/react-hooks'; import { - SubmitHandler, - FormMapping, FieldBag, + FormInput, + FormWithDynamicListsInput, + FormWithoutDynamicListsInput, Form, + FormWithDynamicLists, DynamicListBag, } from '../types'; import {validateAll, makeCleanFields} from '../utilities'; @@ -13,7 +15,7 @@ import {validateAll, makeCleanFields} from '../utilities'; import {useDirty} from './dirty'; import {useReset} from './reset'; import {useSubmit} from './submit'; -import {useDynamicList, useDynamicListDirty, useDynamicListReset} from './list'; +import {useDynamicListDirty, useDynamicListReset} from './list'; /** * A custom hook for managing the state of an entire form. `useForm` wraps up many of the other hooks in this package in one API, and when combined with `useField` and `useList`, allows you to easily build complex forms with smart defaults for common cases. @@ -80,14 +82,22 @@ import {useDynamicList, useDynamicListDirty, useDynamicListReset} from './list'; export function useForm({ fields, onSubmit, + makeCleanAfterSubmit, +}: FormWithoutDynamicListsInput): Form; + +export function useForm({ + fields, + dynamicLists, + onSubmit, + makeCleanAfterSubmit, +}: FormWithDynamicListsInput): FormWithDynamicLists; + +export function useForm({ + fields, dynamicLists, + onSubmit, makeCleanAfterSubmit = false, -}: { - fields: T; - onSubmit?: SubmitHandler>; - makeCleanAfterSubmit?: boolean; - dynamicLists?: DynamicListBag; -}): Form { +}: FormInput) { const fieldsWithLists = useMemo(() => { if (dynamicLists) { const fieldsWithList = {...fields}; @@ -128,13 +138,7 @@ export function useForm({ fieldsRef, ]); - const defaultDynamicListBag = { - defaultDynamicList: useDynamicList<{}>([], () => { - return {}; - }), - }; - - return { + const form: Form = { fields, dirty: dirty || dynamicListDirty, submitting, @@ -143,6 +147,11 @@ export function useForm({ validate, makeClean, submitErrors: errors, - dynamicLists: dynamicLists || defaultDynamicListBag, }; + + if (dynamicLists) { + return {...form, dynamicLists}; + } + + return form; } diff --git a/packages/react-form/src/hooks/list/test/dynamiclist.test.tsx b/packages/react-form/src/hooks/list/test/dynamiclist.test.tsx index 147eeae2d6..69fe421ffb 100644 --- a/packages/react-form/src/hooks/list/test/dynamiclist.test.tsx +++ b/packages/react-form/src/hooks/list/test/dynamiclist.test.tsx @@ -13,9 +13,14 @@ describe('useDynamicList', () => { return {price: '', optionName: '', optionValue: ''}; }; function DynamicListComponent(config: FieldListConfig) { - const {fields, addItem, removeItem, moveItem, reset} = useDynamicList< - Variant - >(config, factory); + const { + fields, + addItem, + removeItem, + moveItem, + reset, + dirty, + } = useDynamicList(config, factory); return ( <> @@ -48,6 +53,7 @@ describe('useDynamicList', () => { +

Dirty: {dirty.toString()}

); } @@ -73,6 +79,9 @@ describe('useDynamicList', () => { }); it('will throw an error if new position is out of index range', () => { + const consoleErrorMock = jest.spyOn(console, 'error'); + consoleErrorMock.mockImplementation(); + const variants: Variant[] = [ {price: 'A', optionName: 'A', optionValue: 'A'}, {price: 'B', optionName: 'B', optionValue: 'B'}, @@ -86,6 +95,8 @@ describe('useDynamicList', () => { .findAll('button', {children: 'Move Variant up'})![0] .trigger('onClick'); }).toThrow('Failed to move item from 0 to -1'); + + consoleErrorMock.mockRestore(); }); it('can remove field', () => { @@ -108,8 +119,6 @@ describe('useDynamicList', () => { .find('button', {children: 'Add Variant'})! .trigger('onClick', clickEvent()); - // const addedTextField = wrapper.findAll(TextField)![0]; - expect(wrapper).toContainReactComponent(TextField, { name: 'price1', value: '', @@ -203,6 +212,66 @@ describe('useDynamicList', () => { expect(wrapper).toContainReactComponent(TextField); }); }); + + describe('dirty dynamic list', () => { + it('handles dirty state when adding a field and resetting it', () => { + const wrapper = mount( + , + ); + + expect(wrapper).toContainReactText('Dirty: false'); + + wrapper + .find(TextField, {name: 'price0'})! + .trigger('onChange', 'new value'); + + expect(wrapper).toContainReactText('Dirty: true'); + + wrapper + .find('button', {children: 'Reset'})! + .trigger('onClick', clickEvent()); + + expect(wrapper).toContainReactText('Dirty: false'); + }); + + it('handles dirty state when adding a field and resetting it', () => { + const wrapper = mount(); + + expect(wrapper).toContainReactText('Dirty: false'); + + wrapper + .find('button', {children: 'Add Variant'})! + .trigger('onClick', clickEvent()); + + expect(wrapper).toContainReactText('Dirty: true'); + + wrapper + .find('button', {children: 'Reset'})! + .trigger('onClick', clickEvent()); + + expect(wrapper).toContainReactText('Dirty: false'); + }); + + it('handles dirty state when removing a field and resetting it', () => { + const wrapper = mount( + , + ); + + expect(wrapper).toContainReactText('Dirty: false'); + + wrapper + .find('button', {children: 'Remove Variant'})! + .trigger('onClick', clickEvent()); + + expect(wrapper).toContainReactText('Dirty: true'); + + wrapper + .find('button', {children: 'Reset'})! + .trigger('onClick', clickEvent()); + + expect(wrapper).toContainReactText('Dirty: false'); + }); + }); }); describe('add mulitiple items with payload', () => { diff --git a/packages/react-form/src/hooks/test/form-with-dynamic-list.test.tsx b/packages/react-form/src/hooks/test/form-with-dynamic-list.test.tsx new file mode 100644 index 0000000000..7195e9b821 --- /dev/null +++ b/packages/react-form/src/hooks/test/form-with-dynamic-list.test.tsx @@ -0,0 +1,145 @@ +import React from 'react'; +import {mount} from '@shopify/react-testing'; + +import { + FormWithDynamicVariantList, + TextField, + isDirty, + fakeProduct, + hitSubmit, + hitReset, + waitForSubmit, +} from './utilities'; + +import {submitSuccess, submitFail} from '..'; + +describe('useForm with dynamic list', () => { + describe('dirty state', () => { + it('dirty state is false when no field has been changed', () => { + const wrapper = mount( + , + ); + + expect(isDirty(wrapper)).toBe(false); + }); + + it('dirty state is true when a new variant item has been added', () => { + const wrapper = mount( + , + ); + + wrapper.find('button', {children: 'Add item'}).trigger('onClick'); + + expect(isDirty(wrapper)).toBe(true); + }); + + it('dirty state is true when a variant item has been removed', () => { + const wrapper = mount( + , + ); + + wrapper.find('button', {children: 'Remove item'}).trigger('onClick'); + + expect(isDirty(wrapper)).toBe(true); + }); + + it('dirty state is true when a variant item has been edited', () => { + const wrapper = mount( + , + ); + + wrapper + .find(TextField, {label: 'price'}) + .trigger('onChange', 'next price'); + + expect(isDirty(wrapper)).toBe(true); + }); + }); + + describe('submit', () => { + it('validates dynamic list fields with their latest values before submitting and bails out if any fail', () => { + const submitSpy = jest.fn(() => Promise.resolve(submitSuccess())); + const product = fakeProduct(); + const wrapper = mount( + , + ); + + const textFields = wrapper.findAll(TextField, {label: 'option'}); + + textFields.forEach(textField => textField.trigger('onChange', '')); + hitSubmit(wrapper); + + expect(submitSpy).not.toHaveBeenCalled(); + + expect(wrapper).toContainReactComponentTimes( + TextField, + textFields.length, + { + error: 'Option name is required!', + }, + ); + }); + + it('propagates remote submission errors to matching fields', async () => { + const errors = [ + { + field: ['variants', '0', 'price'], + message: 'The server hates your price', + }, + ]; + const promise = Promise.resolve(submitFail(errors)); + const wrapper = mount( + promise} + />, + ); + + await waitForSubmit(wrapper, promise); + + expect(wrapper).toContainReactComponent(TextField, { + error: errors[0].message, + }); + }); + }); + + describe('reset', () => { + it('reset dynamic list after adding new item', () => { + const wrapper = mount( + , + ); + + wrapper.find('button', {children: 'Add item'}).trigger('onClick'); + + expect(wrapper).toContainReactComponentTimes(TextField, 3, { + label: 'option', + }); + + hitReset(wrapper); + + expect(wrapper).toContainReactComponentTimes(TextField, 2, { + label: 'option', + }); + expect(isDirty(wrapper)).toBe(false); + }); + + it('reset dynamic list after removing item', () => { + const wrapper = mount( + , + ); + + wrapper.find('button', {children: 'Remove item'}).trigger('onClick'); + + expect(wrapper).toContainReactComponentTimes(TextField, 1, { + label: 'option', + }); + + hitReset(wrapper); + + expect(wrapper).toContainReactComponentTimes(TextField, 2, { + label: 'option', + }); + expect(isDirty(wrapper)).toBe(false); + }); + }); +}); diff --git a/packages/react-form/src/hooks/test/form.test.tsx b/packages/react-form/src/hooks/test/form.test.tsx index 9458d19ee0..a947d7353e 100644 --- a/packages/react-form/src/hooks/test/form.test.tsx +++ b/packages/react-form/src/hooks/test/form.test.tsx @@ -1,200 +1,21 @@ import React from 'react'; -import faker from 'faker'; import {mount} from '@shopify/react-testing'; -import {SubmitHandler} from '../../types'; -import {positiveNumericString, notEmpty} from '../../validation'; - import { - useList, - useField, - useForm, - submitSuccess, - submitFail, - useDynamicList, -} from '..'; - -interface SimpleProduct { - title: string; - description: string; - defaultVariant: { - optionName: string; - optionValue: string; - price: string; - }; - variants: { - id: string; - optionName: string; - optionValue: string; - price: string; - }[]; -} + ProductForm, + TextField, + isDirty, + changeTitle, + fakeProduct, + hitSubmit, + hitReset, + hitClean, + waitForSubmit, +} from './utilities'; + +import {submitSuccess, submitFail} from '..'; describe('useForm', () => { - function ProductForm({ - data, - onSubmit, - makeCleanAfterSubmit, - dynamicListEnabled = false, - }: { - data: SimpleProduct; - onSubmit?: SubmitHandler; - makeCleanAfterSubmit?: boolean; - dynamicListEnabled: boolean; - }) { - const variantsEmptyFactory = () => { - return {id: '', price: '', optionName: '', optionValue: ''}; - }; - - const title = useField({ - value: data.title, - validates: notEmpty('Title is required!'), - }); - const description = useField(data.description); - - const defaultVariant = { - price: useField({ - value: data.defaultVariant.price, - validates: positiveNumericString('price must be a number'), - }), - optionName: useField(data.defaultVariant.optionName), - optionValue: useField(data.defaultVariant.optionValue), - }; - - const variants = useList({ - list: data.variants, - validates: { - price: positiveNumericString('price must be a number'), - }, - }); - - const dynamicVariants = useDynamicList(data.variants, variantsEmptyFactory); - - const dynamicListProp = dynamicListEnabled - ? { - dynamicLists: { - defaultDynamicList: dynamicVariants, - }, - } - : undefined; - - const { - submit, - submitting, - dirty, - reset, - makeClean, - submitErrors, - dynamicLists, - } = useForm({ - fields: {title, description, defaultVariant, variants}, - ...dynamicListProp, - onSubmit: onSubmit as any, - makeCleanAfterSubmit, - }); - - const { - defaultDynamicList: {addItem, fields: dynamicListFields, removeItem}, - } = dynamicLists; - - return ( -
- {submitting &&

loading...

} - {submitErrors.length > 0 && - submitErrors.map(({message}) =>

{message}

)} - -
- - -
-
- - - -
- {variants.map(({price, optionName, optionValue, id}) => { - return ( -
- - - -
- ); - })} - {dynamicListFields.map((field, index) => ( - <> - - - - ))} - - - - - - ); - } - - function isDirty(wrapper) { - try { - expect(wrapper).toContainReactComponent('button', { - type: 'reset', - disabled: false, - }); - expect(wrapper).toContainReactComponent('button', { - type: 'submit', - disabled: false, - }); - } catch { - return false; - } - return true; - } - - function changeTitle(wrapper, newTitle) { - wrapper.find(TextField, {label: 'title'})!.trigger('onChange', newTitle); - } - - function hitSubmit(wrapper) { - wrapper.find('button', {type: 'submit'})!.trigger('onClick', clickEvent()); - } - - async function waitForSubmit(wrapper, successPromise) { - hitSubmit(wrapper); - - await wrapper.act(async () => { - await successPromise; - }); - } - - function hitReset(wrapper) { - wrapper.find('button', {type: 'reset'})!.trigger('onClick', clickEvent()); - } - - function hitClean(wrapper) { - wrapper.find('button', {type: 'button'})!.trigger('onClick', clickEvent()); - } - describe('dirty state', () => { it('dirty state is false when no field has been changed', () => { const wrapper = mount(); @@ -500,115 +321,4 @@ describe('useForm', () => { expect(initialMakeCleanHandler).toBe(newMakeCleanHandler); }); }); - - describe('dynamicLists', () => { - it('reflects dynamic list being dirty in forms dirty state', () => { - const wrapper = mount( - , - ); - - wrapper - .find('button', { - children: 'Add Item', - })! - .trigger('onClick', clickEvent()); - - expect(isDirty(wrapper)).toBe(true); - - hitReset(wrapper); - - expect(isDirty(wrapper)).toBe(false); - }); - - it('resets dynamic list fields when forms reset is called', () => { - const wrapper = mount( - , - ); - - wrapper - .find('button', { - children: 'Add Item', - })! - .trigger('onClick', clickEvent()); - - expect(wrapper).toContainReactComponent(TextField, { - value: '', - label: 'dynamicField2', - }); - - hitReset(wrapper); - - expect(wrapper).not.toContainReactComponent(TextField, { - value: '', - label: 'dynamicField2', - }); - }); - - it('renders empty dynamic list when dynamic list is not enabled', () => { - const wrapper = mount(); - - const dynamicListFieldsRemoveButtons = wrapper.findAll('button', { - children: 'Remove dynamic field', - }); - - expect(dynamicListFieldsRemoveButtons).toHaveLength(0); - }); - }); }); - -interface TextFieldProps { - value: string; - label: string; - name?: string; - error?: string; - onChange(value): void; - onBlur(): void; -} - -function TextField({ - label, - name = label, - onChange, - onBlur, - value, - error, -}: TextFieldProps) { - return ( - <> - - {error &&

{error}

} - - ); -} - -function fakeProduct(): SimpleProduct { - return { - title: faker.commerce.product(), - description: faker.lorem.paragraph(), - defaultVariant: { - price: faker.commerce.price(), - optionName: 'material', - optionValue: faker.commerce.productMaterial(), - }, - variants: Array.from({length: 2}).map(() => ({ - id: faker.random.uuid(), - price: faker.commerce.price(), - optionName: faker.lorem.word(), - optionValue: faker.commerce.productMaterial(), - })), - }; -} - -function clickEvent() { - // we don't actually use these at all so it is ok to just return an empty object - return {} as any; -} diff --git a/packages/react-form/src/hooks/test/utilities/components/FormWithDynamicVariantList.tsx b/packages/react-form/src/hooks/test/utilities/components/FormWithDynamicVariantList.tsx new file mode 100644 index 0000000000..7065b8f66f --- /dev/null +++ b/packages/react-form/src/hooks/test/utilities/components/FormWithDynamicVariantList.tsx @@ -0,0 +1,95 @@ +import React from 'react'; + +import {SubmitHandler} from '../../../../types'; +import {notEmpty} from '../../../../validation'; +import {useDynamicList, useField, useForm} from '../../..'; + +import {SimpleProduct, Variant} from './types'; +import {TextField} from './TextField'; + +export function FormWithDynamicVariantList({ + data, + onSubmit, + makeCleanAfterSubmit, +}: { + data: SimpleProduct; + onSubmit?: SubmitHandler; + makeCleanAfterSubmit?: boolean; +}) { + const variantFactory = () => { + return { + id: Date.now().toString(), + price: '', + optionName: '', + optionValue: '', + }; + }; + + const { + fields: {title}, + dynamicLists: {variants}, + submit, + submitting, + dirty, + reset, + makeClean, + submitErrors, + } = useForm({ + fields: { + title: useField({ + value: data.title, + validates: notEmpty('Title is required!'), + }), + }, + dynamicLists: { + variants: useDynamicList( + { + list: data.variants, + validates: { + optionName: notEmpty('Option name is required!'), + }, + }, + variantFactory, + ), + }, + onSubmit: onSubmit as any, + makeCleanAfterSubmit, + }); + + return ( +
+ {submitting &&

loading...

} + {submitErrors.length > 0 && + // eslint-disable-next-line react/no-array-index-key + submitErrors.map(({message}, index) =>

{message}

)} + +
+ +
+ {variants.fields.map(({price, optionName, optionValue, id}, index) => { + return ( +
+ + + + +
+ ); + })} + + + + +
+ ); +} diff --git a/packages/react-form/src/hooks/test/utilities/components/ProductForm.tsx b/packages/react-form/src/hooks/test/utilities/components/ProductForm.tsx new file mode 100644 index 0000000000..ec224e1321 --- /dev/null +++ b/packages/react-form/src/hooks/test/utilities/components/ProductForm.tsx @@ -0,0 +1,82 @@ +import React from 'react'; + +import {SubmitHandler} from '../../../../types'; +import {positiveNumericString, notEmpty} from '../../../../validation'; +import {useList, useField, useForm} from '../../..'; + +import {SimpleProduct} from './types'; +import {TextField} from './TextField'; + +export function ProductForm({ + data, + onSubmit, + makeCleanAfterSubmit, +}: { + data: SimpleProduct; + onSubmit?: SubmitHandler; + makeCleanAfterSubmit?: boolean; +}) { + const title = useField({ + value: data.title, + validates: notEmpty('Title is required!'), + }); + const description = useField(data.description); + + const defaultVariant = { + price: useField({ + value: data.defaultVariant.price, + validates: positiveNumericString('price must be a number'), + }), + optionName: useField(data.defaultVariant.optionName), + optionValue: useField(data.defaultVariant.optionValue), + }; + + const variants = useList({ + list: data.variants, + validates: { + price: positiveNumericString('price must be a number'), + }, + }); + + const {submit, submitting, dirty, reset, makeClean, submitErrors} = useForm({ + fields: {title, description, defaultVariant, variants}, + onSubmit: onSubmit as any, + makeCleanAfterSubmit, + }); + + return ( +
+ {submitting &&

loading...

} + {submitErrors.length > 0 && + submitErrors.map(({message}) =>

{message}

)} + +
+ + +
+
+ + + +
+ {variants.map(({price, optionName, optionValue, id}) => { + return ( +
+ + + +
+ ); + })} + + + +
+ ); +} diff --git a/packages/react-form/src/hooks/test/utilities/components/TextField.tsx b/packages/react-form/src/hooks/test/utilities/components/TextField.tsx new file mode 100644 index 0000000000..7e9b26605f --- /dev/null +++ b/packages/react-form/src/hooks/test/utilities/components/TextField.tsx @@ -0,0 +1,35 @@ +import React from 'react'; + +interface TextFieldProps { + value: string; + label: string; + name?: string; + error?: string; + onChange(value): void; + onBlur(): void; +} + +export function TextField({ + label, + name = label, + onChange, + onBlur, + value, + error, +}: TextFieldProps) { + return ( + <> + + {error &&

{error}

} + + ); +} diff --git a/packages/react-form/src/hooks/test/utilities/components/index.ts b/packages/react-form/src/hooks/test/utilities/components/index.ts new file mode 100644 index 0000000000..b2f805fe7c --- /dev/null +++ b/packages/react-form/src/hooks/test/utilities/components/index.ts @@ -0,0 +1,4 @@ +export {FormWithDynamicVariantList} from './FormWithDynamicVariantList'; +export {ProductForm} from './ProductForm'; +export {TextField} from './TextField'; +export * from './types'; diff --git a/packages/react-form/src/hooks/test/utilities/components/types.ts b/packages/react-form/src/hooks/test/utilities/components/types.ts new file mode 100644 index 0000000000..52a3a0b0fe --- /dev/null +++ b/packages/react-form/src/hooks/test/utilities/components/types.ts @@ -0,0 +1,13 @@ +export interface Variant { + id: string; + optionName: string; + optionValue: string; + price: string; +} + +export interface SimpleProduct { + title: string; + description: string; + defaultVariant: Omit; + variants: Variant[]; +} diff --git a/packages/react-form/src/hooks/test/utilities/index.ts b/packages/react-form/src/hooks/test/utilities/index.ts new file mode 100644 index 0000000000..10b3b3c6e4 --- /dev/null +++ b/packages/react-form/src/hooks/test/utilities/index.ts @@ -0,0 +1,68 @@ +import faker from 'faker'; + +import {SimpleProduct, TextField} from './components'; + +export * from './components'; + +export function isDirty(wrapper) { + try { + expect(wrapper).toContainReactComponent('button', { + type: 'reset', + disabled: false, + }); + expect(wrapper).toContainReactComponent('button', { + type: 'submit', + disabled: false, + }); + } catch { + return false; + } + return true; +} + +export function changeTitle(wrapper, newTitle) { + wrapper.find(TextField, {label: 'title'})!.trigger('onChange', newTitle); +} + +export function hitSubmit(wrapper) { + wrapper.find('button', {type: 'submit'})!.trigger('onClick', clickEvent()); +} + +export async function waitForSubmit(wrapper, successPromise) { + hitSubmit(wrapper); + + await wrapper.act(async () => { + await successPromise; + }); +} + +export function hitReset(wrapper) { + wrapper.find('button', {type: 'reset'})!.trigger('onClick', clickEvent()); +} + +export function hitClean(wrapper) { + wrapper.find('button', {type: 'button'})!.trigger('onClick', clickEvent()); +} + +export function fakeProduct(): SimpleProduct { + return { + title: faker.commerce.product(), + description: faker.lorem.paragraph(), + defaultVariant: { + price: faker.commerce.price(), + optionName: 'material', + optionValue: faker.commerce.productMaterial(), + }, + variants: Array.from({length: 2}).map(() => ({ + id: faker.random.uuid(), + price: faker.commerce.price(), + optionName: faker.lorem.word(), + optionValue: faker.commerce.productMaterial(), + })), + }; +} + +export function clickEvent() { + // we don't actually use these at all so it is ok to just return an empty object + return {} as any; +} diff --git a/packages/react-form/src/types.ts b/packages/react-form/src/types.ts index 4f8c51b67a..d7ba034045 100755 --- a/packages/react-form/src/types.ts +++ b/packages/react-form/src/types.ts @@ -65,6 +65,24 @@ export type FieldDictionary = { [Key in keyof Record]: Field; }; +export interface FormWithoutDynamicListsInput { + fields: T; + onSubmit?: SubmitHandler>; + makeCleanAfterSubmit?: boolean; +} + +export interface FormWithDynamicListsInput< + T extends FieldBag, + D extends DynamicListBag +> extends FormWithoutDynamicListsInput { + dynamicLists: D; +} + +export interface FormInput + extends FormWithoutDynamicListsInput { + dynamicLists?: D; +} + export interface Form { fields: T; dirty: boolean; @@ -74,7 +92,13 @@ export interface Form { reset(): void; submit(event?: React.FormEvent): void; makeClean(): void; - dynamicLists: DynamicListBag; +} + +export interface FormWithDynamicLists< + T extends FieldBag, + D extends DynamicListBag +> extends Form { + dynamicLists: D; } export interface FormError {