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

Payments: Move lib/stripe to calypso-stripe package #46973

Merged
merged 16 commits into from
Nov 4, 2020

Conversation

sirbrillig
Copy link
Member

@sirbrillig sirbrillig commented Oct 30, 2020

Changes proposed in this Pull Request

This moves the lib/stripe directory (which is used to access Stripe Elements for our credit card form fields) to a new npm package in the monorepo called @automattic/calypso-stripe.

In the process of this move, I converted the functions in that file to TypeScript (it's easiest to review that change by looking at the commits in this PR one-by-one). While doing this, I tried not to modify the function signatures at all, but a few changes were necessary, which I will list below.

  • StripeHookProvider now has fetchStripeConfiguration as a required prop. Previously this was optional, and the default was to use the bound wpcom.js functions. However, the authentication needed for that library is entangled deeply with calypso, making it much easier to inject it from the consumer components rather than try to import it wholesale. All implementations have been updated to have this prop.
  • StripeHookProvider now has an optional locale prop. In Checkout: pass locale to Stripe (1) #43026, we added a mechanism to pass the user's locale to Stripe.js which it will use to localize any error messages it creates. However, that data comes from the calypso Redux store and is therefore tightly coupled to the calypso package. It is easier to inject the string from the consumer components rather than try to pull it directly within the new package. All implementations (other than tests, and unused components) have been updated to have this prop.

This also implements the changes from #46953 and #47002

Testing instructions

Because of the move itself, as well as the modifications to StripeHookProvider above, this touches quite a few places in calypso. They should not be affected, but it's worth testing all of them to be sure. For the most part, if you visit these pages and the credit card form loads without any errors (either in the UI or in the console), then this should be fine.

Screen Shot 2020-11-04 at 11 12 28 AM

(This also touches old checkout for the sake of completeness, but there's no need to test that because it is unused and that code will soon be removed.)

@sirbrillig sirbrillig requested a review from a team as a code owner October 30, 2020 20:46
@matticbot
Copy link
Contributor

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 30, 2020
@sirbrillig sirbrillig self-assigned this Oct 30, 2020
@matticbot
Copy link
Contributor

matticbot commented Oct 30, 2020

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~2113 bytes added 📈 [gzipped])

name            parsed_size           gzip_size
site-purchases      +1968 B  (+0.2%)     +593 B  (+0.3%)
purchases           +1937 B  (+0.2%)     +492 B  (+0.2%)
checkout            +1797 B  (+0.1%)     +577 B  (+0.1%)
woocommerce         +1758 B  (+0.1%)     +451 B  (+0.1%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~1361 bytes removed 📉 [gzipped])

name                                             parsed_size           gzip_size
async-load-design-blocks                             -7494 B  (-0.2%)    -1753 B  (-0.2%)
async-load-calypso-blocks-editor-checkout-modal      +1766 B  (+0.2%)     +392 B  (+0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@tjcafferkey tjcafferkey left a comment

Choose a reason for hiding this comment

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

Might be worth another reviewer double checking the instances outside of the editor checkout modal, but from my point of view this change LGTM.

I've also followed all of the testing steps and seems to work fine too 😄

@sirbrillig sirbrillig force-pushed the update/move-lib-stripe-package branch from 74f36e1 to 69bd67a Compare November 4, 2020 15:40
@sirbrillig sirbrillig merged commit 326ac9a into master Nov 4, 2020
@sirbrillig sirbrillig deleted the update/move-lib-stripe-package branch November 4, 2020 16:13
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 4, 2020
@michaeldcain michaeldcain added [Feature] Checkout The checkout screen and process for purchases made on WordPress.com. [Estimate] 3 labels Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Estimate] 3 [Feature] Checkout The checkout screen and process for purchases made on WordPress.com.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants