Extra: Sniff to check for overruling of WP included scripts

[jrfnl]

WP ships with a number of scrips by default. If a theme or plugin wants to use one of those scripts they should use the version included with WP and not ship their own version.

Ref: https://developer.wordpress.org/reference/functions/wp_register_script/#core-registered-scripts

While this will never be something for which we can get a 100% detection - as people can rename the script files and use arbitrary handle names -, should we try to detect if a theme/plugin tries to enqueue their own script instead of the WP version ?

I imagine we could sniff the function parameters of the wp_enqueue_scripts function and:

• throw an error if one of the reserved handles is used in combination with a value for the scr parameter
• throw a warning if the filename in the scr parameter closely resembles the expected filename of one of the scripts WP ships with.

Additionally we could throw a warning if a deps parameter with one of the default values for a reserved handle is found as there is no need to register that dependency (already done by WP core).

Opinions ?

I imagine that the Theme Review theme and the VIP theme might be interested in including such a sniff as well ? /cc @grappler @sboisvert @david-binda

[grappler]

In the WPTRT for we have a PR open that disallows deregistering core scripts that is used to overwrite the scripts. https://github.com/WPTRT/WordPress-Coding-Standards/pull/26

The most common case that I have seen this happening is with Masonry and and imagesloaded.

The problem that I see with a partial match is that have seen scripts that extend masonry have masonry in the handle and file name. I think we would have to live with a warning instead of an error unless the file matches masonry.pkgd.js or masonry.pkgd.min.js.

I don't fully understand what you mean here:

Additionally we could throw a warning if a deps parameter with one of the default values for a reserved handle is found as there is no need to register that dependency (already done by WP core).

[jrfnl]

@grappler I have seen too many plugins to count which ship with their own version of jQuery causing havoc for other plugins and themes. That's what this sniff would try to prevent.

The Theme sniff to prevent deregistering of core scripts could be used as an additional check, it is related to this, but does not address the same issue.

The problem that I see with a partial match is that have seen scripts that extend masonry have masonry in the handle and file name. I think we would have to live with a warning instead of an error unless the file matches masonry.pkgd.js or masonry.pkgd.min.js.

Example code showing this in more detail would be very welcome so it can be used for the unit tests !

I don't fully understand what you mean here:

Additionally we could throw a warning if a deps parameter with one of the default values for a reserved handle is found as there is no need to register that dependency (already done by WP core).

As WP already (correctly) handles those dependencies, there is no need for a plugin or theme to pass them to WP. Saves some superfluous code getting into plugins/themes which would need to be maintained while the code is not necessary in the first place.

[grappler]

Example code showing this in more detail would be very welcome so it can be used for the unit tests !

Here you go

wp_enqueue_script( 'gama-store-masonry', get_stylesheet_directory_uri() . '/js/masonry.pkgd.min.js', array('jquery'), '1' ); 
wp_enqueue_script( 'awoengine-imageloaded', get_template_directory_uri() . '/js/imagesloaded.pkgd', array(), '20151215', true );

Additionally we could check the JS file headers for jQuery.

As WP already (correctly) handles those dependencies, there is no need for a plugin or theme to pass them to WP. Saves some superfluous code getting into plugins/themes which would need to be maintained while the code is not necessary in the first place.

You mean something like this would return an error. wp_enqueue_script( 'jquery' );

[jrfnl]

@grappler

Additionally we could check the JS file headers for jQuery.

You mean check the file comments of any JS files included in a project for typical text phrases used in the headers of js libraries which WP includes ?

If so, yes, I think that might be a nice addition, though it will never be 100% as often plugins/themes include the minified version of js libraries and comments are often stripped from those.

You mean something like this would return an error. wp_enqueue_script( 'jquery' );

No, that's not what I mean. My suggestion was specifically about the deps parameter.

Maybe these examples will make it a little clearer:

// WP core has already defined the dependency for jquery-ui-datepicker, there is no need to add it.
wp_enqueue_script( 'jquery-ui-datepicker', '', array( 'jquery' ) );

// The `jquery` in this dependency array is superfluous as WP already has
// that defined for `jquery-ui-button`.
wp_enqueue_script(
    'jquery-some-other-add-on',
    plugins_url( 'js/jquery.ui.someaddon.js',
    array( 'jquery', 'jquery-ui-button' )
);

[grappler]

The examples make it better.

• Generally I think there should be no additional parameters defined when enqueuing a registered script.
• I think it will be a bit more complex to only have the absolute minimum dependencies defined. One of the complex one is jquery-masonry which depends on jquery and masonry which depends on imagesloaded.

I think we should create a separate issue for this.

[david-binda]

When reviewing client's code on WordPress.com VIP we time to time run into a custom jQuery bundled to theme or plugin, but I'm not quite sure whether those can be caught by the sniff which is being proposed here, since the jQuery is often part of a bundled and minified JS.

But I can see how something like this might be useful. I'll try to dig for some examples which could had been caught by the proposed sniff and will get back once I have something.

[jrfnl]

@david-binda Had a chance to dig up some examples yet ? Would be nice to get started on this one.