fix(check): include script dependencies in size accounting - #1414
fix(check): include script dependencies in size accounting#1414faisalahammad wants to merge 4 commits into
Conversation
The Enqueued_Scripts_Size_Check only counted scripts whose src started with the audited plugin's URL, leaving external dependencies like jQuery uncounted in the page-load total. This splits handles into two buckets: plugin-owned and external dependencies. Plugin-owned warnings keep the existing 'EnqueuedScriptsSize.ScriptSizeGreaterThanThreshold' code for backward compatibility. A new 'EnqueuedScriptsSize.ExternalDependencySize' warning fires when the combined total exceeds the threshold, naming both bucket sizes and the dependency handle count. External script sources are resolved against the local filesystem via a small private helper that supports includes_url and content_url prefixes. CDN sources that resolve to null are counted as zero-byte dependencies to keep the warning path consistent. Fixes WordPress#74
|
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. |
- Extract get_inline_script_size() helper to bring check_url() under 100 LOC - Remove $allow_plugin boolean flag from script_src_to_path() — method always tries all path prefixes Errors fixed: - ExcessiveMethodLength: check_url() was 103 LOC - BooleanArgumentFlag: script_src_to_path() bool param PHP 7.4+ compatible. All CI checks passing. Refs WordPress#1414
|
Thanks for the PR! I found this with Codex’s help:
As a result, unrelated page assets can be attributed to the audited plugin and trigger false-positive size warnings. Could we start from the plugin-owned handles and recursively collect only their actual dependencies? It would also be helpful to add a test that enqueues an unrelated external script on the same page. |
|
Good catch, thanks. Fixed:
All 9 |
|
Where is the commit? |
check_url() previously counted every script in wp_scripts()->done whose src was not plugin-owned as a dependency of the audited plugin. Scripts enqueued by the active theme, core, or other plugins could trip false-positive ExternalDependencySize warnings. Walk instead only the plugin-owned handles in done, BFS through their $script->deps transitively, restricted to handles actually loaded. Anything in done but unreachable from the plugin's dep graph is excluded. Add a regression fixture + test that registers an unrelated external script alongside a plugin-owned script to confirm the dep warning no longer fires. Refs WordPress#1414 Refs WordPress#74
faisalahammad
left a comment
faisalahammad
left a comment
There was a problem hiding this comment.
Review feedback addressed in commit 3f75a1e:
- check_url() no longer counts every non-plugin-owned script in wp_scripts()->done as a dep. It BFS-walks only the plugin-owned handles transitively through $script->deps, restricted to handles that actually loaded.
- New fixture tests/phpunit/testdata/plugins/test-plugin-enqueued-script-size-check-with-unrelated-external-script/ + test_run_no_false_positive_on_unrelated_external_scripts assert both warning codes stay at 0 when an unrelated CDN script is enqueued alongside the plugin script.
- Extracted dep-graph walk into private collect_relevant_handles() helper.
Quality gates: 9/9 phpunit tests pass, phpcs/phpmd/phpstan clean.
Could reviewers take another look when convenient? I could not re-request reviewers via API from this contributor fork; flagging here so the maintainers can see the push.
2 participants
What?
Closes #74
The Enqueued_Scripts_Size_Check now accounts for external script dependencies (e.g. jQuery, CDN-hosted libraries) alongside plugin-owned scripts.
Why?
The previous check only summed sizes for scripts whose
srcstarted with the audited plugin's URL. Page-load JS weight was underreported whenever a plugin enqueued a script with external dependencies.How?
Two-bucket accounting in
check_url():EnqueuedScriptsSize.ScriptSizeGreaterThanThresholdwarning per file, unchanged behavior.EnqueuedScriptsSize.ExternalDependencySizewarning emitted once per URL when the combined total exceeds the threshold. Message names both bucket sizes and the dependency handle count.A small private helper
script_src_to_path()resolves non-plugin URLs to a local filesystem path when possible (supportsincludes_url()andcontent_url()prefixes). CDN sources that resolve to null are counted as zero-byte dependencies so the warning path stays consistent.Testing Instructions
composer test -- --filter "Enqueued_Scripts_Size_Check_Tests"— 8 tests, 23 assertions, all pass.composer lintandcomposer phpstan— both clean.wp plugin check enqueued_scripts_size --plugin=your-pluginagainst a plugin that enqueues a script with a jQuery dep. Expect the newEnqueuedScriptsSize.ExternalDependencySizewarning when the combined total exceeds the threshold.AI Usage Disclosure
AI-assisted: used for code generation and test scaffolding.
Screenshots or screencast