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

Remove sync Stripe wrapper #1

Merged
merged 3 commits into from
Jan 23, 2020
Merged

Conversation

dweedon-stripe
Copy link
Contributor

Summary & motivation

Remove the synchronous Stripe wrapper in favor of encouraging loadStripe as the default way to load Stripe.js using a modern toolchain. The synchronous wrapper was difficult to document and had minimal value.

Testing & documentation

I updated the tests and documentation.

Copy link
Contributor

@christopher-stripe christopher-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. I noticed a small typo in the readme (wasn't part of this PR):

... directly from https://js.stripe.com. You cannot included it in a bundle or host it yourself.

Copy link
Contributor

@hofman-stripe hofman-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following section in the Readme should be reworked and integrated in each subsection below now that there is only the loadStripe + import way explained before:

If you are adding the <script> tag manually, make sure you do so on every
page. If you are relying on the script insertion that this module provides, and
you utilize code splitting or only include your JavaScript app on your checkout
page, you will need to take extra steps to ensure Stripe.js is available
everywhere.

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

Successfully merging this pull request may close these issues.

3 participants