From d5f90054b3b2973e9d08fe6772c19260c211f456 Mon Sep 17 00:00:00 2001
From: Adam Walker <awalker@stripe.com>
Date: Tue, 3 Jan 2023 15:20:09 -0800
Subject: [PATCH 1/2] Make sure events are attached if useEffect happens before
 useLayoutEffect

---
 src/components/createElementComponent.tsx | 74 ++++++++++++++++++-----
 src/utils/useAttachEvent.ts               | 21 +++++--
 2 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/src/components/createElementComponent.tsx b/src/components/createElementComponent.tsx
index 53132e8..63fb534 100644
--- a/src/components/createElementComponent.tsx
+++ b/src/components/createElementComponent.tsx
@@ -80,22 +80,42 @@ const createElementComponent = (
     // For every event where the merchant provides a callback, call element.on
     // with that callback. If the merchant ever changes the callback, removes
     // the old callback with element.off and then call element.on with the new one.
-    useAttachEvent(elementRef, 'blur', onBlur);
-    useAttachEvent(elementRef, 'focus', onFocus);
-    useAttachEvent(elementRef, 'escape', onEscape);
-    useAttachEvent(elementRef, 'click', onClick);
-    useAttachEvent(elementRef, 'loaderror', onLoadError);
-    useAttachEvent(elementRef, 'loaderstart', onLoaderStart);
-    useAttachEvent(elementRef, 'networkschange', onNetworksChange);
-    useAttachEvent(elementRef, 'lineitemclick', onLineItemClick);
-    useAttachEvent(elementRef, 'confirm', onConfirm);
-    useAttachEvent(elementRef, 'cancel', onCancel);
-    useAttachEvent(
+    const attachBlurEvent = useAttachEvent(elementRef, 'blur', onBlur);
+    const attachFocusEvent = useAttachEvent(elementRef, 'focus', onFocus);
+    const attachEscapeEvent = useAttachEvent(elementRef, 'escape', onEscape);
+    const attachClickEvent = useAttachEvent(elementRef, 'click', onClick);
+    const attachLoadErrorEvent = useAttachEvent(
+      elementRef,
+      'loaderror',
+      onLoadError
+    );
+    const attachLoaderStartEvent = useAttachEvent(
+      elementRef,
+      'loaderstart',
+      onLoaderStart
+    );
+    const attachNetworksChangeEvent = useAttachEvent(
+      elementRef,
+      'networkschange',
+      onNetworksChange
+    );
+    const attachLineItemClickEvent = useAttachEvent(
+      elementRef,
+      'lineitemclick',
+      onLineItemClick
+    );
+    const attachConfirmEvent = useAttachEvent(elementRef, 'confirm', onConfirm);
+    const attachCancelEvent = useAttachEvent(elementRef, 'cancel', onCancel);
+    const attachShippingAddressChangeEvent = useAttachEvent(
       elementRef,
       'shippingaddresschange',
       onShippingAddressChange
     );
-    useAttachEvent(elementRef, 'shippingratechange', onShippingRateChange);
+    const attachShippingRateChangeEvent = useAttachEvent(
+      elementRef,
+      'shippingratechange',
+      onShippingRateChange
+    );
 
     let readyCallback: UnknownCallback | undefined;
     if (type === 'cart') {
@@ -117,7 +137,7 @@ const createElementComponent = (
       }
     }
 
-    useAttachEvent(elementRef, 'ready', readyCallback);
+    const attachReadyEvent = useAttachEvent(elementRef, 'ready', readyCallback);
 
     const changeCallback =
       type === 'cart'
@@ -127,7 +147,11 @@ const createElementComponent = (
           }
         : onChange;
 
-    useAttachEvent(elementRef, 'change', changeCallback);
+    const attachChangeEvent = useAttachEvent(
+      elementRef,
+      'change',
+      changeCallback
+    );
 
     const checkoutCallback =
       type === 'cart'
@@ -137,7 +161,11 @@ const createElementComponent = (
           }
         : onCheckout;
 
-    useAttachEvent(elementRef, 'checkout', checkoutCallback);
+    const attachCheckoutEvent = useAttachEvent(
+      elementRef,
+      'checkout',
+      checkoutCallback
+    );
 
     React.useLayoutEffect(() => {
       if (elementRef.current == null && elements && domNode.current != null) {
@@ -149,6 +177,22 @@ const createElementComponent = (
         }
         elementRef.current = element;
         element.mount(domNode.current);
+
+        attachBlurEvent();
+        attachFocusEvent();
+        attachEscapeEvent();
+        attachClickEvent();
+        attachLoadErrorEvent();
+        attachLoaderStartEvent();
+        attachNetworksChangeEvent();
+        attachLineItemClickEvent();
+        attachConfirmEvent();
+        attachCancelEvent();
+        attachShippingAddressChangeEvent();
+        attachShippingRateChangeEvent();
+        attachReadyEvent();
+        attachChangeEvent();
+        attachCheckoutEvent();
       }
     });
 
diff --git a/src/utils/useAttachEvent.ts b/src/utils/useAttachEvent.ts
index acbd8b4..14182b9 100644
--- a/src/utils/useAttachEvent.ts
+++ b/src/utils/useAttachEvent.ts
@@ -6,15 +6,28 @@ export const useAttachEvent = <A extends unknown[]>(
   event: string,
   cb?: (...args: A) => any
 ) => {
-  React.useEffect(() => {
-    if (!cb || !elementRef.current) {
-      return () => {};
+  const removePreviousEvent = React.useRef(() => {});
+
+  const addEventCallback = React.useCallback(() => {
+    removePreviousEvent.current();
+
+    if (!elementRef.current || !cb) {
+      removePreviousEvent.current = () => {};
+      return;
     }
 
     const element = elementRef.current;
 
     (element as any).on(event, cb);
 
-    return () => (element as any).off(event, cb);
+    removePreviousEvent.current = () => {
+      if (element === elementRef.current) {
+        (element as any).off(event, cb);
+      }
+    };
   }, [cb, event, elementRef]);
+
+  React.useEffect(() => addEventCallback(), [addEventCallback]);
+
+  return addEventCallback;
 };

From 058828deecb94d5ed202b990d295fcdd55124d15 Mon Sep 17 00:00:00 2001
From: Adam Walker <awalker@stripe.com>
Date: Tue, 3 Jan 2023 15:20:32 -0800
Subject: [PATCH 2/2] Make sure .on is only called once

---
 src/components/createElementComponent.test.tsx | 18 ++++++++++++++++++
 src/utils/useAttachEvent.ts                    |  2 +-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/components/createElementComponent.test.tsx b/src/components/createElementComponent.test.tsx
index 5ee650d..53173c5 100644
--- a/src/components/createElementComponent.test.tsx
+++ b/src/components/createElementComponent.test.tsx
@@ -253,6 +253,22 @@ describe('createElementComponent', () => {
       );
     });
 
+    it('adds an event handler', () => {
+      const mockHandler = jest.fn();
+      render(
+        <Elements stripe={mockStripe}>
+          <CardElement onChange={mockHandler} />
+        </Elements>
+      );
+
+      expect(simulateOn).toBeCalledTimes(1);
+      expect(simulateOn).toBeCalledWith('change', mockHandler);
+
+      const changeEventMock = Symbol('change');
+      simulateEvent('change', changeEventMock);
+      expect(mockHandler).toHaveBeenCalledWith(changeEventMock);
+    });
+
     it('removes old event handlers when an event handler changes', () => {
       const mockHandler = jest.fn();
       const mockHandler2 = jest.fn();
@@ -263,6 +279,8 @@ describe('createElementComponent', () => {
       );
 
       expect(simulateOn).toBeCalledWith('change', mockHandler);
+
+      expect(simulateOn).toBeCalledTimes(1);
       expect(simulateOff).not.toBeCalled();
 
       rerender(
diff --git a/src/utils/useAttachEvent.ts b/src/utils/useAttachEvent.ts
index 14182b9..3ceb189 100644
--- a/src/utils/useAttachEvent.ts
+++ b/src/utils/useAttachEvent.ts
@@ -27,7 +27,7 @@ export const useAttachEvent = <A extends unknown[]>(
     };
   }, [cb, event, elementRef]);
 
-  React.useEffect(() => addEventCallback(), [addEventCallback]);
+  React.useLayoutEffect(() => addEventCallback(), [addEventCallback]);
 
   return addEventCallback;
 };