Support » Plugin: commonWP » Minified wp/gh files

  • Hi Milan,

    First off, I’d like to say, good job on this plugin!

    I’ve dabbled with this kind of thing myself, although at a much more basic level.

    function enqueue_jsdelivr( $src, $handle ) {
    
        if ( strpos(  $src, includes_url() ) !== false ) {
    
            global $wp_version;
            return explode( '?', str_replace( includes_url(),'https://cdn.jsdelivr.net/gh/wordpress/wordpress@'.$wp_version.'/wp-includes/', $src ))[0];
        }
    
        return $src;
    }
    
    add_filter( 'script_loader_src', 'enqueue_jsdelivr', 10, 2 );
    add_filter( 'style_loader_src', 'enqueue_jsdelivr', 10, 2 );

    Having a flick through the code, I can see references to getting the minified file from npm.

    I know its early in the release of the plugin, but have you or would you consider taking advantage of jsdelivr’s ability to generate minified files if none exist.

    https://cdn.jsdelivr.net/wp/plugins/wordfence/tags/7.1.12/css/wordfenceBox.1536768948.css
    and
    https://cdn.jsdelivr.net/wp/plugins/wordfence/tags/7.1.12/css/wordfenceBox.1536768948.min.css
    as an example?

    I understand that this might not be something everyone would want, but perhaps adding something like define(‘commonwp_minify_all’, true); to wp-config.php could be a way for those who wish to toggle it on?

    Thanks 🙂

    • This topic was modified 2 months, 3 weeks ago by  michaelrhmw.
    • This topic was modified 2 months, 3 weeks ago by  michaelrhmw.
    • This topic was modified 2 months, 3 weeks ago by  michaelrhmw.
    • This topic was modified 2 months, 3 weeks ago by  michaelrhmw.
Viewing 9 replies - 1 through 9 (of 9 total)
  • +1 for minified fetching. Would love to see this happen sooner rather than later.

    Great idea, you should submit as an issue:

    https://github.com/dimadin/commonWP/issues

    +1 was thinking the same

    • You might name the constant something like COMMONWP_MINIFY for some consistency with WP constants. Not a big deal.
    • Just an idea: It might be nice to check for the commonWP constant in addition to a generic constant of some sort that other plugin authors might share. As an example, the Amazon Web Services plugin checks for the existence of either DBI_SECRET_ACCESS_KEY or AWS_SECRET_ACCESS_KEY. Perhaps like COMMONWP_MINIFY and JSDELIVR_MINIFY … (or something else reasonable, unique, yet generic)

    (PS, don’t forget to give it a 5-star rating folks!)

    FYI – I’ve created https://github.com/dimadin/commonWP/issues/1 for this.

    FYI – I’ve created https://github.com/dimadin/commonWP/issues/1 for this.

    Nice, although it links directly to my comment rather than this page in general.

    Plugin Author Milan Dinić

    (@dimadin)

    Hi all, thanks for your suggestion and sorry for late reply, I needed some time to write detailed explanation, and to think about pros and cons and possible implementation.

    First, note that this feature wasn’t there during development of commonWP, it was enabled just last week.

    There are some things to consider. Like I wrote when explaining how plugin works in its READMEs, it compares local file and remote file (file on jsDelivr) if they are identical. When there is minified file only on jsDelivr, there can’t be comparison because there is no local minified file.

    Because of the way automatic minification on jsDelivr works, minified file can be larger than original in some cases. For example, files might not show in their name that they are already minified. jsDelivr will insert its header and that will make new file bigger. Example: original, automatically minified. Or, for JavaScript files, jsDelivr will insert both header and sourcemaps information, and if original file is already small, minified might be larger. Example: original, automatically minified. Or, when applying compression, for small files difference might be minor, just tens of bytes. Example with brotli: original, automatically minified.

    Also, it is recommended by jsDelivr to not use subresource integrity when doing minification.

    I can’t enable this by default, because I can’t be 100% sure that automatic minification was successful, I can’t bring users to situations where commonWP causes their sites to stop working correctly even though I know I use technique that might cause this, I advertised and promised that commonWP is as secure as possible (and I really believe that this should be the case).

    Personally, I strongly believe that this is kind of things that developers of plugins/themes should already enable. I know that maintaining two copies of files was painful but there are now build tools for help. (Core does this: they only have development copies but they have build tool that does minification for release.)
    Developers should be more responsible with things they put on WP.org repositories, things should be optimized for that environment and general, production usage.

    All being said, I’ll add experimental feature for this that is optional and disabled by default, and I will ask you to test it for any issues, but note that it might take a while before I finish it.

    Hi Milan,

    Thanks for the reply.
    I agree that it should be a completely opt in feature.

    In the greater/wide spread use, it wont matter if they are minified or not, provided they are cached, and that would be great, let us all dream of that day!

    But until then, it might be useful in some cases.

    You could possibly have something like, a const that needs to be defined for the feature to be active.

    Then the normal check that checks to see if the local and remote file match, could also check for the minified version, which could then check the file size of the both .min.x and .x files and compare to see if the .min.x is actually smaller, if it is use it. then I guess you would disable the integrity check (can this be done on a per file basis?)

    Thanks Milan.
    -Michael.

    Plugin Author Milan Dinić

    (@dimadin)

    Hey guys, I committed changes for this in separate branch on GitHub and you are welcome to test and report any issues. If you don’t use git or Composer versions, just replace content of /inc/Process.php file with one from here (you can do this via built-in plugin editor).

    Then, no matter how you start to use this, you must enable this optional feature. It follows approach I already used in commonWP, which is using WordPress hooks, not constants. You need one liner whenever you save custom code:

    add_filter( 'commonwp_use_dynamically_minified', '__return_true' );

    You can make custom callback that accepts second parameter, object of Process class so you can fine-tune it per file.

    So, how this works:

    • At begging, everything works as usual, so original file must be exists and must be equal both locally and remotely, and standard logic and hooks is used.
    • Then, after standard processing is completed, method that tries to use minified file is used:
    • It checks if SCRIPT_DEBUG constant is true, skip in that case.
    • It checks if true is passed to commonwp_use_dynamically_minified filter, because this is optional.
    • It checks if file already doesn’t end with .min.js for scripts or .min.css for styles.
    • It checks if dynamically minified remote file exists (returns 200 status header).
    • It checks that bytes size of remote file is lower that bytes size of original, local file. (Without any compression taken into account.)
    • It checks if jsDelivr says that it skipped minification for file.

    Word of caution here: even though bytes size of dynamically minified file is lower, for small files (1-2KB or less) it is possible that after compression is applied (brotli, gzip), original unminified file is actually smaller than minified one. You should test and see what are actual results with files you use. (This is simple: in a new browser tab, open Networks tab in Developer Tools, open minified and unminified file, do force refresh, and look at Transferred and Size columns.)

    Thanks Milan.

Viewing 9 replies - 1 through 9 (of 9 total)
  • You must be logged in to reply to this topic.