-
Notifications
You must be signed in to change notification settings - Fork 58
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
Implement Enqueued_Scripts_Scope_Check
#485
Conversation
* @throws Exception Thrown when preparation fails. | ||
*/ | ||
public function prepare() { | ||
$orig_scripts = isset( $GLOBALS['wp_scripts'] ) ? $GLOBALS['wp_scripts'] : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified to:
$orig_scripts = isset( $GLOBALS['wp_scripts'] ) ? $GLOBALS['wp_scripts'] : null; | |
$orig_scripts = $GLOBALS['wp_scripts'] ?? null; |
But not required.
$demo_posts = array_map( | ||
static function ( $post_type ) { | ||
return array( | ||
'post_title' => "Demo {$post_type} post", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Advised by PhpStorm:
'post_title' => "Demo {$post_type} post", | |
'post_title' => "Demo $post_type post", |
'update_post_term_cache' => false, | ||
); | ||
|
||
$the_query = new \WP_Query( $args ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If use WP_Query
is added to the top:
$the_query = new \WP_Query( $args ); | |
$the_query = new WP_Query( $args ); |
$the_query->the_post(); | ||
|
||
$urls[] = get_permalink(); | ||
$post = get_post(); | ||
$taxonomy_names = get_post_taxonomies( $post ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$the_query->the_post(); | |
$urls[] = get_permalink(); | |
$post = get_post(); | |
$taxonomy_names = get_post_taxonomies( $post ); | |
$the_query->next_post(); | |
$urls[] = get_permalink( $the_query->post ); | |
$taxonomy_names = get_post_taxonomies( $the_query->post ); |
|
||
/* Restore original Post Data */ | ||
wp_reset_postdata(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now unneeded if we don't call the_post()
above.
/* Restore original Post Data */ | |
wp_reset_postdata(); |
if ( isset( $plugin_script['count'] ) && ( $url_count === $plugin_script['count'] ) ) { | ||
$this->add_result_warning_for_file( | ||
$result, | ||
__( 'This script is being loaded in all contexts.', 'plugin-check' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it a problem if a script is loaded in all contexts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not always a problem, but often times this is done in error and not actually on purpose. This can also be often observed in the wp-admin, when a plugin loads JS on every page instead of only its own settings page.
This check is aimed to detect the former, but of course it can't be bulletproof. Hence this is a warning and not an error.
@westonruter Just noting that I agree with the suggestions, but since they would need to be done in |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Note: this is basically an exact copy of
Enqueued_Styles_Scope_Check
right now. And the other script/style-related checks are very similar too.I suggest creating some abstraction in a new, separate PR to remove repeated code.
Fixes #23