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 reconfigureOnUpdate prop. Reconfigure whenever stripeKey changes. #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ This will give you a default *Stripe-style* button which looks like this:
token={this.onToken} // submit callback
opened={this.onOpened} // called when the checkout popin is opened (no IE6/7)
closed={this.onClosed} // called when the checkout popin is closed (no IE6/7)
// Note: `reconfigureOnUpdate` should be set to true IFF, for some reason
// you are using multiple stripe keys
reconfigureOnUpdate={false}
// Note: you can change the event to `onTouchTap`, `onClick`, `onTouchStart`
// useful if you're using React-Tap-Event-Plugin
triggerEvent="onTouchTap"
Expand All @@ -106,7 +103,7 @@ This will give you a default *Stripe-style* button which looks like this:
### Other info
This was probably terribly written, I'll look at any PR coming my way.

### Resources
### Resources

* [Accept Stripe Payments with React and Express](https://www.robinwieruch.de/react-express-stripe-payment/)

Expand Down
32 changes: 10 additions & 22 deletions StripeCheckout.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export default class ReactStripeCheckout extends React.Component {
label: 'Pay With Card',
locale: 'auto',
ComponentClass: 'span',
reconfigureOnUpdate: false,
triggerEvent: 'onClick',
}

Expand Down Expand Up @@ -57,13 +56,6 @@ export default class ReactStripeCheckout extends React.Component {
// Runs when the script tag is created, but before it is added to the DOM
onScriptTagCreated: PropTypes.func,

// By default, any time the React component is updated, it will call
// StripeCheckout.configure, which may result in additional XHR calls to the
// stripe API. If you know the first configuration is all you need, you
// can set this to false. Subsequent updates will affect the StripeCheckout.open
// (e.g. different prices)
reconfigureOnUpdate: PropTypes.bool,

// =====================================================
// Required by stripe
// see Stripe docs for more info:
Expand Down Expand Up @@ -250,8 +242,8 @@ export default class ReactStripeCheckout extends React.Component {
document.body.appendChild(script);
}

componentDidUpdate() {
if (!scriptLoading) {
componentDidUpdate(prevProps) {
if (!scriptLoading && this.props.stripeKey !== prevProps.stripeKey) {
Copy link

Choose a reason for hiding this comment

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

This is the wrong comparison to make. The stripeHandler is a static member of ReactStripeCheckout, so it may have been set by a completely different instance of the component.

Copy link
Author

Choose a reason for hiding this comment

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

I see what you are saying about multiple instances of the component.

All I'm doing here is replacing the reconfigureOnUpdate option with a prop check that checks for the only prop that matters for reconfiguring.

As currently written it doesn't appear that this component will work with multiple instances of the component each with their own stripe key and this PR isn't attempting to fix that.

Copy link

Choose a reason for hiding this comment

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

Agreed. That's why I put together #116 based on your work, which does allow for proper support for multiple instances with different Stripe keys.

this.updateStripeHandler();
}
}
Expand All @@ -267,13 +259,11 @@ export default class ReactStripeCheckout extends React.Component {
}

onScriptLoaded = () => {
if (!ReactStripeCheckout.stripeHandler) {
ReactStripeCheckout.stripeHandler = StripeCheckout.configure({
key: this.props.stripeKey,
});
if (this.hasPendingClick) {
this.showStripeDialog();
}
ReactStripeCheckout.stripeHandler = StripeCheckout.configure({
key: this.props.stripeKey,
});
if (this.hasPendingClick) {
this.showStripeDialog();
}
}

Expand Down Expand Up @@ -324,11 +314,9 @@ export default class ReactStripeCheckout extends React.Component {
});

updateStripeHandler() {
if (!ReactStripeCheckout.stripeHandler || this.props.reconfigureOnUpdate) {
ReactStripeCheckout.stripeHandler = StripeCheckout.configure({
key: this.props.stripeKey,
});
}
ReactStripeCheckout.stripeHandler = StripeCheckout.configure({
key: this.props.stripeKey,
});
}

showLoadingDialog(...args) {
Expand Down
1 change: 0 additions & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ declare module "react-stripe-checkout" {
alipay?: boolean
bitcoin?: boolean
allowRememberMe?: boolean
reconfigureOnUpdate?: boolean
triggerEvent?: "onTouchTap" | "onClick" | "onTouchStart"
}

Expand Down
1 change: 0 additions & 1 deletion spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ const props = {
alipay: false,
bitcoin: false,
allowRememberMe: false,
reconfigureOnUpdate: false,
triggerEvent: 'onClick',
className: 'StripeCheckout'
}
Expand Down