From 00f3e0051d3ff5a1398c6b4cc1db944e2ac22a0b Mon Sep 17 00:00:00 2001 From: awalker-stripe <51334696+awalker-stripe@users.noreply.github.com> Date: Thu, 8 Dec 2022 09:13:30 -0800 Subject: [PATCH] Only call `element.on` when the merchant passes in a callback (#360) * Only attach callbacks the merchant defines * Update tests --- .../createElementComponent.test.tsx | 225 +++++++++++------- src/components/createElementComponent.tsx | 214 ++++++----------- src/utils/useAttachEvent.ts | 20 ++ src/utils/useCallbackReference.ts | 17 -- 4 files changed, 237 insertions(+), 239 deletions(-) create mode 100644 src/utils/useAttachEvent.ts delete mode 100644 src/utils/useCallbackReference.ts diff --git a/src/components/createElementComponent.test.tsx b/src/components/createElementComponent.test.tsx index a712e08..5ee650d 100644 --- a/src/components/createElementComponent.test.tsx +++ b/src/components/createElementComponent.test.tsx @@ -19,21 +19,13 @@ describe('createElementComponent', () => { let mockElements: any; let mockElement: any; let mockCartElementContext: any; - let simulateChange: any; - let simulateBlur: any; - let simulateFocus: any; - let simulateEscape: any; - let simulateReady: any; - let simulateClick: any; - let simulateLoadError: any; - let simulateLoaderStart: any; - let simulateNetworksChange: any; - let simulateCheckout: any; - let simulateLineItemClick: any; - let simulateConfirm: any; - let simulateCancel: any; - let simulateShippingAddressChange: any; - let simulateShippingRateChange: any; + + let simulateElementsEvents: Record; + let simulateOn: any; + let simulateOff: any; + const simulateEvent = (event: string, ...args: any[]) => { + simulateElementsEvents[event].forEach((fn) => fn(...args)); + }; beforeEach(() => { mockStripe = mocks.mockStripe(); @@ -42,57 +34,23 @@ describe('createElementComponent', () => { mockStripe.elements.mockReturnValue(mockElements); mockElements.create.mockReturnValue(mockElement); jest.spyOn(React, 'useLayoutEffect'); - mockElement.on = jest.fn((event, fn) => { - switch (event) { - case 'change': - simulateChange = fn; - break; - case 'blur': - simulateBlur = fn; - break; - case 'focus': - simulateFocus = fn; - break; - case 'escape': - simulateEscape = fn; - break; - case 'ready': - simulateReady = fn; - break; - case 'click': - simulateClick = fn; - break; - case 'loaderror': - simulateLoadError = fn; - break; - case 'loaderstart': - simulateLoaderStart = fn; - break; - case 'networkschange': - simulateNetworksChange = fn; - break; - case 'checkout': - simulateCheckout = fn; - break; - case 'lineitemclick': - simulateLineItemClick = fn; - break; - case 'confirm': - simulateConfirm = fn; - break; - case 'cancel': - simulateCancel = fn; - break; - case 'shippingaddresschange': - simulateShippingAddressChange = fn; - break; - case 'shippingratechange': - simulateShippingRateChange = fn; - break; - default: - throw new Error('TestSetupError: Unexpected event registration.'); - } + + simulateElementsEvents = {}; + simulateOn = jest.fn((event, fn) => { + simulateElementsEvents[event] = [ + ...(simulateElementsEvents[event] || []), + fn, + ]; }); + simulateOff = jest.fn((event, fn) => { + simulateElementsEvents[event] = simulateElementsEvents[event].filter( + (previouslyAddedFn) => previouslyAddedFn !== fn + ); + }); + + mockElement.on = simulateOn; + mockElement.off = simulateOff; + mockCartElementContext = mocks.mockCartElementContext(); jest .spyOn(ElementsModule, 'useCartElementContextWithUseCase') @@ -246,6 +204,9 @@ describe('createElementComponent', () => { ); expect(mockElements.create).toHaveBeenCalledWith('card', options); + + expect(simulateOn).not.toBeCalled(); + expect(simulateOff).not.toBeCalled(); }); it('mounts the element', () => { @@ -257,6 +218,9 @@ describe('createElementComponent', () => { expect(mockElement.mount).toHaveBeenCalledWith(container.firstChild); expect(React.useLayoutEffect).toHaveBeenCalled(); + + expect(simulateOn).not.toBeCalled(); + expect(simulateOff).not.toBeCalled(); }); it('does not create and mount until Elements has been instantiated', () => { @@ -289,6 +253,28 @@ describe('createElementComponent', () => { ); }); + it('removes old event handlers when an event handler changes', () => { + const mockHandler = jest.fn(); + const mockHandler2 = jest.fn(); + const {rerender} = render( + + + + ); + + expect(simulateOn).toBeCalledWith('change', mockHandler); + expect(simulateOff).not.toBeCalled(); + + rerender( + + + + ); + + expect(simulateOff).toBeCalledWith('change', mockHandler); + expect(simulateOn).toHaveBeenLastCalledWith('change', mockHandler2); + }); + it('propagates the Element`s ready event to the current onReady prop', () => { const mockHandler = jest.fn(); const mockHandler2 = jest.fn(); @@ -303,11 +289,32 @@ describe('createElementComponent', () => { ); - simulateReady(); + const mockEvent = Symbol('ready'); + simulateEvent('ready', mockEvent); expect(mockHandler2).toHaveBeenCalledWith(mockElement); expect(mockHandler).not.toHaveBeenCalled(); }); + it('propagates the Pay Button Element`s ready event to the current onReady prop', () => { + const mockHandler = jest.fn(); + const mockHandler2 = jest.fn(); + const {rerender} = render( + + {}} /> + + ); + rerender( + + {}} /> + + ); + + const mockEvent = Symbol('ready'); + simulateEvent('ready', mockEvent); + expect(mockHandler2).toHaveBeenCalledWith(mockEvent); + expect(mockHandler).not.toHaveBeenCalled(); + }); + it('sets cart in the CartElementContext', () => { expect(mockCartElementContext.cart).toBe(null); @@ -337,8 +344,58 @@ describe('createElementComponent', () => { }, }; - simulateReady(readyEvent); + simulateEvent('ready', readyEvent); + expect(mockCartElementContext.cartState).toBe(readyEvent); + + const changeEvent = { + elementType: 'cart', + id: 'cart_session_id_change', + lineItems: { + count: 1, + }, + }; + simulateEvent('change', changeEvent); + expect(mockCartElementContext.cartState).toBe(changeEvent); + + const checkoutEvent = { + elementType: 'cart', + id: 'cart_session_id_checkout', + lineItems: { + count: 2, + }, + }; + simulateEvent('checkout', checkoutEvent); + expect(mockCartElementContext.cartState).toBe(checkoutEvent); + }); + + it('sets cartState in the CartElementContext when passing in callbacks', () => { + const onReady = jest.fn(); + const onChange = jest.fn(); + const onCheckout = jest.fn(); + + render( + + + + ); + + expect(mockCartElementContext.cartState).toBe(null); + + const readyEvent = { + elementType: 'cart', + id: 'cart_session_id_ready', + lineItems: { + count: 0, + }, + }; + + simulateEvent('ready', readyEvent); expect(mockCartElementContext.cartState).toBe(readyEvent); + expect(onReady).toBeCalledWith(readyEvent); const changeEvent = { elementType: 'cart', @@ -347,8 +404,9 @@ describe('createElementComponent', () => { count: 1, }, }; - simulateChange(changeEvent); + simulateEvent('change', changeEvent); expect(mockCartElementContext.cartState).toBe(changeEvent); + expect(onChange).toBeCalledWith(changeEvent); const checkoutEvent = { elementType: 'cart', @@ -357,8 +415,9 @@ describe('createElementComponent', () => { count: 2, }, }; - simulateCheckout(checkoutEvent); + simulateEvent('checkout', checkoutEvent); expect(mockCartElementContext.cartState).toBe(checkoutEvent); + expect(onCheckout).toBeCalledWith(checkoutEvent); }); it('propagates the Element`s change event to the current onChange prop', () => { @@ -376,7 +435,7 @@ describe('createElementComponent', () => { ); const changeEventMock = Symbol('change'); - simulateChange(changeEventMock); + simulateEvent('change', changeEventMock); expect(mockHandler2).toHaveBeenCalledWith(changeEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -395,7 +454,7 @@ describe('createElementComponent', () => { ); - simulateBlur(); + simulateEvent('blur'); expect(mockHandler2).toHaveBeenCalledWith(); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -414,7 +473,7 @@ describe('createElementComponent', () => { ); - simulateFocus(); + simulateEvent('focus'); expect(mockHandler2).toHaveBeenCalledWith(); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -433,7 +492,7 @@ describe('createElementComponent', () => { ); - simulateEscape(); + simulateEvent('escape'); expect(mockHandler2).toHaveBeenCalledWith(); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -453,7 +512,7 @@ describe('createElementComponent', () => { ); const clickEventMock = Symbol('click'); - simulateClick(clickEventMock); + simulateEvent('click', clickEventMock); expect(mockHandler2).toHaveBeenCalledWith(clickEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -473,7 +532,7 @@ describe('createElementComponent', () => { ); const loadErrorEventMock = Symbol('loaderror'); - simulateLoadError(loadErrorEventMock); + simulateEvent('loaderror', loadErrorEventMock); expect(mockHandler2).toHaveBeenCalledWith(loadErrorEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -492,7 +551,7 @@ describe('createElementComponent', () => { ); - simulateLoaderStart(); + simulateEvent('loaderstart'); expect(mockHandler2).toHaveBeenCalledWith(); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -511,7 +570,7 @@ describe('createElementComponent', () => { ); - simulateNetworksChange(); + simulateEvent('networkschange'); expect(mockHandler2).toHaveBeenCalledWith(); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -531,7 +590,7 @@ describe('createElementComponent', () => { ); const checkoutEventMock = Symbol('checkout'); - simulateCheckout(checkoutEventMock); + simulateEvent('checkout', checkoutEventMock); expect(mockHandler2).toHaveBeenCalledWith(checkoutEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -551,7 +610,7 @@ describe('createElementComponent', () => { ); const lineItemClickEventMock = Symbol('lineitemclick'); - simulateLineItemClick(lineItemClickEventMock); + simulateEvent('lineitemclick', lineItemClickEventMock); expect(mockHandler2).toHaveBeenCalledWith(lineItemClickEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -571,7 +630,7 @@ describe('createElementComponent', () => { ); const confirmEventMock = Symbol('confirm'); - simulateConfirm(confirmEventMock); + simulateEvent('confirm', confirmEventMock); expect(mockHandler2).toHaveBeenCalledWith(confirmEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -591,7 +650,7 @@ describe('createElementComponent', () => { ); const cancelEventMock = Symbol('cancel'); - simulateCancel(cancelEventMock); + simulateEvent('cancel', cancelEventMock); expect(mockHandler2).toHaveBeenCalledWith(cancelEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -617,7 +676,7 @@ describe('createElementComponent', () => { ); const shippingAddressChangeEventMock = Symbol('shippingaddresschange'); - simulateShippingAddressChange(shippingAddressChangeEventMock); + simulateEvent('shippingaddresschange', shippingAddressChangeEventMock); expect(mockHandler2).toHaveBeenCalledWith(shippingAddressChangeEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); @@ -643,7 +702,7 @@ describe('createElementComponent', () => { ); const shippingRateChangeEventMock = Symbol('shippingratechange'); - simulateShippingRateChange(shippingRateChangeEventMock); + simulateEvent('shippingratechange', shippingRateChangeEventMock); expect(mockHandler2).toHaveBeenCalledWith(shippingRateChangeEventMock); expect(mockHandler).not.toHaveBeenCalled(); }); diff --git a/src/components/createElementComponent.tsx b/src/components/createElementComponent.tsx index abd4e8e..53132e8 100644 --- a/src/components/createElementComponent.tsx +++ b/src/components/createElementComponent.tsx @@ -10,7 +10,7 @@ import { useElementsContextWithUseCase, useCartElementContextWithUseCase, } from './Elements'; -import {useCallbackReference} from '../utils/useCallbackReference'; +import {useAttachEvent} from '../utils/useAttachEvent'; import {ElementProps} from '../types'; import {usePrevious} from '../utils/usePrevious'; import { @@ -41,8 +41,6 @@ interface PrivateElementProps { options?: UnknownOptions; } -const noop = () => {}; - const capitalized = (str: string) => str.charAt(0).toUpperCase() + str.slice(1); const createElementComponent = ( @@ -55,21 +53,21 @@ const createElementComponent = ( id, className, options = {}, - onBlur = noop, - onFocus = noop, - onReady = noop, - onChange = noop, - onEscape = noop, - onClick = noop, - onLoadError = noop, - onLoaderStart = noop, - onNetworksChange = noop, - onCheckout = noop, - onLineItemClick = noop, - onConfirm = noop, - onCancel = noop, - onShippingAddressChange = noop, - onShippingRateChange = noop, + onBlur, + onFocus, + onReady, + onChange, + onEscape, + onClick, + onLoadError, + onLoaderStart, + onNetworksChange, + onCheckout, + onLineItemClick, + onConfirm, + onCancel, + onShippingAddressChange, + onShippingRateChange, }) => { const {elements} = useElementsContextWithUseCase(`mounts <${displayName}>`); const elementRef = React.useRef(null); @@ -79,23 +77,67 @@ const createElementComponent = ( `mounts <${displayName}>` ); - const callOnReady = useCallbackReference(onReady); - const callOnBlur = useCallbackReference(onBlur); - const callOnFocus = useCallbackReference(onFocus); - const callOnClick = useCallbackReference(onClick); - const callOnChange = useCallbackReference(onChange); - const callOnEscape = useCallbackReference(onEscape); - const callOnLoadError = useCallbackReference(onLoadError); - const callOnLoaderStart = useCallbackReference(onLoaderStart); - const callOnNetworksChange = useCallbackReference(onNetworksChange); - const callOnCheckout = useCallbackReference(onCheckout); - const callOnLineItemClick = useCallbackReference(onLineItemClick); - const callOnConfirm = useCallbackReference(onConfirm); - const callOnCancel = useCallbackReference(onCancel); - const callOnShippingAddressChange = useCallbackReference( + // 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( + elementRef, + 'shippingaddresschange', onShippingAddressChange ); - const callOnShippingRateChange = useCallbackReference(onShippingRateChange); + useAttachEvent(elementRef, 'shippingratechange', onShippingRateChange); + + let readyCallback: UnknownCallback | undefined; + if (type === 'cart') { + readyCallback = (event) => { + setCartState( + (event as unknown) as stripeJs.StripeCartElementPayloadEvent + ); + onReady && onReady(event); + }; + } else if (onReady) { + if (type === 'payButton') { + // Passes through the event, which includes visible PM types + readyCallback = onReady; + } else { + // For other Elements, pass through the Element itself. + readyCallback = () => { + onReady(elementRef.current); + }; + } + } + + useAttachEvent(elementRef, 'ready', readyCallback); + + const changeCallback = + type === 'cart' + ? (event: stripeJs.StripeCartElementPayloadEvent) => { + setCartState(event); + onChange && onChange(event); + } + : onChange; + + useAttachEvent(elementRef, 'change', changeCallback); + + const checkoutCallback = + type === 'cart' + ? (event: stripeJs.StripeCartElementPayloadEvent) => { + setCartState(event); + onCheckout && onCheckout(event); + } + : onCheckout; + + useAttachEvent(elementRef, 'checkout', checkoutCallback); React.useLayoutEffect(() => { if (elementRef.current == null && elements && domNode.current != null) { @@ -107,112 +149,6 @@ const createElementComponent = ( } elementRef.current = element; element.mount(domNode.current); - element.on('ready', (event) => { - if (type === 'cart') { - // we know that elements.on event must be of type StripeCartPayloadEvent if type is 'cart' - // we need to cast because typescript is not able to infer which overloaded method is used based off param type - if (setCartState) { - setCartState( - (event as unknown) as stripeJs.StripeCartElementPayloadEvent - ); - } - // the cart ready event returns a CartStatePayload instead of the CartElement - callOnReady(event); - } else if (type === 'payButton') { - callOnReady(event); - } else { - callOnReady(element); - } - }); - - element.on('change', (event) => { - if (type === 'cart' && setCartState) { - // we know that elements.on event must be of type StripeCartPayloadEvent if type is 'cart' - // we need to cast because typescript is not able to infer which overloaded method is used based off param type - setCartState( - (event as unknown) as stripeJs.StripeCartElementPayloadEvent - ); - } - callOnChange(event); - }); - - // Users can pass an onBlur prop on any Element component - // just as they could listen for the `blur` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('blur', callOnBlur); - - // Users can pass an onFocus prop on any Element component - // just as they could listen for the `focus` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('focus', callOnFocus); - - // Users can pass an onEscape prop on any Element component - // just as they could listen for the `escape` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('escape', callOnEscape); - - // Users can pass an onLoadError prop on any Element component - // just as they could listen for the `loaderror` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('loaderror', callOnLoadError); - - // Users can pass an onLoaderStart prop on any Element component - // just as they could listen for the `loaderstart` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('loaderstart', callOnLoaderStart); - - // Users can pass an onNetworksChange prop on any Element component - // just as they could listen for the `networkschange` event on any Element, - // but only the Card and CardNumber Elements will trigger the event. - (element as any).on('networkschange', callOnNetworksChange); - - // Users can pass an onClick prop on any Element component - // just as they could listen for the `click` event on any Element, - // but only the PaymentRequestButton will actually trigger the event. - (element as any).on('click', callOnClick); - - // Users can pass an onCheckout prop on any Element component - // just as they could listen for the `checkout` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on( - 'checkout', - (event: stripeJs.StripeCartElementPayloadEvent) => { - if (type === 'cart' && setCartState) { - // we know that elements.on event must be of type StripeCartPayloadEvent if type is 'cart' - // we need to cast because typescript is not able to infer which overloaded method is used based off param type - setCartState(event); - } - callOnCheckout(event); - } - ); - - // Users can pass an onLineItemClick prop on any Element component - // just as they could listen for the `lineitemclick` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('lineitemclick', callOnLineItemClick); - - // Users can pass an onConfirm prop on any Element component - // just as they could listen for the `confirm` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('confirm', callOnConfirm); - - // Users can pass an onCancel prop on any Element component - // just as they could listen for the `cancel` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('cancel', callOnCancel); - - // Users can pass an onShippingAddressChange prop on any Element component - // just as they could listen for the `shippingaddresschange` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on( - 'shippingaddresschange', - callOnShippingAddressChange - ); - - // Users can pass an onShippingRateChange prop on any Element component - // just as they could listen for the `shippingratechange` event on any Element, - // but only certain Elements will trigger the event. - (element as any).on('shippingratechange', callOnShippingRateChange); } }); diff --git a/src/utils/useAttachEvent.ts b/src/utils/useAttachEvent.ts new file mode 100644 index 0000000..acbd8b4 --- /dev/null +++ b/src/utils/useAttachEvent.ts @@ -0,0 +1,20 @@ +import React from 'react'; +import * as stripeJs from '@stripe/stripe-js'; + +export const useAttachEvent = ( + elementRef: React.MutableRefObject, + event: string, + cb?: (...args: A) => any +) => { + React.useEffect(() => { + if (!cb || !elementRef.current) { + return () => {}; + } + + const element = elementRef.current; + + (element as any).on(event, cb); + + return () => (element as any).off(event, cb); + }, [cb, event, elementRef]); +}; diff --git a/src/utils/useCallbackReference.ts b/src/utils/useCallbackReference.ts deleted file mode 100644 index 625b2c5..0000000 --- a/src/utils/useCallbackReference.ts +++ /dev/null @@ -1,17 +0,0 @@ -import React from 'react'; - -export const useCallbackReference = ( - cb?: (...args: A) => any -): ((...args: A) => void) => { - const ref = React.useRef(cb); - - React.useEffect(() => { - ref.current = cb; - }, [cb]); - - return (...args): void => { - if (ref.current) { - ref.current(...args); - } - }; -};