recurly / recurly-js

Simple subscription billing in the browser
https://js.recurly.com/
MIT License
645 stars 138 forks source link

ready() not called with PayPal DirectStrategy #664

Closed zorfling closed 3 years ago

zorfling commented 3 years ago

We're currently using Braintree for PayPal payments and are looking at swapping across to PayPal direct.

Our current code waits for the result of recurly.PayPal() to emit ready

In the BraintreeStrategy this works, and has been in production for some time

However when removing the braintree clientAuthorization to use the DirectStrategy, it doesn't seem to ever emit ready

Edit: Actually I can see in DirectStrategy it's emitting ready in the constructor - which is obviously prior to any listener being attached.

Is it fair to just assume PayPal direct is always ready?

Perhaps a doc update would help here. (actually PayPal ready isn't even documented...)

chrissrogers commented 3 years ago

Hi @zorfling -- yes, it's safe to assume PayPal direct is always ready for use once instantiated, you're spot on. So feel free to discard its use.

Your read on the strategies is spot on as well. The PayPal instance will not yet have bound its deferred listener by the time the DirectStrategy has emitted it. Functionally, this is okay as readiness is a factor only for loading of the Braintree dependency. The readiness sentinel itself is there primarily for the BraintreeStrategy, and I'd say is due for a bit of cleaning up. We haven't documented it as we primarily use it internally, though there is certainly a case for its use for Braintree+PayPal.. I'll have a ponder on this. Rest assured, though, that it won't be removed entirely.

Thanks for your very detailed report here! It's always massively appreciated, and do please keep us informed of any questions that come up.

zorfling commented 3 years ago

Thanks for the quick response @chrissrogers - glad I was on the right track, I've gone ahead with it as always ready and it works a treat. Cheers!