joshnuss / svelte-stripe

Everything you need to add Stripe Elements to your Svelte project
https://sveltestripe.com
MIT License
431 stars 42 forks source link

Bug when mounting/unmounting Payment Elements #122

Open frederichoule opened 2 months ago

frederichoule commented 2 months ago

Describe the bug

When creating, destroying and recreating the Payment Element, we get an error related to getContext() in Payment Elements.

[HMR][Svelte] Unrecoverable HMR error in <LinkAuthenticationElement>: next update will trigger a full reload
logError @ proxy.js?v=1f1e0bb9:15
Show 1 more frame
Show lessUnderstand this error
LinkAuthenticationElement.svelte:17 Uncaught (in promise) TypeError: Cannot destructure property 'elements' of 'getContext(...)' as it is undefined.
    at instance (LinkAuthenticationElement.svelte:17:11)
    at init (chunk-S2NM74UB.js?v=1f1e0bb9:2190:23)
    at new LinkAuthenticationElement (LinkAuthenticationElement.svelte:46:24)
    at createProxiedComponent (svelte-hooks.js?v=1f1e0bb9:341:9)
    at new ProxyComponent (proxy.js?v=1f1e0bb9:242:7)
    at new Proxy<LinkAuthenticationElement> (proxy.js?v=1f1e0bb9:349:11)
    at Array.create_default_slot (+page.svelte:105:11)
    at create_slot (chunk-S2NM74UB.js?v=1f1e0bb9:101:25)
    at create_if_block (Elements.svelte:3:45)
    at create_fragment (Elements.svelte:64:24)

Reproduction

Severity

blocking all usage

Additional Information

No response

frederichoule commented 2 months ago

By forcing Elements.svelte to re-set the context when the variables are updated, it seems to work.

$: setContext('stripe', { stripe, elements });

frederichoule commented 2 months ago

Tentative PR here: https://github.com/joshnuss/svelte-stripe/pull/123

frederichoule commented 2 months ago

Hey @joshnuss did you have any chance to look into this, or find a better fix? Thanks

joshnuss commented 2 months ago

Hi @frederichoule!

I'm wondering, what's different in your repo vs this one? Did you change anything?

Because when I try it with the live Svelte PaymentElement example it seems to work OK

frederichoule commented 2 months ago

Did you try my repo? There's a new button in the Svelte PaymentElement example page that says "Hide/Show" and replicates the bug.

The only change to the actual payment element is there: https://github.com/joshnuss/svelte-stripe/pull/123/commits/254b636b4fbd351b64e74819ae475d39c43095a0

Line 71: $: setContext('stripe', { stripe, elements });

frederichoule commented 2 months ago

Hey @joshnuss, the example wasn't in the repo, sorry it's my fault. It is now.

Example: https://github.com/frederichoule/svelte-stripe Fix: https://github.com/joshnuss/svelte-stripe/pull/123

frederichoule commented 1 month ago

Any update @joshnuss ?

TheRealThor commented 1 month ago

I am getting the same error when I use Elements in a multi-step form where the user advances to the payment element step, but goes back and then again to the step with the payment element, then an error is thrown and the app stops working:

Unhandled Promise Rejection: TypeError: Right side of assignment cannot be destructured

The error stems from calling getContext in an element that returns an undefined value.

I rectified the bug by simply adding to the Elements.svelte a onDestroy lifecycle function. When unmounting the Elements component, the elements objects and its contents gets garbage collected.

onDestroy(() => { if (elements) { elements = null; } });

frederichoule commented 4 days ago

Getting ready for production in the next few days, and no update from @joshnuss - so here's what I did to fix it without forking the whole library:

<script lang="ts">
    let stripeElements: StripeElements | null = $state(null);
    const svelteStripeFix = (e: HTMLDivElement) => {
        $effect(() => {
            return () => {
                stripeElements = null;
            };
        });
    };
</script>
<div use:svelteStripeFix>
  <Elements bind:elements={stripeElements}><!-- Elements here //--></Elements>
</div>

Basically, as soon as the div surrounding the Elements is destroyed, we just clear the binded reference to it and then everything works again the next time you mount it in the same payment flow.

Thanks @TheRealThor for pointing me in the right direction!

joshnuss commented 1 day ago

@frederichoule apologies for the delay. I will take a look this week.