Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use official Stripe loader #25

Open
lindyhopchris opened this issue Jan 29, 2021 · 4 comments · May be fixed by #50
Open

Use official Stripe loader #25

lindyhopchris opened this issue Jan 29, 2021 · 4 comments · May be fixed by #50

Comments

@lindyhopchris
Copy link
Collaborator

Stripe now provide an official Stripe loader module:
https://github.com/stripe/stripe-js

We should use that to load Stripe.

@lindyhopchris
Copy link
Collaborator Author

@st-h just wanted to check your opinion on this before I implement...

Basically I think we should switch to using the official Stripe loader; always better to use something official imho!

The Stripe loader is an async function... therefore I think when we switch to it, we should change this package so that you always have to explicitly call the Stripe service's load method. That's so the developer is explicilty handling the async nature of loading Stripe, and where they want to do this within their application code.

Personally I think that's better than calling the async Stripe loader behind the scenes.

Do you think that's an acceptable change?

@st-h
Copy link
Contributor

st-h commented Feb 15, 2021

@lindyhopchris Personally, I think it would be fine to make the consuming app to explicitly load stripe when needed. We are already using it like that in our app through the config param. I think it's common for payment to be only a subset of the features of an ember app and usually there is no need to load stripe unless someone visits a page/route that needs stripe.

If I understand it correctly, the stripe loading wrapper will just replace the custom code to load stripe within this addon? That would be perfectly fine and less code to maintain 👍

@lindyhopchris
Copy link
Collaborator Author

Great - thanks for the response, we're on exactly the same page! I'll sort it out this week.

Fyi I tagged a 1.0.0 this morning from the master branch. The develop branch has the next major release so once I've taken care of this issue I might tag a 2.0.0-rc.1.

@st-h
Copy link
Contributor

st-h commented Feb 15, 2021

@lindyhopchris awesome. thanks a bunch 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants