stripe / react-stripe-js

React components for Stripe.js and Stripe Elements
https://stripe.com/docs/stripe-js/react
MIT License
1.76k stars 270 forks source link

[BUG]: Elements component can create multiple instances of stripeJs.StripeElements #296

Closed randypuro closed 2 years ago

randypuro commented 2 years ago

What happened?

Recent changes to this library have changed when and how often the useEffect which creates the StripeJs.StripeElements instance runs.

Because of the addition of options to the dependency list, it reruns on every render for those who have configured the Elements component with a non-memoized set of options. This a a reasonable assumption since:

  1. The examples in the docs do it this way
  2. The docs state "Once the stripe prop has been set, these options can’t be changed." implying they are basically read once.

If we pass a promise to the Elements stripe attribute, and there are multiple renders prior to that promise resolving, the effect will run multiple times and attached multiple resolvers to the promise and therefore create multiple elements instances.

In the typical flow, the first resolution will create any child PaymentElements attached to the first elements instance. Subsequent useElements in child instances will get the newer elements instances after that.

When trying to checkout (confirmPayment with useElements), the checkout fails and we then get the error "Invalid value for stripe.confirmPayment(): elements should have a mounted Payment Element."

For others facing this issue, one of many ways to work around it is to wrap the options argument in a useMemo, or to pass a Stripe instance to Elements component rather than a Stripe promise.

Environment

No response

Reproduction

No response

bmathews-stripe commented 2 years ago

Thanks for the report and the helpful investigation @randypuro! We'll get this fixed.

pascuflow commented 2 years ago

Dropping back to "@stripe/react-stripe-js": "^1.7.1" until this is fixed.

bmathews-stripe commented 2 years ago

@randypuro I'm having trouble reproducing this! Any more insight into how you're running into this issue? Here's what I tried: https://codesandbox.io/s/react-stripe-js-forked-vvddpn?file=/src/index.js based on the scenario you described.

Internally, when the options prop changes, the effect's cleanup is ran which sets a local variable isMounted (misleading name) to false. When the stripe promise resolves, isMounted is used to determine if the context should be updated.

In your scenario, that means:

1) Elements instance initially rendered with promise provided as stripe prop. 2) The effect is ran (effect instance 1), isMounted initializes to true, and the promise is awaited 3) The parent is re-rendered, options change and the effect's cleanup function is invoked which sets isMounted to false. 4) The effect is ran again (effect instance 2), isMounted initializes to true, and the promise is awaited 5) Effect instance 1's promise resolves, but isMounted is false, so it bails 5) Effect instance 2's promise resolves, isMounted is true, so it sets the stripe/elements instance on context.

mnpenner commented 2 years ago

FWIW I hit this a couple days ago too. Details here: https://stackoverflow.com/q/72334261/65387

randypuro commented 2 years ago

HI @bmathews-stripe . Yes, hard for me to effectively repro using your codesandbox as well. Definitely seems like a race condition. FWIW, our integration specs were consistently flaky at a 50% rate until we memoized our options to Elements. I wonder if the amount of work done in the real world app is sufficiently large to cause react to breakup operations more, or if there's something else subtle with React 18's concurrent renderer.

Thanks for your explanation of the logic - it makes sense, but my guess is that there are windows where both promise callbacks can run (or rather, the first can run while the second is being created at which point the second will run as well on immediate promise resolution). For instance, it's my understand the useEffect callbacks don't necessarily run synchronously with the render - there can be a timeout between where the old callback can still fire before the new callback is created.

The other issue is that the gate to prevent re-creation of the context runs in one frame, whereas the actual creation runs in another. One quick solution could be to use the functional argument version of setContext to accurately check the prior value at the time of the promise callback.

Anyway, my apologies I can't put something together that's more definitive, but I do think it's worth thinking about how the functionality could be broken up to a single series of gates that are more guaranteed to run linearly (eg. multiple effects to chain the operations with specific dependencies for each step).

I hope that helps.

bmathews-stripe commented 2 years ago

Thanks @randypuro! Super helpful to know that it was intermittent in your test environment.

Two last questions:

1) Which version of react were you using? 2) Were you using StrictMode?

randypuro commented 2 years ago

React 18.1, not using StrictMode.

bmathews-stripe commented 2 years ago

@randypuro and @mnpenner: We've released a (likely) fix in v1.8.1. Thanks!

randypuro commented 2 years ago

@bmathews-stripe , thank you! Given our consistently flaky integration specs, I was able to confirm the fix has addressed the issue.

Small thought to do with as you please. Should you run into issues again in the future, or want to simplify (eliminate the local closure gate), I think it might make sense to split this effect in 2. One to set a new state variable with the resolved stripe instance from the passed in stripe option (or parsed in this case), and the second to create the context based on the resolved stripe and varying options. This would simplify the dependencies so that the first only depends on a single unchanging arg (so only one event listener added to the promise), and the second gates running exclusively through the state (whether stripe exists and context not yet created).

Anyway, regardless, thank you for the fix!

bmathews-stripe commented 2 years ago

@randypuro Nice, thanks for testing that out and confirming.

That's a good idea. Thanks for the suggestion!

martin3walker commented 1 year ago

Hello,

I'm running into this same error: IntegrationError: Invalid value for stripe.confirmPayment(): elements should have a mounted Payment Element or Pay Button Element. I'm using version 1.16.0 of @stripe/react-stripe-js.

Following the workaround advice above, I'm passing in a resolved instance of the stripe.js SDK into the stripe prop of the Elements provider. I'm also memoizing the options object where I pass in the client secret.

const PaymentDetails = ({
  formState,
  setIsLoading,
  setCurrentFormStep,
  stripe,
}: FormStepProps) => {
  const options = useMemo(() => {
    const { clientSecret } = formState;
    return {
      clientSecret,
    };
  }, [formState]);

  return (
    <Elements stripe={stripe} options={options}>
        <StripePaymentForm />
    </Elements>
 )

In the <StripePaymentForm /> component I'm calling the following click handler to submit the data from a <PaymentElement /> :

const clickHandler = async (event: any) => {
    event.preventDefault();
    setIsLoading(true);

    if (!stripe || !elements) {
      return;
    }

    try {
      await stripe.confirmPayment({
        elements,
        confirmParams: {
          return_url: window.location.href,
        },
        redirect: 'if_required',
      });

    } catch (error) {
      console.log(error);
    }
  };

The error is being thrown at stripe.confirmPayment. The <PaymentElement /> is rendered, so I'm confused as to why it says it isn't mounted.

Do we think this is the same bug? Perhaps it is a mistake on my end as I'm relatively new to Stripe 🤷

martin3walker commented 1 year ago

I've added the following to my click handler just to make sure the <PaymentElement /> really is mounted.

const paymentElement = elements?.getElement('payment');
paymentElement?.mount('#payment');

Those two lines throw the following error: IntegrationError: This Element is already mounted. Useunmount()to unmount the Element before re-mounting.

martin3walker commented 1 year ago

@martinalong-stripe @tylersmith-stripe Do you think this is worth reopening?

martin3walker commented 1 year ago

My problem ended up being unrelated to this. I set a loading state in the click handler that was replacing the PaymentElement with a loading component. This was messing with Stripe's flow. Just FYI in case anyone else runs into a similar issue.

oscyp commented 1 year ago

We had a same issue as @martin3walker.

Code snippet which was breaking elements component:

if (isLoading) {          <---------------- removing this if statement solves the problem
 return <Spinner />
 }

 return (
 ...
 <PaymentElement />
 ...
 )
mario-garcia commented 1 year ago

I am very much still seeing this exact issue in 1.16.5. Awaiting the loadStripe() promise didn't work. Wrapping the options in useMemo() also didn't work. Downgrading to 1.7.1 didn't work. And I don't really understand the "get rid of the loader/spinner" suggestion, because if i do that, i end up trying to create an Elements component with an invalid clientSecret In order to create a payment element, you must pass a valid PaymentIntent or SetupIntent client secret when creating the Elements group.. The client secret here is undefined because it hasn't loaded from my backend api that creates the payment intent yet (that's why the loader/spinner is there in the first place). Any other suggestions? If not it, it looks like I'm hosed.

bmathews-stripe commented 1 year ago

@mario-garcia:

And I don't really understand the "get rid of the loader/spinner" suggestion

The PaymentElement has to be mounted and on the page when calling stripe.confirmPayment. The folks above were replacing the PaymentElement with a load spinner in their pay/click handler, which was causing the IntegrationError: Invalid value for stripe.confirmPayment(): elements should have a mounted Payment Element or Pay Button Element. error.

Let me know if that clarification helps.

mario-garcia commented 1 year ago

aaaaahhh. thanks @bmathews-stripe. that clarifies it and indeed was my issue. thanks again.

SawkaDev commented 1 year ago

interesting that fixed the error on my end. If you want the loader, just ensure to put it inside where you are returning the payment form or card element

 return (
<>
  {loading && <Spinner />}
 <PaymentElement />
 </>
 )

I can add that this was not an issue when testing on Firefox, but I believe this is an issue in Safari, Edge, and Chrome that fixed me issue. Thanks!

k1sul1 commented 1 year ago

I had the same issue.

Turns out I had accidentally used <Elements></Elements> twice while adapting the official example to my needs.

Did not have to useMemo or the like.

lexy8168 commented 1 year ago

still stuck here any help will appreciate https://stackoverflow.com/questions/77214324

this is how my checkout looks like ` useEffect(() => { //create a payment intent as soon as the page loads fetch("http://localhost:4242/create-payment-intent", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ items: cartItems, userEmail: customerEmail, shipping: shippingAddress, billing: billingAddress, description, }), }) .then((res) => { if (res.ok) { return res.json(); } return res.json().then((json) => Promise.reject(json)); }) .then((data) => { setClientSecret(data.clientSecret); }) .catch((error) => { setMessage("Failled to initialize checkout"); toast.error(error.message); //rem this error may be very descriptive next time show defined error to the user });

}, []);

const appearance = {
    theme: "stripe",
};

const stripe = useMemo(() => {
    return stripePromise;
},[]);

const options = useMemo(() => {
    return {
        appearance,
        clientSecret
    }
},[appearance,clientSecret])

return ( 
    <div>
        <section>
            <div className="container">{!clientSecret && <h3>{ message}</h3>}</div>
        </section>
        {clientSecret && (
            <Elements options={options} stripe={stripe}>
            <CheckoutForm  />
            </Elements>
        )}
    </div>
) 

`

this is my checkout form

` useEffect(() => { if (!stripe) { return; }

    const clientSecret = new URLSearchParams(window.location.search).get(
        "payment_intent_client_secret"
    );

    if (!clientSecret) {
        return;
    }

const handleSubmit = async (e) => {
    e.preventDefault();
    setMessage(null);

    if (!stripe || !elements) {
        return;
    }
    const confirmPayment = await stripe.confirmPayment({
        elements,
        confirmParams: {
            return_url: "http::/localhost:4243/checkout-success",
        },
        redirect: "if_required",
    })
        .then((result) => {
            //ok payment intent or it could be abad error 
            if (result.error) {
                toast.error(result.error.message);
                //maybe the customer has entered wrong account details 
                setMessage(result.error.message);
                return;
            }
            console.log(result);        
            if (result.paymentIntent) {
                if (result.paymentIntent.status === "succeeded") {
                    setIsLoading(false);
                    //customer will be directed to the return url you provided or redirected if it needs inter mediate
                    toast.success("payment successful");
                    saveOrder();
                }
            }
        });
    setIsLoading(false);

};

return (
    <section>
  <div className={`container ${styles.checkout}`}>
    <h2>Checkout</h2>
    <form onSubmit={handleSubmit}>
      <div>
        <Card cardClass={styles.card}>
          <CheckoutSummary />
        </Card>
      </div>
      <div>
        <Card cardClass={`${styles.card} ${styles.pay}`}>
          <h3>Stripe Checkout</h3>
          <PaymentElement id={styles["payment-element"]} />
          <button
            disabled={isLoading || !stripe || !elements}
            id="submit"
            className={styles.button}
          >
            <span id="button-text">
              {isLoading ? (
                <img
                  src={spinnerImg}
                  alt="Loading..."
                  style={{ width: "20px" }}
                />
              ) : (
                "Pay now"
              )}
            </span>
          </button>
          {/* Show any error or success messages */}
          {message && <div id={styles["payment-message"]}>{message}</div>}
        </Card>
      </div>
    </form>
  </div>
</section>

`

Ayeobasan commented 11 months ago

hi everyone please i have being facing same issue for like 2 weeks not still no solution what im i doing wrong i have try all possible solution but still did'nt work

` useEffect(() => { // Extract transaction ID and client secret from the URL const urlParams = new URLSearchParams(window.location.search); const transactionId = location?.pathname.split("/")[5]; const clientSecretUrl = urlParams.get("client_secret");

if (transactionId && clientSecretUrl) {
  setTransactionId(transactionId);
  setClientSecret(clientSecretUrl);

  // Fetch fund payment details using the transaction ID
  fetchPaymentDetails(transactionId);

  // Initialize Stripe Elements
  // initializeStripeElements();
}

}, []);`

` const handlePayment = async (e) => { e.preventDefault(); setMessage(null);

if (!stripe || !elements) {
  return <p>Loadinging Payment Method</p>;
}

setPaymentLoading(true);
const paymentElement = elements?.getElement("payment");
paymentElement?.mount("#payment");

try {
  const { error, paymentIntent } = await stripe.confirmPayment({
    elements,
    redirect: "if_required",

    confirmParams: {
      return_url: "https://localhost:5173/success",
    },
  });

  if (error) {
    toast.error(error.message);
    setMessage(error.message);
    setPaymentLoading(false);
    return;
  }

  if (paymentIntent && paymentIntent.status === "succeeded") {
    toast.success("Payment successful");
    // Additional logic for a successful payment
    // e.g., saveOrder();
  }

} catch (error) {
  console.error("Payment error:", error);
  toast.error(error.message);
}

stripe.retrievePaymentIntent(clientSecret).then(({ paymentIntent }) => {
  switch (paymentIntent.status) {
    case "succeeded":
      setMessage("Payment succeeded!");
      break;
    case "processing":
      setMessage("Your payment is processing.");
      break;
    case "requires_payment_method":
      setMessage("Your payment was not successful, please try again.");
      break;
    default:
      setMessage("Something went wrong.");
      break;
  }
});
setPaymentLoading(false);

};`

const appearance = {
    theme: "stripe",
  };
  const elementsOptions = useMemo(() => {
    return {
      stripe: stripePromise,
      options: {
        clientSecret:
        clientSecret,
        appearance,
      },
    };
  }, [stripePromise, clientSecret, appearance]);

  return (
    <>
      <Elements
        {...elementsOptions}
      >    
                  <div>
                      <PaymentElement id="payment-element" />

                      <button
                        className="text-white bg-[#1C4EBF] w-full h-12"
                        disabled={paymentLoading || !stripe || !elements}
                        id="submit"
                      >
                        <span id="button-text">
                          {paymentLoading ? "Processing ... " : "Pay now"}
                        </span>
                      </button>
                      {/* Show any error or success messages */}
                      {message && <div id="payment-message">{message}</div>}
                </div>
      </Elements>
anshumangogate commented 6 months ago

Hi, I'm also facing the same issue. I'm using stripe version 2.3.1 The moment I click on submit, my Payment Element gets unmounted & remounted on the screen and I get this error. I have tried all possible solutions, nothing works.

I figured out when I'm setting state in submit form, this problem is happening. Can anyone help me what could be issue?