-
Notifications
You must be signed in to change notification settings - Fork 211
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
Unnecessary resources and HTTP requests on product and/or cart pages #2065
Comments
Just adding that I am able to replicate this behaviour on a site with just StoreFront (3.9.1), WooCommerce (5.7.1) and WooCommerce Stripe Gateway (5.6.2). I also have the Payment Request Buttons and New checkout experience disabled. Not sure if this is the intended behaviour. Screen.Capture.on.2021-10-11.at.10-14-19.mp4 |
Hi there, I have the same issue on my site, just a quick look into the code shows the issue very clearly: File: /includes/class-wc-gateway-stripe.php Lines 405 to 416
! is_product() basically means whatever option is set is not taken into account just continue to enqueue the scripts on the product pages. From my understanding it should be something like this:
Would be really nice to have a fix merged for this! |
That's a good idea. Thank you, @PaulSchiretz . In addition, you could also include a filter, so that theme/plugin developers can add or remove conditions. So basically this:
Plugin team, please action this soon. |
Can please someone have a look at this! I mean this issue might be solved in 10minutes and it would help so many of us, just give us something a filter or anything.... References of the same issue over and over again: |
I too found my way here with poor page time load performance on product pages linked to Current Proposal Undesired? - review Stripe documentation... however on reading the Stripe documentation
The documentation explicitly states:
This makes me believe the proposed pull request #2110 would not be desired... Loading script "async" / asynchronouslyA work around for WordPress not yet having https://github.com/WordPress/twentytwenty/blob/6d0a5240af108d02f58ec797fd49b32458a4c698/functions.php#L135-L140 Although then I question if this plugin's Perhaps something like the Stripe NPM implementation? SummaryThis doesn't mean that you can't avoid loading the plugin |
@DeanMarkTaylor This seems very reasonable, and well thought out. Thanks a lot. Having said that, some people may still not want the extra script(s) on every (shopping) page, so this should at least be an option, maybe via a true/false filter. |
Thank you all for the reports and the discussion you've got going on here! We apologize for the issue, and — as has been noted above — we are actively working to address the problem. Like @DeanMarkTaylor rightly pointed out in #2065 (comment) Stripe recommends we load the Stripe JS on as many pages as possible to help combat fraud, which limits the appropriate solutions we can come up with a bit. We're still discussing this but right now we're leaning towards keeping the defaults as they currently are, but offer a filter that will allow merchants to not load the Stripe JS when Payment Request Buttons (Apple Pay/Google Pay) are disabled on the Product and Cart pages. Merchants will have to weigh the risk of potentially increased rates of attempted/succesful fraud on their stores versus the potential increase in conversion rates from shorter page load times. We'd be happy to hear more of your thoughts on the matter, so keep them coming 🙂 |
@reykjalin Filters sound good to me, i think there is no right or wrong in this case, so let the user decide should be the option of choice 🙂 Could also be a setting of the plugin as @galbaras mentioned, a checkbox maybe "Stripe fraud protection" on/off... but filters as of your pull request also do the trick for me. As long as we're able to disable the scripts, if we don't want them, everything is fine! I just don't get why everyone is just like "hey let's load some scripts on every pageview, even if we mostly don't need them" sounds a bit like googles recapture approach, let's load it on every page, when a simple hidden honeypot field for forms is exactly as effective and the performance impact is close to zero. But hey that's just my 2 cents.... Anyway, THANKS a lot Kristófer for taking on the issue! 🙂 |
Disable advanced fraud detectionRegarding not loading not loading Stripe.js ( This would pass the
PerformanceAlthough I believe the critical factor here is the This is for both user experience and SEO, checking Google Page Insights scores for product pages with "Enable Payment Request Buttons. (Apple Pay/Google Pay)" disabled. I see "Main-Thread Blocking Time" of over 1 second (1,067 ms) being added to the page load / time to interactive. But it is true that Stripe is also the largest 3rd party library on the page at 220KiB - so avoiding loading completely unless required for checkout may be preferential for some markets (where data usage / internet performance is a concern). So, disabling loading of Stripe.js (
|
I have this issue too on my site, and have had some conversations with WC support staff, who seem to offer only solutions that require turning code of on a page by page basis. This is on a site, where we: It really is too much I think, to ask a user to disable a feature that the user is NOT using. Thanks, please fix this! William |
We could just treat the new strip.com script as a new feature. This means that it's disabled by default on existing installations, with the option to enable it. For simplicity and speed of release, though, we can start with adding |
Taking into account my comments regarding async loading implementation here... I have started a pull request #2118 which is targeting loading the Stripe.JS library asynchronously. Feel free to code review it over there, the code has not been tested or even executed at-all... |
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.
We've merged the filter solution and it should be released this coming Wednesday with Stripe v5.8.0. I'm still going to re-open this issue so we can talk a bit more about optimal solutions aside from the filters we've just added. I would also like to add that I do plan on replying to the latest comments and keep this discussion going. I'll get back to you all on that this week 🙂 |
I have deactivated WooCommerce and our Formidable Forms WooCommerce Addon and I am still seeing the js.stripe.com and m.stripe.network loading? Please make sure to not load these if the plugin is installed but not activated, would be truly grateful. Thank you. |
@jenfloods There may be some caching involved. Also, this seems like a new issue (plugin active when WooCommerce isn't), so do some more investigations and start a new issue once you're done. |
@reykjalin Thank you for this change. One thing I don't see in it is adding |
Can you explain this further?
Sincerely,
Jen Niles
*Jennifer A. Niles*Front-end Web Developer
*ASFPM | *8301 Excelsior Dr. | Madison, WI 53717
tel: 608-828-3000 | cell: 608-963-5280 | floods.org
<http://www.floods.org/> | *Email Jen ***@***.***>*
…On Mon, Nov 8, 2021 at 7:47 PM Gal Baras ***@***.***> wrote:
@reykjalin <https://github.com/reykjalin> Thank you for this change. One
thing I don't see in it is adding async to the scripts when they are
included. This should be the last bit to close off this issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2065 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMH7QRYAALEQLCYGPBOFGCTULB4TDANCNFSM5FUZLOFA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
If there's page caching and/or page optimisation on the site, clear it. If there's caching on your browser, clear it. Then, gather all the pertinent information, including relevant setup and steps to reproduce the problem, and open a new issue, because your issue is likely different from this one. |
A quick TL;DR: I agree with all of your conclusions: this issue shouldn't be considered resolved until the Stripe JS is loaded asynchronously. Unfortunately this isn't as simple as just adding the We'll keep working on this, but loading the JS asynchronously will take significantly longer than adding the filters did. From @galbaras:
I think there's some miscommunication here, so I just want to clear that up now: the stripe.com script (what I've been referring to as "Stripe JS") is not a new feature; it's necessary to process payments and always has been.
Unfortunately this is not a trivial feature to add as @DeanMarkTaylor pointed out in #2065 (comment).
Yes, this is intentional. Like @DeanMarkTaylor pointed out in #2065 (comment) this isn't simple. I go into more detail about that further down in this comment. From @DeanMarkTaylor:
I would consider this outside the scope of this issue since disabling advanced fraud detection would have a negligible effect on the performance problems reported in this issue; the Stripe JS is still loaded in it's entirety.
Thank you so much for putting that together! Unfortunately I don't think this is the most future proof solution we can come up with. The best approach, in my opinion, would be to move the classic shortcode JS out of the Migrating to
Both approaches (
There are no benefits to the " From @jenfloods:
Just to be clear on one part: the Stripe JS (loaded from js.stripe.com) is necessary to process payments. As such it will be included on all pages where payments can be processed, e.g. the Checkout page, and the Cart and product pages when Payment Request Buttons are enabled. @galbaras is also right in that this issue is separate from the issue reported here 🙂 If you're seeing the script loaded on product pages or the cart page, read on: I agree with @galbaras' initial assessment; this is probably related to caching — either through your hosting provider, or in your browser. Try opening the page you're seeing the script loaded on in private browsing mode; that will ensure your browser's cache is empty. If the problem persists, please open a new issue |
@reykjalin Thank you for the considered reply. Short term, using a filter to add |
This js.stripe.com - v3/ still appears after WP Rocket and clear cache from
yesterday.
[image: image.png]
Sincerely,
Jen Niles
*Jennifer A. Niles*Front-end Web Developer
*ASFPM | *8301 Excelsior Dr. | Madison, WI 53717
tel: 608-828-3000 | cell: 608-963-5280 | floods.org
<http://www.floods.org/> | *Email Jen ***@***.***>*
…On Tue, Nov 9, 2021 at 5:16 AM Gal Baras ***@***.***> wrote:
@reykjalin <https://github.com/reykjalin> Thank you for the considered
reply.
Short term, using a filter to add async doesn't seem like a big deal.
Long term, perhaps everyone in this discussion can support
https://core.trac.wordpress.org/ticket/51124, which would make adding
script parameters easy peasy.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2065 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMH7QR66642ME2OWG2UFHULULD7I7ANCNFSM5FUZLOFA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
@jenfloods would you be so kind as to open a new issue since what you're facing is unrelated to this issue? |
@reykjalin Was that Wednesday the 10th or the 17th of November? |
Sorry for the confusion around the release date everyone! We were aiming for Wednesday last week (November 10) but decided to delay the release because of issues we discovered while testing the release. We are currently aiming to release version 5.8.0 tomorrow, November 17. |
I am the only one who still have the same problem with the last release ? :/ When on 5.5.0 it's fine, but on 5.8.0 I still have these unwanted files on product page 🙄 |
No, I'm seeing it as well. If anything the page I am testing on is worse now than it was previously. ApplePay/etc, are all disabled on the site (just CC checkout on the "Checkout" page). No "Buy Now" options. Using the update today (5.8.0) |
@ozidrice @wilstart have you enabled the new filters that we've included to disable any extra JS on the cart and product pages? You can enable them with this snippet: 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' ); |
I just added this, and Google page load is still detecting 2 different callouts to I don't see any difference in the page ranking with or without these "add_filter" added. So, I'd have to see this issue it not fixed. And, while I may not appreciate the difficulties involved in the coding here, it still seems fundamentally flawed that I have to add code to disable a feature I am not using... Just to confirm my Stripe settings: In the "Manage" settings for Stripe, "Enable express checkouts" is NOT checked. Even if this were checked, you have options there to enable this feature on Product pages, Checkout or Cart pages, so you have all the UI to decide whether you need to run this code on Product pages (Cart or Checkout pages as well) don't you? |
I would normally agree, but this is not clear cut, considering Stripe recommends it, and it's generally good when the plugin implements Stripe recommendations. Sadly, I also see both the local and external Stripe assets being included in the page, despite both of the filters being in effect 😞 I'm wondering about this code:
Should the filter not apply when Oops... Same for the cart check, BTW. |
Hi all, Thank you for reporting the issues you've had with getting the filters to work! I've looked into this, and it seems I made a mistake when creating the filters we introduced in version 5.8.0; they don't take into account whether the PRBs are enabled or not. I've opened a PR to fix the issue in #2179. There's currently no easy way to work around the issue so we're aiming to get a patch release out today. I apologize for the inconvenience this has caused and thank you all for your patience! EDIT: Version 5.8.1 has been released with a fix; the filters should now work as intended. |
@galbaras That would make it so that the scripts can be disabled when the PRB is supposed to be shown on the product page 🙂 The code works as intended when the PRBs are enabled. The mistake I made was not running the filters when the PRBS are disabled (see #2065 (comment)). |
I can confirm that v5.8.1 doesn't load Stripe resources on product and/or cart pages when the respective filters are used. |
I confirm it too, it's working perfectly with the two filters mentioned earlier |
With the latest update, do we still need to add in those filter codes?
Sincerely,
Jen Niles
*Jennifer A. Niles*Front-end Web Developer
*ASFPM | *8301 Excelsior Dr. | Madison, WI 53717
tel: 608-828-3000 | cell: 608-963-5280 | floods.org
<http://www.floods.org/> | *Email Jen ***@***.***>*
…On Wed, Nov 24, 2021 at 7:10 AM ozidrice ***@***.***> wrote:
I confirm it too, it's working perfectly with the two filters mentioned
earlier
Perfect, thanks a lot ! 💯
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2065 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMH7QR4FGXSKTKK5XGK6GULUNTP3DANCNFSM5FUZLOFA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
@jenfloods Yes you would still need to add them. Cheers |
I don't understand this as a fix. You have UI that explicitly lists where to activate Stripe wrt payment on non-checkout pages. Yet we have to add PHP filters as well to disable Stripe code on pages where no payout options are offered to the user? So, this isn't really a fix from what I understand, but rather a workaround? If so, when will this actually be properly fixed? Why can't the plugin install these filters itself by default, and then disable them when the user CHECKS the boxes to add these Stripe payment options? As a "for instance": What happens when I install these filters, and then sometime later I CHECK these payment options expecting to now load Stripe on a product page for a PayNow option? (That is, the UI has PayNow options checked and you have these filters installed)? |
The plugin now implements new Stripe recommendations. As such, we should all be loading the Stripe scripts on every page, and the plugin is now compliant with that. Those of us who think they know better than Stripe, or don't need the extra protection, can now prevent those scripts from loading. The filters only block resource loading when payment buttons are disabled. When they are enabled, the filter will be ignored. |
Why? Surely you would expect better than "so and so tells us so" - this is religion, not science, and given the absolutely noticeable and deleterious effects it has on every product page, we deserve a better explanation than "because Stripe says so". How is this serving our customers who just want to look at what we are offering, but not buying (at least not today) and so there is NO transaction that Stripe is involved in. Yet, each and every product page pays this penalty. I think you are mis-characterising my concerns. You could even load this "extra protection" on the Cart page, where at least you have some reasonable intention to create an interaction which Stripe (you have chosen to buy something) and the Cart page is still before the ACTUAL checkout page, where in my sites, the payment conditions are being established. If executing their code had no measurable performance hit, well, I guess I'd be more forgiving. But "because Stripe says so" given that, and without a deeper questioning/pushback from you guys is what I find unacceptable. Please reconsider your position. |
This plugin was written precisely to facilitate Stripe payments. On one hand, it "listens" to WooCommerce. One the other, to Stripe. You wouldn't want it to continue using an old API, for example, and this is a kind of new API, which you would have to use if you used Stripe directly. You now have a way to make things work for you, and the new feature has already rolled out. Let's give it a rest. |
People that have implemented the filters, have you noticed any real improvement in performance and/or did you see any issues in fraud protection? |
Performance gain is huge, no issues what so ever till now :-) |
v5.6.2 loads an external script from Stripe, as a well as several "local" scripts and a "local" stylesheet on product pages and on the cart page. These all load synchronously.
When the page loads, there are additional requests for documents and GIF images.
When disabling the
Payment Request Buttons
options, some of these requests disappear, but not all of them.When enabling the
New checkout experience
(credit/debit card only), the remaining requests disappear. However, once there's a product in the cart, they appear on the cart page and on any page showing the cart widget.Please refer to https://wordpress.org/support/topic/stripe-js-loading-on-product-page-destroys-site-speed/ for more.
Steps to reproduce the behavior
Note:
Payment Request Buttons
andNew checkout experience
are currently disabled.Expected behavior
No HTTP requests matching "stripe" on product or cart pages. Stripe is only required on the checkout page.
Environment
The text was updated successfully, but these errors were encountered: