[New sniff] Verify that wp_deregister_script() isn't called

[carolinan]

Rule:

WARNING : Themes must not deregister core scripts.

Ref: https://make.wordpress.org/themes/handbook/review/required/#stylesheets-and-scripts

"Required to use core-bundled scripts rather than including their own version of that script. For example jQuery."

This is basically meant to only check that core scripts aren't being deregistered, however maintaining a list of core scripts for that purpose would be a maintenance nightmare, so returning a warning when any such call is encountered is the current solution.

Theme check file covering this rule:

https://github.com/Otto42/theme-check/blob/master/checks/deregister.php

To do:

• [ ] Add / clarify the rule in the Theme Review handbook to the Requirements page.
• [ ] Create unit tests
• [ ] Create new sniff

[Pross]

See https://github.com/WPTRT/WordPress-Coding-Standards/pull/26

[grappler]

@WPTRT/core Can you think of any reason why a them should be able able to deregister a script? I can' think of any. If we can't think of any we could change the sniff from a WARNING to an ERROR.

[justintadlock]

There's definitely at least one reason. For example, a theme might want to run a custom media player on the front end. It could deregister or dequeue the core scripts and roll its own. I do this with styles and don't think it's a stretch to think that a theme could do it with the media player scripts.

That's all I got at the moment.

[Pross]

So is the answer to check for deregistering certain scripts?

[justintadlock]

Definitely. We can stop themes from deregistering core WP scripts (can always dequeue in my example above). Themes must have the ability to deregister parent theme and/or plugin scripts.

[Pross]

Should we include ALL scripts from https://developer.wordpress.org/reference/functions/wp_enqueue_script/#defaults or should a list be drawn up?

[justintadlock]

I'd just include the most obvious problems in an error, which are probably jquery (and maybe masonry). I don't think I've seen a theme attempt to deregister anything else.

The entire reason behind this came about because we had themes trying to deregister the core jQuery and register their own. So, the core of the problem is this:

wp_deregister_script( 'jquery'

wp_register_script( 'jquery'

Catch those two things and you've caught 99% of the problem.

[Pross]

Updated PR does exactly this ^

[jrfnl]

From: https://github.com/WPTRT/WordPress-Coding-Standards/pull/26#discussion_r70920334

Just a thought: what about keeping this list in line with the scripts WP core protects itself against from being deregistered on the admin side ? The list can be found in the code of this function: https://developer.wordpress.org/reference/functions/wp_deregister_script/

The current PR #26 checks against jquery and is about ready for merge consideration.

Is there a decision that that's enough or are what we have currently opinions and does this still have to go to the Theme Review Board for definite approval ?

/cc @grappler

[justintadlock]

I think we're probably OK with this. If we need to add more, the code already looks like it's set up for just plugging in new script handles.

[dingo-d]

The sniff for this exists: NoDeregisterCoreScript. If this is correct @jrfnl can you close this issue?