Problem/Motivation

In earlier versions of the module, stripe js was not loaded on every page of the site. This changed in a recent commit somewhere, possibly involving the stripe payment element.

Loading js globally represents a considerable performance hit for supposed security gains. We can see up to a second of extra load time depending on network conditions. Stripe "recommends" loading their js globally, but it is not a requirement.

See #3083393: Limit Stripe scripts and cookies to pages where they are required.

Steps to reproduce

Enable the stripe card element gateway. Notice stripe.js is loaded on every single page of the site, not just where it is required (checkout).

Proposed resolution

I guess we are stuck with it now. Keep the current behaviour but add an override checkbox in configuration (checked by default).

Issue fork commerce_stripe-3465365

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

John Pitcairn created an issue. See original summary.

johnpitcairn’s picture

johnpitcairn commented
Issue summary: View changes
johnpitcairn’s picture

johnpitcairn commented
Issue summary: View changes
jsacksick’s picture

jsacksick commented

As I see it, the problem is you could have multiple Stripe payment gateways on your site (with potentially different conditions).
So a setting at the gateway level would mean loading all gateways, and checking if toggle is on on any of them (which isn't optimal).

Alternative would be checking a setting flag... Like the following:
Settings::get('commerce_stripe_global_library_include', TRUE);. (poor naming, wasn't inspired) or a separate config object (not the best UX wise).

Another option if you're really bothered by that is to use a hook_module_implements_alter() to unregister the hook_page_attachments() implementation from Commerce Stripe.

Also, we probably should have discussed this before re-adding the library to all pages, considering the issue linked / previous complaints.

tomtech’s picture

tomtech commented
Assigned: Unassigned » tomtech

TomTech opened merge request !124

  • 65a70ac7 committed on 8.x-1.x 
    Issue #3465365 by TomTech, john pitcairn, jsacksick: Regression: Allow...
tomtech’s picture

tomtech commented
Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.