Merge pull request #2141 from Shopify/update-trigger

Updated trigger to use act

UNRELEASED.md

@@ -10,6 +10,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Enhancements
 
+- Updated `trigger` to use `act` ([#2141](https://github.com/Shopify/polaris-react/pull/2141))
 - Changed border color of `Drop zone` to have better contrast from the background and to be lighter when disabled ([#2119](https://github.com/Shopify/polaris-react/pull/2119))
 - Adjusted search results overlay to take up 100% height of the screen on small screens and to match the width of the search bar on large screens. ([#2103](https://github.com/Shopify/polaris-react/pull/2103))
 - Added skipToContentTarget prop to Frame component ([#2080](https://github.com/Shopify/polaris-react/pull/2080))

src/components/ActionMenu/components/RollupActions/tests/RollupActions.test.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import {ReactWrapper} from 'enzyme';
 import {HorizontalDotsMinor} from '@shopify/polaris-icons';
-import {mountWithAppProvider, trigger, act} from 'test-utilities/legacy';
+import {mountWithAppProvider, trigger} from 'test-utilities/legacy';
 
 import {Button, Popover} from 'components';
 
@@ -60,10 +60,7 @@ describe('<RollupActions />', () => {
       expect(popoverComponent.prop('active')).toBe(true);
 
       const firstActionListItem = wrapper.find(ActionListItem).first();
-      act(() => {
-        trigger(firstActionListItem, 'onAction');
-      });
-      popoverComponent.update();
+      trigger(firstActionListItem, 'onAction');
 
       popoverComponent = wrapper.find(Popover);
       expect(popoverComponent.prop('active')).toBe(false);
@@ -110,8 +107,5 @@ function findPopoverActivator(wrapper: Wrapper) {
 
 function activatePopover(wrapper: Wrapper) {
   const activator = findPopoverActivator(wrapper);
-  act(() => {
-    trigger(activator, 'onClick');
-  });
-  wrapper.update();
+  trigger(activator, 'onClick');
 }

src/components/Card/tests/Card.test.tsx

@@ -3,7 +3,6 @@ import {
   mountWithAppProvider,
   trigger,
   findByTestID,
-  act,
 } from 'test-utilities/legacy';
 import {Card, Badge, Button, Popover, ActionList} from 'components';
 import {WithinContentContext} from '../../../utilities/within-content-context';
@@ -141,10 +140,8 @@ describe('<Card />', () => {
       expect(popover).toHaveLength(1);
       expect(popover.prop('active')).toBe(false);
 
-      act(() => {
-        trigger(disclosureButton, 'onClick');
-      });
-      card.update();
+      trigger(disclosureButton, 'onClick');
+
       expect(
         card
           .find(Popover)

src/components/Frame/components/ContextualSaveBar/tests/ContextualSaveBar.test.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import {act, mountWithAppProvider, trigger} from 'test-utilities/legacy';
+import {mountWithAppProvider, trigger} from 'test-utilities/legacy';
 import {Button, Image} from 'components';
 import {ContextualSaveBar} from '../ContextualSaveBar';
 import {DiscardConfirmationModal} from '../components';
@@ -117,9 +117,7 @@ describe('<ContextualSaveBar />', () => {
         const discardConfirmationModal = contextualSaveBar.find(
           DiscardConfirmationModal,
         );
-        act(() => {
-          trigger(discardConfirmationModal, 'onCancel');
-        });
+        trigger(discardConfirmationModal, 'onCancel');
 
         expect(discardConfirmationModal.prop('open')).toBe(false);
       });
@@ -140,9 +138,8 @@ describe('<ContextualSaveBar />', () => {
           DiscardConfirmationModal,
         );
 
-        act(() => {
-          trigger(discardConfirmationModal, 'onDiscard');
-        });
+        trigger(discardConfirmationModal, 'onDiscard');
+
         expect(discardAction.onAction).toHaveBeenCalled();
       });
 

src/components/Navigation/components/Item/tests/Item.test.tsx

@@ -2,7 +2,7 @@ import React from 'react';
 import {PlusMinor} from '@shopify/polaris-icons';
 import {matchMedia} from '@shopify/jest-dom-mocks';
 import {Icon, UnstyledLink, Indicator, Badge} from 'components';
-import {act, trigger, mountWithAppProvider} from 'test-utilities/legacy';
+import {trigger, mountWithAppProvider} from 'test-utilities/legacy';
 import {NavigationContext} from '../../../context';
 
 import {Item, ItemProps} from '../Item';
@@ -61,13 +61,11 @@ describe('<Nav.Item />', () => {
     const item = itemForLocation('/admin/orders');
 
     matchMedia.setMedia(() => ({matches: true}));
-    act(() => {
-      trigger(item.find(UnstyledLink).first(), 'onClick', {
-        preventDefault: jest.fn(),
-        currentTarget: {
-          getAttribute: () => '/admin/orders',
-        },
-      });
+    trigger(item.find(UnstyledLink).first(), 'onClick', {
+      preventDefault: jest.fn(),
+      currentTarget: {
+        getAttribute: () => '/admin/orders',
+      },
     });
 
     expect(item.find(Secondary).prop('expanded')).toBe(true);

src/test-utilities/legacy.tsx

@@ -9,8 +9,6 @@ import {
   WithPolarisTestProviderOptions,
 } from '../components';
 
-export {act};
-
 export type AnyWrapper = ReactWrapper<any, any> | CommonWrapper<any, any>;
 
 export function findByTestID(root: ReactWrapper<any, any>, id: string) {
@@ -30,6 +28,8 @@ export function matchByTestID(root: ReactWrapper<any, any>, regexp: RegExp) {
   return root.findWhere(matchesTestID);
 }
 
+const reactAct = act as (func: () => void | Promise<void>) => Promise<void>;
+
 export function trigger(wrapper: AnyWrapper, keypath: string, ...args: any[]) {
   if (wrapper.length === 0) {
     throw new Error(
@@ -53,20 +53,38 @@ export function trigger(wrapper: AnyWrapper, keypath: string, ...args: any[]) {
     );
   }
 
-  // eslint-disable-next-line callback-return
-  const returnValue = callback(...args);
-  updateRoot(wrapper);
+  let returnValue: any;
+
+  const promise = reactAct(() => {
+    // eslint-disable-next-line callback-return
+    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>;
+    }
+  });
 
-  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;
 }
 
+function isPromise<T>(promise: T | Promise<T>): promise is Promise<T> {
+  return (
+    promise != null && typeof promise === 'object' && 'then' in (promise as any)
+  );
+}
+
 function updateRoot(wrapper: AnyWrapper) {
   (wrapper as any).root().update();
 }