Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: LinkAuthenticationElement no longer fires onChange event when it is mounted. #365

Closed
alexdiv87 opened this issue Dec 22, 2022 · 10 comments
Labels
bug Something isn't working

Comments

@alexdiv87
Copy link

alexdiv87 commented Dec 22, 2022

What happened?

Hi There, I am upgrading from version 1.7.0 to the latest 1.16.1. We are noticing a difference in behavior with the LinkAuthenticationElement. Previously, when this component was mounted, the onChange event would consistently fire. We used this event to rely on Stripe to perform the email validation. After upgrading, we are seeing that the onChange event doesn't always fire when the component is mounted, it seems intermittent. Sometimes it fires, and other times it does not. I'll include a code snippet, any help would be appreciated.

export const StripeLinkAuthenticationElement = (): React.ReactElement => {
  const [email, setEmail] = useLocalStorage<string>(
    EnumStorageKeys.CUSTOMER_EMAIL,
    '',
  )

  const {
    paymentIntentId,
    orderSessionId,
    setIsLinkAuthenticatorReady,
    setIsLinkAuthenticatorCompleted,
  } = useContext(StripeComponentsContext)

  const video = useStore((state) => state.video)
  const checkoutItem = useStore(selectedCheckoutItemSelector)

  return (
    <LinkAuthenticationElement
      options={{ defaultValues: { email: email ? email : '' } }}
      onChange={(e) => {
        if (setIsLinkAuthenticatorCompleted) {
          setIsLinkAuthenticatorCompleted(e.complete)
          setEmail(e.value.email)
        }
      }}
      onReady={() => {
        if (setIsLinkAuthenticatorReady) {
          setIsLinkAuthenticatorReady(true)
        }
      }}
    />
  )
}

As a result, when Stripe Link is enabled, the customer may open the checkout form and have all of their information pre-populated, but we since the onChange event doesn't fire, their email address does not get validated and the button remains disabled. You can reproduce this by visiting this URL: https://fw-staging.tv/embed-url.html?video=oAwwE0&channel=alexdiv&max_videos=1 . Choose any non $0 product and click "Buy Now", Login using Stripe Link, close the popup and reopen. it will open a checkout form where sometimes the button is enabled (onChange event fired). And other times, the button remains disabled when onChange never fired.

What dictates whether onChange fires when the component mounts? Do you have any tools for manually validating email addresses, if we need to manually validate the email address in the onReady event, we want to ensure we're using the same validation that is used in the onChange event.

Environment

Chrome, MacOS

Reproduction

https://fw-staging.tv/embed-url.html?video=oAwwE0&channel=alexdiv&max_videos=1

@alexdiv87 alexdiv87 added the bug Something isn't working label Dec 22, 2022
@awalker-stripe
Copy link
Contributor

Hi @alexdiv87, I'm sorry you're experiencing a bug since updating the version. Thank you for providing all of these details and the reproduction!

To clarify some of your questions, the intended behavior is that onChange fires whenever Link is autofilled, and the complete field is your best bet for this sort of validation.

For the bug, I was able to reproduce the issue when I visited the link you provided, but unfortunately didn't have much luck reproducing the issue on my own test integrations. However, we recently made a change between 1.16.0 to 1.16.1 that alters some of our logic for how we add event listeners, and I'd love to nail down whether that's causing the issue you're experiencing here.

Could you try installing version 1.16.0, and let me know if the issue persists?

For one final thing to try: could you add a log outside of the if statement, and see whether it logs when the issue occurs? For example:

onChange={(e) => {
        console.log("LinkAuthenticationElement change!", !!setIsLinkAuthenticatorCompleted, e);
        if (setIsLinkAuthenticatorCompleted) {
          setIsLinkAuthenticatorCompleted(e.complete)
          setEmail(e.value.email)
        }
      }}

@alexdiv87
Copy link
Author

alexdiv87 commented Dec 23, 2022

Hi @awalker-stripe , Thanks for getting back to me so swiftly! I did what you said and tested out version 1.16.0 and it seems to work fine. I see the onChange event firing consistently (although, sometimes it fires twice and others times only once, but this is not a big deal for us). The problem seems to be with 1.16.1. I'll attach some videos showing the behavior of both versions:

v1.16.0:
https://vimeo.com/784017234

v1.16.1:
https://vimeo.com/784017466

In regards to your integration testing of 1.16.1, perhaps some more info could help. I thought I should mention that when we render the Stripe Components, we are doing so using React's createPortal method like so:

export const RenderStripeComponents: FC<Props> = ({ hidden }) => {
  const stripeContainerEl = useRef<HTMLSlotElement | null>(null)
  const { clientSecret } = useContext(StripeComponentsContext)
  const slot = useRenderStripeElementsContainer({
    stripeContainerEl,
    clientSecret,
  })

  return (
    <>
      <slot name={SLOT_NAME} ref={stripeContainerEl}></slot>
      {slot
        ? createPortal(
            <StripeContainer>
              <FormSection type={FormTypes.AUTHENTICATOR} hidden={hidden} />
              <FormSection type={FormTypes.SHIPPING} hidden={hidden} />
              <FormSection type={FormTypes.PAYMENT} hidden={hidden} />
            </StripeContainer>,
            slot,
          )
        : null}
    </>
  )
}

export const StripeContainer = ({
  children,
}: {
  children: React.ReactNode
}): React.ReactElement => {
  const { products } = useProducts()
  const accountId = products
    ? (products[0].payment_providers || []).find((p) => p.provider === 'stripe')
        ?.uid
    : ''
  const { clientSecret } = useContext(StripeComponentsContext)

  const header: {
    betas: string[]
    apiVersion: string
    stripeAccount?: string
  } = useMemo(() => {
    return {
      betas: ['link_beta_2'],
      apiVersion: '2020-08-27;link_beta=v1',
      stripeAccount: accountId,
    }
  }, [accountId])

  const stripe = useMemo(
    () => loadStripe(defaultStripePublishableKey, header),
    [header],
  )

  return (
    <Elements
      stripe={stripe}
      options={{
        clientSecret,
        appearance,
      }}
    >
      {children}
    </Elements>
  )
}

export const useRenderStripeElementsContainer = ({
  stripeContainerEl,
  clientSecret,
}: Props): HTMLElement | undefined => {
  const [slot, setSlot] = useState<HTMLElement | undefined>(undefined)

  const renderStripeElements = useCallback(() => {
    if (stripeContainerEl && stripeContainerEl.current) {
      const storyblock = stripeContainerEl.current.getRootNode().host
      const span = document.createElement('span')
      span.slot = SLOT_NAME
      span.id = STRIPE_CONTAINER_ID
      storyblock?.appendChild(span)
      setSlot(span)
    }
  }, [stripeContainerEl])

  useEffect(() => {
    if (clientSecret) {
      renderStripeElements()
    }
  }, [clientSecret, renderStripeElements])

  return slot
}

@vasiliicuhar
Copy link

Relevant for PaymentElement as well. Downgrading to 1.16.0 fixes it.
It seems that effect in useAttachEvent fires to early, when elementRef.current is still null.

Environment

@stripe/react-stripe-js: 1.16.1
react: 18.1.0
react-dom: 18.1.0

@awalker-stripe
Copy link
Contributor

Hi @alexdiv87 and @vasiliicuhar,

Happy New Year, and thank you for your patience on this. v1.16.2 reverts the change in v1.16.1 & should fix the issue, but please feel free to re-open if you have any further questions or if the issue persists.

Thanks!

@alexdiv87
Copy link
Author

Awesome! Thanks @awalker-stripe Happy New Year to you too!

@vasiliicuhar
Copy link

@awalker-stripe do you plan to iterate on it later?
Seems like a good optimisation. useAttachEvent hook implementation is the issue.

useEffect is tricky to get right. But one thing for sure, element has to be a real dependency, not a ref, for it to work

useEffect(() => {
  element.on(/* ... */)
  return element.off(/* ... */)
}, [/* important >> */ element])

@alexdiv87
Copy link
Author

alexdiv87 commented Jan 4, 2023

Hey @awalker-stripe , I wonder if you could help me with another issue or I need to file a ticket somewhere else? It's more of a question..

When we upgrade to 1.16.0 or 1.16.2, we are seeing a warning in the console which states:

react-stripe.umd.js?796c:290 Unsupported prop change: options.clientSecret is not a mutable property.

This occurs because we are not remounting the Stripe Form when a user clicks Back button. The flow is like so:

  1. Open Payment Form (Component Mounts)
  2. Create Setup Intent
  3. Confirm Setup Intent
  4. User clicks Back
  5. User sees Payment Form Again (It does not Remount), they cannot change their previous Setup Intent because it was already confirmed, so we have to create a new Setup Intent. This generates a new secret, which when we pass the new secret to <Elements />, we see this warning.

From what I can tell, everything seems to be working fine. Although, when it comes to Stripe Link in Staging Env, I can't see which Card they selected, so i'm not able to determine if the Customer is getting charged on the last card they selected, or if it used the first card that was associated with the client secret that was created in the first confirmed Setup Intent.

My question is:

What is the impact of this warning?

@awalker-stripe
Copy link
Contributor

@vasiliicuhar I appreciate it & totally agree with your comment on the dependencies — this makes sense to me. As a heads up, I do have reproductions of the original issue, and any changes would be tested against that to make sure we don't re-introduce it (thank you again for the in-depth description of your integrations!)

@alexdiv87 Similar thread: #246 (comment). In general, we don't support updating the client secret like this, and it could lead to problems down the line. However, there's a workaround in that thread that can get <Elements /> to remount. Outside of that, I'd probably recommend reaching out to the folks over at Stripe Support, as they're better equipped to work with you on your particular integration.

@alexdiv87
Copy link
Author

Thanks again for all your help. @awalker-stripe

@NestorRV
Copy link

@awalker-stripe If i try the workaround you recommended (adding key={clientSecret}), I get the following exception now:

Uncaught (in promise) Error: Frame not initialized.
    at e.canMakePayment (v3:1:109698)
    at r._canMakePaymentForBackingLibrary (v3:1:137328)
    at v3:1:139261
    at v3:1:60842
    at async Promise.all (index 0)

image

You mentioned that you do not support support this way of updating the client secret but it's what is documented in your docs: https://stripe.com/docs/payments/quickstart#use-webhook. Any thoughts for a real fix or a fix for the exception? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants