Only load Stripe JS on product page when required #2110

reykjalin · 2021-11-02T20:46:11Z

Changes proposed in this Pull Request:

Fixes #2065

Don't load Stripe JS on product pages when PRBs are disabled.
Don't load Stripe JS on the cart page when PRBs are disabled.

Testing instructions

Note: The test should be performed with UPE enabled and disabled, and on both the Shortcode and Block-based cart pages.

Make sure PRBs are only present on the Checkout page.
git switch develop.
Open the Network tab of your browser's developer tools.
Open any product details page.
Notice that we load scripts from js.stripe.com.
Add a product to your cart and go to your store's cart page.
Notice that we load scripts from js.stripe.com.
git switch fix/2065-only-load-stripe-js-on-product-pages-when-required
Open any product details page.
Make sure we still load scripts from js.stripe.com.
Add a product to your cart and go to your store's cart page.
Make sure we still load scripts from js.stripe.com.
Use the Code Snippets plugin to add the following filters:

add_filter( 'wc_stripe_load_scripts_on_product_page_when_prbs_disabled', '__return_false' );
add_filter( 'wc_stripe_load_scripts_on_cart_page_when_prbs_disabled', '__return_false' );

Open any product details page.
Make sure no scripts are loaded from js.stripe.com
Add a product to your cart and go to your store's cart page.
Make sure no scripts are loaded from js.stripe.com

Covered with tests (or have a good reason not to test in description ☝️)
Added changelog entry (or does not apply)
Tested on mobile (or does not apply)

Post merge

Added testing instructions to the Release Testing Instructions wiki page (or does not apply)

Previously we always loaded the Stripe JS on relevant pages through the main gateway class, but now we have an additional check there to make sure that doesn't happen.

Previously we always loaded the Stripe JS on relevant pages through the main gateway class, but now we have an additional check there to make sure this doesn't happen.

.vscode/launch.json

diegocurbelo

Code looks good, and it works as expected in both the product and shortcode cart pages... but it still loads Stripe in the blocks cart.

Also, should we create an issue to do this for UPE?

…ages-when-required

reykjalin · 2021-11-04T07:45:44Z

it still loads Stripe in the blocks cart.

Yeah, I hadn't thought about it there 🤔 It's less of a problem in the Blocks integration though since the Stripe JS SDK is loaded asynchronously. We should still make removing unnecessary JS an option.

Also, should we create an issue to do this for UPE?

I think we can just address this here 🤔

@DeanMarkTaylor brought up a point in #2065 (comment) that we've been discussing in p1635879932054100-slack-C01BZUL57SQ regarding how Stripe recommends we load the Stripe JS on all pages for fraud prevention purposes.

Currently we only load the Stripe JS on Product, Cart, and Checkout pages, and I don't think we should change that. However, based on Stripe's own documentation we probably shouldn't be disabling the scripts by default.

I think the best path forward would be to introduce a filter that will allow merchants to disable the Stripe JS on product and cart pages when the PRBs are not loaded. We'll still load the Stripe JS by default to follow Stripe's recommendation on loading the script on multiple pages for fraud prevention purposes, but merchants that are willing to accept the risk of potentially increased rates of fraud can disable the script entirely.

These filters will only be applied when the PRBs are disabled on the product and cart pages. This will give merchants the option to not load the extra JS when they're not using the PRBs on those pages. Stripe's JS includes some fraud prevention mechanisms, so disabling the scripts using these filters does increase the risk of successul fraud on a merchant's store, but that's something merchants will have to decide on their own.

includes/class-wc-gateway-stripe.php

includes/class-wc-stripe-helper.php

Only applies on Product and Cart pages when PRBs are disabled.

includes/class-wc-stripe-blocks-support.php

includes/payment-methods/class-wc-stripe-upe-payment-gateway.php

…ages-when-required

diegocurbelo

After adding the 2 filters... the product and shortcode cart don't load the Stripe JS, but both pages have a warning at the top (scrolling up after page load), the blocks cart does not have the warning but the Stripe JS is still loaded:

…ages-when-required

reykjalin · 2021-11-06T07:58:48Z

both pages have a warning at the top (scrolling up after page load)

@diegocurbelo I'm unable to reproduce the issue 🤔

What versions of WP/WC/Blocks are you using?

the blocks cart does not have the warning but the Stripe JS is still loaded

Ah yeah, I can reproduce this. I'll look into that! Nice catch!

reykjalin · 2021-11-06T08:15:06Z

@diegocurbelo It seems maybe we can't detect this for the blocks integration?

When I'm on the page with the cart block (designated as the Cart page) both is_cart() and has_block( 'woocommerce/cart' ) return false 🤨 .

This is definitely less of a problem with the Blocks integration because the Stripe JS is loaded asynchronously via the Stripe SDK npm package using loadStripe(), but it seems like we can't offer this filter in the Blocks integration.

If we only rely on the settings (the result from WC_Stripe_Helper::should_load_scripts_on_cart_page()) we'd end up removing the script from the Checkout page too, and that's not an option.

All in all it looks like we may have to just not apply the filter for the blocks integration? What do you think?

diegocurbelo · 2021-11-08T20:49:47Z

All in all it looks like we may have to just not apply the filter for the blocks integration? What do you think?

I agree... The JS loads asynchronously in that case

…ages-when-required

The function used to retrieve the script handles is run at a point where the block hasn't been loaded, so we can't detect whether we're on a cart or product page. As such we can't make an informed decisions on whether or not to load the script. The script is loaded asynchronously in the blocks checkout though, so this shouldn't be degradation in performance as it relates to the issue reported in #2065.

The UPE payment_scripts() function is not called on product pages, and as such it is not necessary to check for that condition when unloading the Stripe JS.

diegocurbelo

Code looks good... and tests well 🚢 🚢

woocommerce/woocommerce-gateway-stripe#2110

Only load Stripe JS on product page when required

4d737c3

Previously we always loaded the Stripe JS on relevant pages through the main gateway class, but now we have an additional check there to make sure that doesn't happen.

reykjalin added the pr: in progress label Nov 2, 2021

Only load Stripe JS on the cart page when required

fd11638

Previously we always loaded the Stripe JS on relevant pages through the main gateway class, but now we have an additional check there to make sure this doesn't happen.

reykjalin added pr: needs review and removed pr: in progress labels Nov 2, 2021

reykjalin requested a review from diegocurbelo November 2, 2021 20:58

add changelog entry

4fa9d66

diegocurbelo reviewed Nov 3, 2021

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

diegocurbelo requested changes Nov 3, 2021

View reviewed changes

diegocurbelo added pr: needs author reply and removed pr: needs review labels Nov 3, 2021

DeanMarkTaylor mentioned this pull request Nov 4, 2021

Unnecessary resources and HTTP requests on product and/or cart pages #2065

Closed

Kristófer Reykjalín added 2 commits November 4, 2021 03:32

Merge branch 'develop' into fix/2065-only-load-stripe-js-on-product-p…

11b1f27

…ages-when-required

revert launch json config change

94b2851

reykjalin commented Nov 4, 2021

View reviewed changes

includes/class-wc-gateway-stripe.php Outdated Show resolved Hide resolved

Add helper functions to determine when to load js

a152af7

reykjalin commented Nov 4, 2021

View reviewed changes

includes/class-wc-stripe-helper.php Show resolved Hide resolved

Disable JS for UPE and Blocks when appropriate

38b03e1

Only applies on Product and Cart pages when PRBs are disabled.

reykjalin commented Nov 4, 2021

View reviewed changes

includes/class-wc-stripe-blocks-support.php Outdated Show resolved Hide resolved

includes/payment-methods/class-wc-stripe-upe-payment-gateway.php Outdated Show resolved Hide resolved

reykjalin requested a review from diegocurbelo November 4, 2021 08:35

reykjalin added pr: needs review and removed pr: needs author reply labels Nov 4, 2021

Merge branch 'develop' into fix/2065-only-load-stripe-js-on-product-p…

a085933

…ages-when-required

DeanMarkTaylor mentioned this pull request Nov 5, 2021

Load Stripe.JS library asynchronously #2118

Draft

4 tasks

diegocurbelo requested changes Nov 5, 2021

View reviewed changes

diegocurbelo added pr: needs author reply and removed pr: needs review labels Nov 5, 2021

Merge branch 'develop' into fix/2065-only-load-stripe-js-on-product-p…

c7c0beb

…ages-when-required

reykjalin added pr: needs review and removed pr: needs author reply labels Nov 6, 2021

reykjalin requested a review from diegocurbelo November 6, 2021 08:16

Kristófer R added 4 commits November 8, 2021 16:29

return empty list of script handles for blocks support

4e0c5b7

Merge branch 'develop' into fix/2065-only-load-stripe-js-on-product-p…

0bc288c

…ages-when-required

Remove redundant product page check for UPE filter

9bc82b5

The UPE payment_scripts() function is not called on product pages, and as such it is not necessary to check for that condition when unloading the Stripe JS.

diegocurbelo approved these changes Nov 8, 2021

View reviewed changes

diegocurbelo added pr: ready to merge and removed pr: needs review labels Nov 8, 2021

reykjalin merged commit a2db575 into develop Nov 9, 2021

reykjalin deleted the fix/2065-only-load-stripe-js-on-product-pages-when-required branch November 9, 2021 00:43

reykjalin mentioned this pull request Nov 23, 2021

Run filters to disable Stripe JS when PRBs are disabled #2179

Merged

4 tasks

sebastianmoran-mainwp added a commit to mainwp/Code-Snippets-Functions that referenced this pull request Nov 24, 2021

Create disable-stripe-scripts-loading-if-prbs-is-disabled.txt

63b85f4

woocommerce/woocommerce-gateway-stripe#2110

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only load Stripe JS on product page when required #2110

Only load Stripe JS on product page when required #2110

reykjalin commented Nov 2, 2021 •

edited

Loading

diegocurbelo left a comment

reykjalin commented Nov 4, 2021

diegocurbelo left a comment

reykjalin commented Nov 6, 2021

reykjalin commented Nov 6, 2021

diegocurbelo commented Nov 8, 2021

diegocurbelo left a comment

Only load Stripe JS on product page when required #2110

Only load Stripe JS on product page when required #2110

Conversation

reykjalin commented Nov 2, 2021 • edited Loading

Changes proposed in this Pull Request:

Testing instructions

diegocurbelo left a comment

Choose a reason for hiding this comment

reykjalin commented Nov 4, 2021

diegocurbelo left a comment

Choose a reason for hiding this comment

reykjalin commented Nov 6, 2021

reykjalin commented Nov 6, 2021

diegocurbelo commented Nov 8, 2021

diegocurbelo left a comment

Choose a reason for hiding this comment

reykjalin commented Nov 2, 2021 •

edited

Loading