Skip to content
This repository has been archived by the owner on Mar 27, 2021. It is now read-only.

API Review: handler for error when Stripe.js isn't loaded #249

Closed
asolove-stripe opened this issue Sep 14, 2018 · 1 comment
Closed

API Review: handler for error when Stripe.js isn't loaded #249

asolove-stripe opened this issue Sep 14, 2018 · 1 comment

Comments

@asolove-stripe
Copy link
Contributor

asolove-stripe commented Sep 14, 2018

This API change would resolve #237.

Motivation

When using the default integration, where an apiKey is passed in to StripeProvider, it currently throws an error if Stripe.js isn't already loaded. The error message correctly explains one possible cause of the problem: if the user failed to load Stripe.js or did so asynchronously, they need to change to using the async integration.

However, this error can also occur in production, in cases where the end-user's browser has odd network connectivity issues or blocks the loading of Stripe.js. In this case, it is inappropriate to throw an error from our component's constructor because this can be hard for React applications to catch and handle in an appropriate way. (ErrorBoundary makes this possible, but is slightly tricky to set up and not available in React 15, which this library supports.)

Proposal

The StripeProvider component should accept a new property, onStripeJsNotLoaded, which must be a function taking no arguments and with an ignored return value.

  • If the StripeProvider receives an apiKey, and therefore is not using an asynchronous/supplied-Stripe-instance integration, and if window.Stripe is not defined when we go to create a Stripe instance, then:
    • If onStripeJsNotLoaded is defined, it will get called when the StripeProvider is constructed. StripeProvider will then render its children, but not pass in a stripe instance, just as if we were in the asynchronous integration and it had not loaded yet.
    • if it is not defined, we will trigger the current error and add a note about the onStripeJsNotLoaded

Alternatives considered

  • We could do nothing and simply recommend that users check for window.Stripe being undefined themselves and handle this case before rendering StripeProvider, but this seems like a pit of failure.
@davidpatters0n
Copy link

Is there a reason why this was closed ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants