Problem/Motivation

It seems that after updating to Commerce 3.x the way libraries are attached to payments form through CommercePaymentGateway attributes has changed, as can be seen in the following changelog: https://www.drupal.org/node/3468908.

This appears to be the cause of multiple PHP Warnings: Warning: Array to string conversion in Drupal\Core\Asset\LibraryDependencyResolver->getMinimalRepresentativeSubset() . Here's a screenshot with all the messages that appear:

commerce_unzer_warnings

The js_library property in the CommercePaymentGateway attribute is now deprecated and the current implementation doesn't align with the new libraries requirement for attributes. However, in the tests I have been able to perform, the problem doesn't disappear when js_library is replaced by libraries. It seems the problem persist in both options because the Drupal's Asset resolver (AttachedAssets.php) cannot process, leading to the "Array to string conversion" warning.

The environment where this is happening has the following core and module versions:

  • Drupal 11.2.8
  • Commerce 3.2.0
  • Commerce Unzer 2.0.0-alpha1

Proposed resolution

  1. Update the CommercePaymentGateway: Remove the deprecated js_library property from UnzerOnSite.
  2. Library injection: Move the library attachment to PaymentMethodAddForm::buildCreditCardForm. This follows the pattern used by other modern Commerce payment providers like Stripe, to ensure the library is only loaded when the form is actually rendered, avoiding "leaking" assets to other parts of the checkout. See https://git.drupalcode.org/project/commerce_stripe/-/blob/2.x/src/Plugin... + https://git.drupalcode.org/project/commerce_stripe/-/blob/2.x/src/Plugin...
  3. Apply the changes to both 2.0.x-dev and 3.0.x-dev
CommentFileSizeAuthor
commerce_unzer_warnings.png960.05 KBdimaro

Issue fork commerce_unzer-3569934

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

dimaro created an issue. See original summary.

dimaro’s picture

dimaro commented
Issue summary: View changes
dimaro’s picture

dimaro commented
Issue summary: View changes

dimaro opened merge request !6

dimaro’s picture

dimaro commented
Assigned: dimaro » Unassigned
Status: Active » Needs review
dimaro’s picture

dimaro commented
Issue summary: View changes
dimaro’s picture

dimaro commented
Issue summary: View changes
dimaro’s picture

dimaro commented
Issue summary: View changes
cspitzlay’s picture

cspitzlay
Primary language German
Location 🇩🇪🇪🇺
commented

Hi,

I don't understand what you mean by

... like Stripe, to ensure the library is only loaded when the form is actually rendered, avoiding "leaking" assets to other parts of the checkout

when the stripe code you linked to still references the library in the CommercePaymentGateway annotation, and even with the deprecated key "js_library":

/**
 * Provides the Stripe payment gateway.
 *
 * @CommercePaymentGateway(
 *   id = "stripe",
 *   label = "Stripe Card Element",
 *   display_label = "Stripe Card Element",
 *   forms = {
 *     "add-payment-method" = "Drupal\commerce_stripe\PluginForm\Stripe\PaymentMethodAddForm",
 *   },
 *   payment_method_types = {"credit_card"},
 *   credit_card_types = {
 *     "amex", "dinersclub", "discover", "jcb", "maestro", "mastercard",
 *   "visa", "unionpay"
 *   },
 *   js_library = "commerce_stripe/form",
 *   requires_billing_information = FALSE,
 * )
 */

Is it really ok to remove the library from the plugin?

dimaro’s picture

dimaro commented

Hello @cspitzlay,

By "leaking assets", I mean that when a library is defined in the CommercePaymentGateway annotation – if I'm not very much mistaken – it's often globally attached to the checkout page or preloaded even if the specific payment form is not yet active or visible to the user. So, I thought that moving the library to buildCreditCardForm ensures the assets / libraries are only injected into the render array when the credit card form is actually built.

Is it really ok to remove the library from the plugin?

Then, regarding how the Commerce Stripe module does it, at first I thought exactly the same as you. In fact, I'm still wondering if for backward compatibility reasons it would be okay to keep the js_library property in the annotation. However, keeping js_library in the annotation continues to trigger the "Array to string conversion" warning, even if we simultaneously attache the library throught the buildCreditCardForm method. That's why I decided to remove js_library, despite the fact that it still appears in Stripe. I haven't tried Stripe yet, and it might be causing the same warning I mentioned.

Finally, I also decided to remove js_library because having the library defined in two planes sometines is prone to errors, at least at this time whent it appears that the "libraries" property is not working correctly within the CommercePaymentGateway annotation. Perhaps I'm overlooking something, but I have tried different approaches and none of them worked, only the option in the Merge request.

  • dimaro committed 3a1bf5d1 on 2.0.x 
    #3569934: Migrate deprecated js_library to explicit form attachment to...
cspitzlay’s picture

cspitzlay
Primary language German
Location 🇩🇪🇪🇺
commented

So, I thought that moving the library to buildCreditCardForm ensures the assets / libraries are only injected into the render array when the credit card form is actually built.

I understood your intention, I was just wondering why you pointed to the stripe case as an example when they didn't drop the key from the plugin.

I merged your change to 2.0.x and cherry-picked it to 3.0.x

  • cspitzlay committed 25065d3e on 3.0.x authored by dimaro 
    #3569934: Migrate deprecated js_library to explicit form attachment to...
cspitzlay’s picture

cspitzlay
Primary language German
Location 🇩🇪🇪🇺
commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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