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

Add (Dequeueable?)ExternalAsset #23

Closed
wants to merge 1 commit into from

Conversation

lkraav
Copy link

@lkraav lkraav commented Mar 16, 2021

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

⬆️ not yet.

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Looking to fix #19

I've done research on what I want happening vs what the current Assets APIs are, but could use some guidance from here on whether this PR direction is making architectural sense.

  • What is the current behavior? (You can also link to an open issue here)

N/A

  • What is the new behavior (if this is a feature change)?

I want to reach something like this

$script = new ExternalScript( 'woocommerce' );
$script->canEnqueue( static fn(): bool => is_checkout() );

I want this to auto-dequeue external asset if canEnqueue() doesn't pass.

I still need to implement External{Script,Style}, but thought I'd post what I have at the moment.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    Hopefully not.

  • Other information:

What are your thoughts about how this idea is looking?

@bueltge bueltge requested a review from Chrico April 21, 2021 21:35
@bueltge bueltge added the enhancement New feature or request label Apr 21, 2021
@Chrico
Copy link
Member

Chrico commented Feb 8, 2023

While I understand the idea behind it...the ExternalAssetHandler::dequeue() and ExternalStyleHandler::dequeue() are never called. Those are just wrappers for 2 WP Core functions. 🤔 For me this is too much overhead. I would like to close this PR for now and take this topic into an internal discussion round at Inpsyde. Maybe we already have somewhere a soltuion flying around 😬 I'll try to keep the #19 updated.

@Chrico Chrico closed this Feb 8, 2023
@lkraav
Copy link
Author

lkraav commented Feb 8, 2023

Yeah it's fine, I ran out of steam for pushing it forward in favor of other business things.

But! Problem still remains. How do you centrally manage en- and also dequeues. Because ideally you'd want a solid Query Monitor status panel to go with it, possibly with some added metadata for dequeue reasons, etc - which calls for a systemic approach.

Is some mu-plugin the best we can do still 🤔

[wp-content/mu-plugins]# cat wp-dequeue-scripts-styles.php 
<?php
namespace CXL\MuPlugins;

add_action( 'wp_print_scripts', static function(): void {

    $dequeue = [
        'scripts' => [
            // @see https://help.metorik.com/article/76-woocommerce-source-tracking
            'metorik-js',
            'wc-aelia-foundation-classes-frontend',
            'wc-aelia-currency-switcher',

            // 'wc-memberships-frontend',
            // 'wc-memberships-blocks-common',
        ],
        'styles' => [
            'classic-theme-styles',
            'metorik-css',
            'sv-wc-payment-gateway-payment-form-v5_10_12',
            'wc-memberships-blocks',
            'wp-block-library',
        ]
    ];

    if ( ! is_admin() ) {
        array_push( $dequeue['scripts'],
            // WordPress core.
            'regenerator-runtime',
            'wp-a11y',
            'wp-dom-ready',
            'wp-hooks',
            'wp-i18n',
            'wp-polyfill',
        );
    }

    if ( ! is_checkout() ) {
        array_push( $dequeue['scripts'],
            'jquery-payment',
            'sv-wc-payment-gateway-payment-form-v5_10_12'
        );
    }

    foreach( $dequeue['scripts'] as $handle ) {
        wp_dequeue_script( $handle );
        wp_deregister_script( $handle );
    }

    foreach( $dequeue['styles'] as $handle ) {
        wp_dequeue_style( $handle );
        wp_deregister_style( $handle );
    }

}, PHP_INT_MAX );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

canEnqueue for 3rd party registered assets?
3 participants