bendrucker/angularjs-stripe

Throw when Stripe.js is not available

Closed this issue · 10 comments

Also makes sense to make 'Stripe' a constant in the index and then just inject it elsewhere.

Per #7 (comment), this seems to be a common mistake

Also it would be nice to not have this break everything if Stripe.js is not available (happens in offline mode).

What does "offline mode" mean? I don't understand the use case you're referencing.

Sitting in the plane coding :-)

Ah, okay. So the reason I hadn't encountered this is because I'm still bundling stripe-debug.js directly. That results in a console.warn so I'm going to have to do the CDN route.

Not sure what I can do here though. Obviously everything's going to break down if you hit code that's making network calls to Stripe. The only thing that'll break up front is the stripeProvider.setPublishableKey method. Letting that fail silently would create opportunities for bugs to slip into live code.

There's two options here: one is to include a super light weight shim only in your dev builds that ensures window.Stripe is at least an empty object.

I've also considered writing a full mock Stripe.js implementation with an Angular-like API for mocking and flushing requests. If you think that's something you'd use I'll consider starting that soon.

Okay, got it. Actually it's not a problem, if the actual call to Stripe fails (since it is not available). But the application should at least be runnable. Maybe check if setPublishableKey is available?

That's the problem though. What's supposed to happen if it's not? In production, the application should fail to bootstrap. In development, you want it to fail silently. There's no having it both ways (and the first one is correct), at least from angular-stripe's perspective. You'd have to supply a mock, whether that's a simple window.Stripe = {} or something more sophisticated.

Okay I tried that:

<script>
  window.Stripe = {}
</script>
<script src="https://js.stripe.com/v2/"></script>

It gives another error now:

TypeError: Cannot read property 'validateCardNumber' of undefined

So it won't be quite that easy, you'll need this instead:

window.Stripe = {
  card: {},
  bankAccount: {}
}

Please be super careful with this. Also, stay tuned for https://github.com/bendrucker/mockstripe.

Yeaa, it feels like kind of a hack. It would be super cool, if angular stripe could be configured to use a Stripe mock (if that's what you have in mind). I could imagine sth. like this:

angular
  .module "myApp"
  .run (
    stripe
    myEnv
  ) ->
    stripe.setUseMock myEnv == "development"
    stripe.setPublishableKey ...

It's virtually never appropriate for libraries to provide an API for using a mock, though plenty of libs do it anyway. You either use the module system or provide a hook for overriding specific internals (like async scheduling).

The point of making a Stripe constant is that you could use dependency injection to provide it more easily:

$provide.constant('Stripe', mock)

When you choose to overwrite that value is up to you. It's a build concern and you should never be putting code that tries to do environment detection like myEnv == "development" in production code.