improve dynamicLists typing

packages/react-form/src/hooks/form.ts

@@ -2,18 +2,20 @@ 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';
 
 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<T extends FieldBag>({
   fields,
   onSubmit,
+  makeCleanAfterSubmit,
+}: FormWithoutDynamicListsInput<T>): Form<T>;
+
+export function useForm<T extends FieldBag, D extends DynamicListBag>({
+  fields,
+  dynamicLists,
+  onSubmit,
+  makeCleanAfterSubmit,
+}: FormWithDynamicListsInput<T, D>): FormWithDynamicLists<T, D>;
+
+export function useForm<T extends FieldBag, D extends DynamicListBag>({
+  fields,
   dynamicLists,
+  onSubmit,
   makeCleanAfterSubmit = false,
-}: {
-  fields: T;
-  onSubmit?: SubmitHandler<FormMapping<T, 'value'>>;
-  makeCleanAfterSubmit?: boolean;
-  dynamicLists?: DynamicListBag;
-}): Form<T> {
+}: FormInput<T, D>) {
   const fieldsWithLists = useMemo(() => {
     if (dynamicLists) {
       const fieldsWithList = {...fields};
@@ -128,13 +138,7 @@ export function useForm<T extends FieldBag>({
     fieldsRef,
   ]);
 
-  const defaultDynamicListBag = {
-    defaultDynamicList: useDynamicList<{}>([], () => {
-      return {};
-    }),
-  };
-
-  return {
+  const form: Form<T> = {
     fields,
     dirty: dirty || dynamicListDirty,
     submitting,
@@ -143,6 +147,11 @@ export function useForm<T extends FieldBag>({
     validate,
     makeClean,
     submitErrors: errors,
-    dynamicLists: dynamicLists || defaultDynamicListBag,
   };
+
+  if (dynamicLists) {
+    return {...form, dynamicLists};
+  }
+
+  return form;
 }

packages/react-form/src/hooks/list/test/dynamiclist.test.tsx

@@ -13,9 +13,14 @@ describe('useDynamicList', () => {
       return {price: '', optionName: '', optionValue: ''};
     };
     function DynamicListComponent(config: FieldListConfig<Variant>) {
-      const {fields, addItem, removeItem, moveItem, reset} = useDynamicList<
-        Variant
-      >(config, factory);
+      const {
+        fields,
+        addItem,
+        removeItem,
+        moveItem,
+        reset,
+        dirty,
+      } = useDynamicList<Variant>(config, factory);
 
       return (
         <>
@@ -48,6 +53,7 @@ describe('useDynamicList', () => {
           <button type="button" onClick={reset}>
             Reset
           </button>
+          <p>Dirty: {dirty.toString()}</p>
         </>
       );
     }
@@ -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(
+          <DynamicListComponent list={randomVariants(1)} />,
+        );
+
+        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(<DynamicListComponent list={randomVariants()} />);
+
+        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(
+          <DynamicListComponent list={randomVariants(1)} />,
+        );
+
+        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', () => {

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(
+        <FormWithDynamicVariantList data={fakeProduct()} />,
+      );
+
+      expect(isDirty(wrapper)).toBe(false);
+    });
+
+    it('dirty state is true when a new variant item has been added', () => {
+      const wrapper = mount(
+        <FormWithDynamicVariantList data={fakeProduct()} />,
+      );
+
+      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(
+        <FormWithDynamicVariantList data={fakeProduct()} />,
+      );
+
+      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(
+        <FormWithDynamicVariantList data={fakeProduct()} />,
+      );
+
+      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(
+        <FormWithDynamicVariantList data={product} onSubmit={submitSpy} />,
+      );
+
+      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(
+        <FormWithDynamicVariantList
+          data={fakeProduct()}
+          onSubmit={() => 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(
+        <FormWithDynamicVariantList data={fakeProduct()} />,
+      );
+
+      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(
+        <FormWithDynamicVariantList data={fakeProduct()} />,
+      );
+
+      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);
+    });
+  });
+});