feedback

packages/enzyme-utilities/src/index.ts

@@ -8,6 +8,9 @@ export type AnyWrapper =
   | CommonWrapper<any, any>
   | CommonWrapper<any, never>;
 
+// Manually casting `act()` until @types/react is updated to include
+// the Promise types for async act introduced in version 16.9.0-alpha.0
+// https://github.com/Shopify/quilt/issues/692
 const act = reactAct as (func: () => void | Promise<void>) => Promise<void>;
 
 export function trigger(wrapper: AnyWrapper, keypath: string, ...args: any[]) {
@@ -38,6 +41,9 @@ export function trigger(wrapper: AnyWrapper, keypath: string, ...args: any[]) {
   const promise = act(() => {
     returnValue = callback(...args);
 
+    // The return type of non-async `act()`, DebugPromiseLike, contains a `then` method
+    // This condition checks the returned value is an actual Promise and returns it
+    // to React’s `act()` call, otherwise we just want to return `undefined`
     if (isPromise(returnValue)) {
       return (returnValue as unknown) as Promise<void>;
     }
@@ -65,8 +71,6 @@ export function findById(wrapper: ReactWrapper<any, any>, id: string) {
 
 function isPromise<T>(promise: T | Promise<T>): promise is Promise<T> {
   return (
-    promise != null &&
-    typeof promise === 'object' &&
-    (promise as any).then != null
+    promise != null && typeof promise === 'object' && 'then' in (promise as any)
   );
 }

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

@@ -148,7 +148,7 @@ describe('useForm', () => {
   });
 
   describe('submit', () => {
-    it('sets submitting to true during submission', async () => {
+    it('sets submitting to true during submission', () => {
       const wrapper = mount(
         <ProductForm
           data={fakeProduct()}
@@ -160,7 +160,7 @@ describe('useForm', () => {
         .find(TextField, {label: 'title'})!
         .trigger('onChange', 'tortoritos, the chip for turtles!');
 
-      await wrapper.act(() => {
+      wrapper.act(() => {
         wrapper
           .find('button', {type: 'submit'})!
           .trigger('onClick', clickEvent());
@@ -182,7 +182,7 @@ describe('useForm', () => {
         .trigger('onChange', 'tortoritos, the chip for turtles!');
 
       await wrapper.act(async () => {
-        await wrapper
+        wrapper
           .find('button', {type: 'submit'})!
           .trigger('onClick', clickEvent());
         await promise;
@@ -193,7 +193,7 @@ describe('useForm', () => {
       });
     });
 
-    it('validates all fields with their latest values before submitting and bails out if any fail', async () => {
+    it('validates all fields with their latest values before submitting and bails out if any fail', () => {
       const submitSpy = jest.fn(() => Promise.resolve(submitSuccess()));
       const product = {
         ...fakeProduct(),
@@ -205,7 +205,7 @@ describe('useForm', () => {
 
       wrapper.find(TextField, {label: 'title'})!.trigger('onChange', '');
 
-      await wrapper.act(() => {
+      wrapper.act(() => {
         wrapper
           .find('button', {type: 'submit'})!
           .trigger('onClick', clickEvent());
@@ -236,9 +236,7 @@ describe('useForm', () => {
       });
     });
 
-    // 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 () => {
+    it('propagates remote submission errors to matching fields', async () => {
       const errors = [
         {
           field: ['variants', '0', 'price'],
@@ -310,7 +308,7 @@ describe('useForm', () => {
         .trigger('onClick', clickEvent());
 
       await wrapper.act(async () => {
-        await wrapper
+        wrapper
           .find('button', {type: 'button'})!
           .trigger('onClick', clickEvent());
         await promise;

packages/react-graphql/src/hooks/tests/mutation.test.tsx

@@ -1,5 +1,4 @@
 import * as React from 'react';
-import {act} from 'react-dom/test-utils';
 import gql from 'graphql-tag';
 import {createGraphQLFactory} from '@shopify/graphql-testing';
 
@@ -58,10 +57,11 @@ describe('useMutation', () => {
       {graphQL},
     );
 
-    mockMutation.find('button')!.trigger('onClick', undefined as any);
+    mockMutation.act(() => {
+      mockMutation.find('button')!.trigger('onClick', undefined as any);
+    });
 
-    // @ts-ignore
-    await act(async () => {
+    await mockMutation.act(async () => {
       await graphQL.resolveAll();
     });
 

packages/react-import-remote/src/test/ImportRemote.test.tsx

@@ -87,6 +87,7 @@ describe('<ImportRemote />', () => {
 
     it('imports a new global if the source changes', async () => {
       const nonce = '';
+      const resolve = createResolvableLoad();
       const importRemote = mount(<ImportRemote {...mockProps} />);
 
       expect(load).toHaveBeenCalledWith(
@@ -97,11 +98,9 @@ describe('<ImportRemote />', () => {
 
       const newSource = 'https://bar.com/foo.js';
 
-      // we need to have the async here otherwise we will get
-      // a console error an `act(...)` here
-      // eslint-disable-next-line require-await
       await importRemote.act(async () => {
         importRemote.setProps({source: newSource});
+        await resolve();
       });
 
       expect(load).toHaveBeenCalledWith(newSource, mockProps.getImport, nonce);
@@ -119,7 +118,7 @@ describe('<ImportRemote />', () => {
       );
 
       await importRemote.act(async () => {
-        await resolve;
+        await resolve();
       });
 
       expect(spy).toHaveBeenCalledWith(result);

packages/react-testing/src/element.ts

@@ -173,7 +173,7 @@ export class Element<Props> {
   trigger<K extends FunctionKeys<Props>>(
     prop: K,
     ...args: DeepPartialArguments<Arguments<Props[K]>>
-  ): Promise<ReturnType<NonNullable<Props[K]>>> {
+  ): ReturnType<NonNullable<Props[K]>> {
     return this.root.act(() => {
       const propValue = this.props[prop];
 
@@ -187,7 +187,7 @@ export class Element<Props> {
     });
   }
 
-  triggerKeypath<T = unknown>(keypath: string, ...args: unknown[]): Promise<T> {
+  triggerKeypath<T = unknown>(keypath: string, ...args: unknown[]): T {
     return this.root.act(() => {
       const {props} = this;
       const parts = keypath.split(/[.[\]]/g).filter(Boolean);

packages/react-testing/src/root.tsx

@@ -17,6 +17,9 @@ import {
   DeepPartialArguments,
 } from './types';
 
+// Manually casting `act()` until @types/react is updated to include
+// the Promise types for async act introduced in version 16.9.0-alpha.0
+// https://github.com/Shopify/quilt/issues/692
 const act = reactAct as (func: () => void | Promise<void>) => Promise<void>;
 
 // eslint-disable-next-line typescript/no-var-requires
@@ -95,6 +98,9 @@ export class Root<Props> {
     const promise = act(() => {
       result = action();
 
+      // The return type of non-async `act()`, DebugPromiseLike, contains a `then` method
+      // This condition checks the returned value is an actual Promise and returns it
+      // to React’s `act()` call, otherwise we just want to return `undefined`
       if (isPromise(result)) {
         return (result as unknown) as Promise<void>;
       }
@@ -150,7 +156,7 @@ export class Root<Props> {
   trigger<K extends FunctionKeys<Props>>(
     prop: K,
     ...args: DeepPartialArguments<Arguments<Props[K]>>
-  ): Promise<ReturnType<NonNullable<Props[K]>>> {
+  ): ReturnType<NonNullable<Props[K]>> {
     return this.withRoot(root => root.trigger(prop, ...(args as any)));
   }
 
@@ -299,8 +305,6 @@ function childrenToTree(fiber: Fiber | null, root: Root<unknown>) {
 
 function isPromise<T>(promise: T | Promise<T>): promise is Promise<T> {
   return (
-    promise != null &&
-    typeof promise === 'object' &&
-    (promise as any).then != null
+    promise != null && typeof promise === 'object' && 'then' in (promise as any)
   );
 }