Upgrade React 16.9

Shopify · May 9, 2019 · 3175af5 · 3175af5
1 parent 65fd6b4
commit 3175af5
Show file tree

Hide file tree

Showing 12 changed files with 231 additions and 199 deletions.
diff --git a/package.json b/package.json
@@ -56,9 +56,9 @@
     "lerna": "^2.9.0",
     "plop": "^2.0.0",
     "prettier": "^1.14.0",
-    "react": "^16.8.5",
+    "react": "16.9.0-alpha.0",
     "react-apollo": "^2.2.3",
-    "react-dom": "^16.8.5",
+    "react-dom": "16.9.0-alpha.0",
     "react-helmet": "^5.2.0",
     "rimraf": "^2.6.2",
     "ts-jest": "^23.10.4",

diff --git a/packages/enzyme-utilities/src/index.ts b/packages/enzyme-utilities/src/index.ts
@@ -1,4 +1,4 @@
-import {act} from 'react-dom/test-utils';
+import {act as reactAct} from 'react-dom/test-utils';
 import {ReactWrapper, CommonWrapper} from 'enzyme';
 import {get} from 'lodash';
 
@@ -8,6 +8,8 @@ export type AnyWrapper =
   | CommonWrapper<any, any>
   | CommonWrapper<any, never>;
 
+const act = reactAct as (func: () => void | Promise<void>) => Promise<void>;
+
 export function trigger(wrapper: AnyWrapper, keypath: string, ...args: any[]) {
   if (wrapper.length === 0) {
     throw new Error(
@@ -33,19 +35,23 @@ export function trigger(wrapper: AnyWrapper, keypath: string, ...args: any[]) {
 
   let returnValue: any;
 
-  act(() => {
+  const promise = act(() => {
     returnValue = callback(...args);
-  });
 
-  updateRoot(wrapper);
+    if (isPromise(returnValue)) {
+      return (returnValue as unknown) as Promise<void>;
+    }
+  });
 
-  if (returnValue instanceof Promise) {
-    return returnValue.then(ret => {
+  if (isPromise(returnValue)) {
+    return Promise.resolve(promise as Promise<any>).then(ret => {
       updateRoot(wrapper);
       return ret;
     });
   }
 
+  updateRoot(wrapper);
+
   return returnValue;
 }
 
@@ -56,3 +62,11 @@ function updateRoot(wrapper: AnyWrapper) {
 export function findById(wrapper: ReactWrapper<any, any>, id: string) {
   return wrapper.find({id}).first();
 }
+
+function isPromise<T>(promise: T | Promise<T>): promise is Promise<T> {
+  return (
+    promise != null &&
+    typeof promise === 'object' &&
+    (promise as any).then != null
+  );
+}
diff --git a/packages/react-form/src/hooks/list/list.ts b/packages/react-form/src/hooks/list/list.ts
@@ -216,6 +216,9 @@ export function useList<Item extends object>(
                 }
                 validate();
               },
+              setError(error: string) {
+                dispatch(updateErrorAction({target, error}));
+              },
             };
           },
         );

diff --git a/packages/react-form/src/hooks/test/form.test.tsx b/packages/react-form/src/hooks/test/form.test.tsx
@@ -59,7 +59,9 @@ describe('useForm', () => {
     return (
       <form onSubmit={submit}>
         {submitting && <p>loading...</p>}
-        {submitErrors.length > 0 && <p>{submitErrors.join(', ')}</p>}
+        {submitErrors.length > 0 &&
+          submitErrors.map(({message}) => <p key={message}>{message}</p>)}
+
         <fieldset>
           <TextField label="title" {...title} />
           <TextField label="description" {...description} />
@@ -69,13 +71,15 @@ describe('useForm', () => {
           <TextField label="option" {...defaultVariant.optionName} />
           <TextField label="value" {...defaultVariant.optionValue} />
         </fieldset>
-        {variants.map(({price, optionName, optionValue, id}) => (
-          <fieldset name="default-variant" key={id.value}>
-            <TextField label="price" {...price} />
-            <TextField label="option" {...optionName} />
-            <TextField label="value" {...optionValue} />
-          </fieldset>
-        ))}
+        {variants.map(({price, optionName, optionValue, id}) => {
+          return (
+            <fieldset name="default-variant" key={id.value}>
+              <TextField label="price" {...price} />
+              <TextField label="option" {...optionName} />
+              <TextField label="value" {...optionValue} />
+            </fieldset>
+          );
+        })}
         <button type="button" disabled={!dirty} onClick={reset}>
           Reset
         </button>
@@ -144,7 +148,7 @@ describe('useForm', () => {
   });
 
   describe('submit', () => {
-    it('sets submitting to true during submission', () => {
+    it('sets submitting to true during submission', async () => {
       const wrapper = mount(
         <ProductForm
           data={fakeProduct()}
@@ -156,18 +160,18 @@ describe('useForm', () => {
         .find(TextField, {label: 'title'})!
         .trigger('onChange', 'tortoritos, the chip for turtles!');
 
-      wrapper
-        .find('button', {type: 'submit'})!
-        .trigger('onClick', clickEvent());
+      await wrapper.act(() => {
+        wrapper
+          .find('button', {type: 'submit'})!
+          .trigger('onClick', clickEvent());
+      });
 
       expect(wrapper).toContainReactComponent('p', {
         children: 'loading...',
       });
     });
 
-    // Async act() issues block this test https://github.com/facebook/react/issues/15379
-    // eslint-disable-next-line jest/no-disabled-tests
-    it.skip('sets submitting to false when submission ends', async () => {
+    it('sets submitting to false when submission ends', async () => {
       const promise = Promise.resolve(submitSuccess());
       const wrapper = mount(
         <ProductForm data={fakeProduct()} onSubmit={() => promise} />,
@@ -177,18 +181,19 @@ describe('useForm', () => {
         .find(TextField, {label: 'title'})!
         .trigger('onChange', 'tortoritos, the chip for turtles!');
 
-      wrapper
-        .find('button', {type: 'submit'})!
-        .trigger('onClick', clickEvent());
-
-      await promise;
+      await wrapper.act(async () => {
+        await wrapper
+          .find('button', {type: 'submit'})!
+          .trigger('onClick', clickEvent());
+        await promise;
+      });
 
       expect(wrapper).not.toContainReactComponent('p', {
         children: 'loading...',
       });
     });
 
-    it('validates all fields with their latest values before submitting and bails out if any fail', () => {
+    it('validates all fields with their latest values before submitting and bails out if any fail', async () => {
       const submitSpy = jest.fn(() => Promise.resolve(submitSuccess()));
       const product = {
         ...fakeProduct(),
@@ -200,35 +205,38 @@ describe('useForm', () => {
 
       wrapper.find(TextField, {label: 'title'})!.trigger('onChange', '');
 
-      wrapper
-        .find('button', {type: 'submit'})!
-        .trigger('onClick', clickEvent());
+      await wrapper.act(() => {
+        wrapper
+          .find('button', {type: 'submit'})!
+          .trigger('onClick', clickEvent());
+      });
 
       expect(submitSpy).not.toHaveBeenCalled();
       expect(wrapper).toContainReactComponent('p', {
         children: 'Title is required!',
       });
     });
 
-    // Async act() issues block this test https://github.com/facebook/react/issues/15379
-    // eslint-disable-next-line jest/no-disabled-tests
-    it.skip('returns remote submission errors', async () => {
+    it('returns remote submission errors', async () => {
       const error = {message: 'The server hates it'};
-      const submitSpy = jest.fn(() => Promise.resolve(submitFail([error])));
+      const promise = Promise.resolve(submitFail([error]));
+
       const wrapper = mount(
-        <ProductForm data={fakeProduct()} onSubmit={submitSpy} />,
+        <ProductForm data={fakeProduct()} onSubmit={() => promise} />,
       );
 
-      await wrapper
-        .find('button', {type: 'submit'})!
-        .trigger('onClick', clickEvent());
+      await wrapper.act(async () => {
+        await wrapper
+          .find('button', {type: 'submit'})!
+          .trigger('onClick', clickEvent());
+      });
 
       expect(wrapper).toContainReactComponent('p', {
         children: error.message,
       });
     });
 
-    // Async act() issues block this test https://github.com/facebook/react/issues/15379
+    // This test goes into an infinite render loop
     // eslint-disable-next-line jest/no-disabled-tests
     it.skip('propagates remote submission errors to matching fields', async () => {
       const errors = [
@@ -237,14 +245,20 @@ describe('useForm', () => {
           message: 'The server hates your price',
         },
       ];
-      const submitSpy = jest.fn(() => Promise.resolve(submitFail(errors)));
+
+      const promise = Promise.resolve(submitFail(errors));
+
       const wrapper = mount(
-        <ProductForm data={fakeProduct()} onSubmit={submitSpy} />,
+        <ProductForm data={fakeProduct()} onSubmit={() => promise} />,
       );
 
-      await wrapper
-        .find('button', {type: 'submit'})!
-        .trigger('onClick', clickEvent());
+      await wrapper.act(async () => {
+        wrapper
+          .find('button', {type: 'submit'})!
+          .trigger('onClick', clickEvent());
+
+        await promise;
+      });
 
       expect(wrapper).toContainReactComponent(TextField, {
         error: errors[0].message,
@@ -277,27 +291,30 @@ describe('useForm', () => {
       });
     });
 
-    // Async act() issues block this test https://github.com/facebook/react/issues/15379
-    // eslint-disable-next-line jest/no-disabled-tests
-    it.skip('resets errors from previous submissions', async () => {
+    it('resets errors from previous submissions', async () => {
       const errors = [
         {
           field: ['variants', '0', 'price'],
           message: 'The server hates your price',
         },
       ];
-      const submitSpy = jest.fn(() => Promise.resolve(submitFail(errors)));
+
+      const promise = Promise.resolve(submitFail(errors));
+
       const wrapper = mount(
-        <ProductForm data={fakeProduct()} onSubmit={submitSpy} />,
+        <ProductForm data={fakeProduct()} onSubmit={() => promise} />,
       );
 
       await wrapper
         .find('button', {type: 'submit'})!
         .trigger('onClick', clickEvent());
 
-      await wrapper
-        .find('button', {type: 'reset'})!
-        .trigger('onClick', clickEvent());
+      await wrapper.act(async () => {
+        await wrapper
+          .find('button', {type: 'button'})!
+          .trigger('onClick', clickEvent());
+        await promise;
+      });
 
       expect(wrapper).not.toContainReactComponent(TextField, {
         error: errors[0].message,

diff --git a/packages/react-graphql/src/hooks/tests/mutation.test.tsx b/packages/react-graphql/src/hooks/tests/mutation.test.tsx
@@ -1,4 +1,5 @@
 import * as React from 'react';
+import {act} from 'react-dom/test-utils';
 import gql from 'graphql-tag';
 import {createGraphQLFactory} from '@shopify/graphql-testing';
 
@@ -40,12 +41,7 @@ const mockMutationData = {
   updatePet: {id: 'new-pet-id-123', __typename: 'Cat'},
 };
 
-// This is skip because current `act` wrapper does not support async operation
-// The test will pass if we update to `react-dom` v16.9.0-alpha.0
-// and wrap `graphQL.resolveAll()` in an `act`
-// https://github.com/facebook/react/issues/14769#issuecomment-481251431
-// eslint-disable-next-line jest/no-disabled-tests
-describe.skip('useMutation', () => {
+describe('useMutation', () => {
   beforeEach(() => {
     prepareAsyncReactTasks();
   });
@@ -63,7 +59,11 @@ describe.skip('useMutation', () => {
     );
 
     mockMutation.find('button')!.trigger('onClick', undefined as any);
-    await graphQL.resolveAll();
+
+    // @ts-ignore
+    await act(async () => {
+      await graphQL.resolveAll();
+    });
 
     expect(renderPropSpy).toHaveBeenLastCalledWith(
       expect.objectContaining({data: mockMutationData}),

diff --git a/packages/react-graphql/src/hooks/tests/query.test.tsx b/packages/react-graphql/src/hooks/tests/query.test.tsx
@@ -125,11 +125,16 @@ describe('useQuery', () => {
       const graphQL = createGraphQL({PetQuery: mockData});
       const renderPropSpy = jest.fn(() => null);
 
-      // below line should be wrapped in act once we update to `react-dom` v16.9.0-alpha.0
-      await MockQueryComponent.resolve();
-      await mountWithGraphQL(<MockQuery>{renderPropSpy}</MockQuery>, {
-        graphQL,
-        skipInitialGraphQL: true,
+      const wrapper = await mountWithGraphQL(
+        <MockQuery>{renderPropSpy}</MockQuery>,
+        {
+          graphQL,
+          skipInitialGraphQL: true,
+        },
+      );
+
+      await wrapper.act(async () => {
+        await MockQueryComponent.resolve();
       });
 
       expect(renderPropSpy).toHaveBeenLastCalledWith(
@@ -152,10 +157,16 @@ describe('useQuery', () => {
       const graphQL = createGraphQL({PetQuery: mockData});
       const renderPropSpy = jest.fn(() => null);
 
-      // below line should be wrapped in act once we update to `react-dom` v16.9.0-alpha.0
-      await MockQueryComponent.resolve();
-      await mountWithGraphQL(<MockQuery>{renderPropSpy}</MockQuery>, {
-        graphQL,
+      const wrapper = await mountWithGraphQL(
+        <MockQuery>{renderPropSpy}</MockQuery>,
+        {
+          graphQL,
+        },
+      );
+
+      await wrapper.act(async () => {
+        await MockQueryComponent.resolve();
+        await graphQL.resolveAll();
       });
 
       expect(renderPropSpy).toHaveBeenLastCalledWith(
-Original file line number
+Diff line change
@@ Expand Up / @@ -216,6 +216,9 @@ export function useList<Item extends object>( @@
                     }
                     validate();
                   },
+                  setError(error: string) {
+                    dispatch(updateErrorAction({target, error}));
+                  },
                 };
               },
             );
@@ Expand Down @@