search

adrienverge / localstripe

A fake but stateful Stripe server that you can run locally, for testing purposes.
GNU General Public License v3.0
192 stars 59 forks source link

localstripe.js: Make compatible with react-stripe-js #175

Closed brandondyck closed 3 years ago

brandondyck commented 3 years ago

This fixes #147.

brandondyck commented 3 years ago

Can you please split each change in an individual commit (easy ones could even be merged in separated PRs, if you'd like), and explain the motivation in each commit message, so reviewers can understand their goal?

Sure, I will split it up and clarify my intent. 🙂

brandondyck commented 3 years ago

I've split this up into several commits. The last one is still rather complex, but I don't think there's a reasonable way to avoid that. 🤷

I also made more changes to fix some problems I found while addressing your comments:

  1. I found a bug in the way mount was using document.querySelector.
  2. The 'localstripe:' span was not being removed by unmount.
  3. Unexpected sequences of mount, unmount, and destroy calls could leave the elements in an inconsistent state and did not match the behavior of Stripe.js very well, so I introduced a field in Element to explicitly track mounting/destruction state. This was kind of a large change, and I'm sorry to give you more work to do after you've already kindly reviewed my PR. 😅 I also hope you don't mind the long essay I wrote in the commit message. 😁
brandondyck commented 3 years ago

I've simplified Element using @adrienverge's idea of keeping an array of children. Element no longer matches Stripe's behavior as closely. mount still throws an exception if the element has been destroyed, because I think it's important to prevent the broken state of having a duplicate set of input fields with no access to it via getElement.