Skip to content

Count script dependencies toward the enqueued size total (#74) - #1488

Open
gunjanjaswal wants to merge 3 commits into
WordPress:trunkfrom
gunjanjaswal:fix/74-count-script-dependencies
Open

gunjanjaswal wants to merge 3 commits into
WordPress:trunkfrom
gunjanjaswal:fix/74-count-script-dependencies

Conversation

@gunjanjaswal

@gunjanjaswal gunjanjaswal commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

What?

Enqueued_Scripts_Size_Check now counts a plugin script's dependencies toward the cumulative page size, not just the scripts served from the plugin's own directory.

Fixes #74

Why?

The check only measured scripts whose src started with the plugin's own URL. Anything a plugin script depends on but that's served from elsewhere, jQuery being the obvious one, was skipped entirely, even though it loads on the page because the plugin enqueued something that needs it. That undercounts the real weight the plugin adds to a page.

How?

  • For each of the plugin's enqueued scripts, walk its dependency graph (->deps, transitively, de-duplicated) and add every dependency's size to the page total.
  • Resolve each handle's src to a local file with a load_script_textdomain-style mapper that understands the plugin directory, wp-content, wp-includes, and the site root (including root-relative and protocol-relative srcs). Scripts served from another host, such as a CDN, can't be measured locally, so they're skipped rather than guessed at.
  • Warnings still point only at the plugin's own files, since those are what the author controls; dependencies just contribute to the total that trips the threshold.

I kept this scoped to the scripts check to keep the diff reviewable. Enqueued_Styles_Size_Check has the same shape and the same gap, so I'm happy to follow up by converging the two into a shared base (as you suggested in the issue) and applying the dependency accounting there too, either in this PR or a separate one, whichever you prefer.

AI Usage Disclosure

  • This PR includes AI-assisted code or content

AI assistance (Claude Code) was used to explore the check and draft the dependency-walking and URL-to-path logic and the test. I reviewed the change, ran the check's PHPUnit tests and PHPCS locally, and take responsibility for what's here.

Testing Instructions

  1. Build a plugin that enqueues a script which declares a dependency served from outside the plugin, e.g. wp_enqueue_script( 'my-script', plugins_url( 'my-script.js', __FILE__ ), array( 'jquery' ) ).
  2. Run Plugin Check's performance checks with a script-size threshold below the combined size of your script plus jQuery, but above your script alone.
  3. Before this change the check stays quiet (jQuery isn't counted); after it, the cumulative total includes the dependency and the threshold warning fires.

Automated: the new test_run_counts_external_dependencies case in Enqueued_Scripts_Size_Check_Tests.php sets a threshold between the plugin script's own size and its size once a dependency is included, and asserts the warning only appears because the dependency is counted.

Open WordPress Playground Preview

Enqueued_Scripts_Size_Check only measured scripts whose src started with
the plugin's own URL, so any dependency served from elsewhere (jQuery, a
shared vendor library) was ignored even though it loads because of the
plugin and adds to the page weight.

Walk each plugin script's dependency graph, de-duplicated, and resolve
every handle's src to a local file with a load_script_textdomain-style
mapper (plugin dir, wp-content, wp-includes, site root). Dependencies
count toward the page total; scripts served from another host, such as a
CDN, are skipped since their size can't be read locally. Warnings still
point only at the plugin's own files.

Fixes WordPress#74.
@github-actions

Copy link
Copy Markdown
Contributor

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: gunjanjaswal <gunjanjaswal@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: jjgrainger <joegrainger@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Inline dependency payloads remain undercounted, and the required dependency-size reporting is missing.

Review effort: Balanced
Findings: 3 Medium severity · 1 Low severity

Open (4)
What changed in this PR

Updates the runtime script-size check to include measurable dependencies in cumulative page weight.

Changes:

  • Traverses and de-duplicates script dependencies.
  • Maps local script URLs to filesystem paths.
  • Adds an uploads-based dependency fixture and PHPUnit coverage.
File Description
includes/​Checker/​Checks/​Performance/​Enqueued_Scripts_Size_Check.php Adds dependency traversal, path resolution, and size accounting.
tests/​phpunit/​tests/​Checker/​Checks/​Enqueued_Scripts_Size_Check_Tests.php Tests direct external dependency accounting.
tests/​phpunit/​testdata/​plugins/​test-plugin-enqueued-script-size-check/​load.php Registers the external dependency fixture.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread includes/Checker/Checks/Performance/Enqueued_Scripts_Size_Check.php Outdated
Comment on lines +250 to 254
/* translators: 1: script file size. 2: tested URL. 3: threshold file size. */
__( 'This script has a size of %1$s which in combination with the other scripts enqueued on %2$s exceeds the script size threshold of %3$s.', 'plugin-check' ),
size_format( $plugin_script['size'] ),
size_format( $measured[ $handle ]['size'] ),
$url,
size_format( $this->threshold_size )
Comment on lines +12 to +16
$uploads = wp_upload_dir();
$dep_file = trailingslashit( $uploads['basedir'] ) . 'plugin-check-external-dependency.js';

if ( ! file_exists( $dep_file ) ) {
file_put_contents( $dep_file, str_repeat( 'a', 1000 ) );
Comment on lines +119 to +123
public function test_run_counts_external_dependencies() {
// Load the test plugin, whose script depends on a 1000 byte script served from outside
// the plugin directory.
require UNIT_TESTS_PLUGIN_DIR . 'test-plugin-enqueued-script-size-check/load.php';

- Use ABSPATH . 'wp-includes/' instead of the WPINC constant, which the
  PHPStan config doesn't have stubbed.
- Count a dependency's inline before/after code even when its file can't
  be resolved locally (e.g. a CDN script); WordPress still prints it.
- Exercise a transitive dependency and a remote (skipped) dependency in
  the fixture, and clean up the fixture's uploads files on tear down.
@gunjanjaswal

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Pushed b9cf12b to address it:

  • Inline payload on unresolvable deps — good catch. When a dependency's file can't be resolved (a CDN script, say), the inline before/after code is still printed, so it's now counted regardless of whether the file itself is measurable.
  • PHPStan — the CI failure was Constant WPINC not found, since it isn't stubbed here. Switched to ABSPATH . 'wp-includes/'.
  • Fixture cleanup — the uploads files the fixture writes are now removed on tear_down().
  • Test coverage — the fixture now also exercises a transitive dependency (the direct dep depends on a second file) and a remote dependency that can't be measured locally and is skipped. The dependency test's threshold sits between the size with only the direct dep and the size once the transitive one is included, so recursive traversal is actually asserted.

On the suggestion to track plugin vs dependency sizes separately and report both in the warning: I read #74 as asking for dependencies to be counted "as part of the total script size for a page load," which is what this does, rather than a change to what the warning reports. Happy to add a separate dependency breakdown if a maintainer wants it, but I left the warning format as-is to keep the change focused.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The warning omits the separate dependency and total-size reporting required by issue #74.

Review effort: Balanced
Findings: 2 Medium severity · 2 Low severity

Open (4)
Resolved since last review (1)

public function test_run_counts_external_dependencies() {
// The fixture's script depends on a 1000 byte script served from the uploads directory,
// which in turn depends on a 500 byte transitive one, plus a remote (CDN) script that
// can't be measured locally and must be skipped rather than fataling.
@gunjanjaswal

Copy link
Copy Markdown
Contributor Author

Thanks for running the review again, @ernilambar. Quick status on where things stand after the latest push (3e35f29): CI is green, and the re-review cleared three of the four findings — the inline payload is now counted even when a dependency's file can't be resolved, the fixture cleans up its uploads files on tear down, and the fixture/tests now exercise a transitive dependency and a remote (CDN) skip, with the threshold sitting between the direct-only and transitive totals so the recursive traversal is actually asserted.

The one remaining item is the suggestion to report the dependency contribution separately in the warning. I read #74 as asking for dependencies to count toward the total (which they now do) rather than a change to the message, so I left the wording alone. That said, I'm happy to add a breakdown if you'd find it useful for triage, something like:

This script has a size of 40 KB which in combination with the other scripts enqueued on {URL} exceeds the script size threshold of 300 KB. Dependencies account for 180 KB of the total.

Want me to add that line (shown only when dependencies actually contribute), or keep the message as-is? Happy to go whichever way you prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enqueued_Scripts_Size_Check includes script dependencies

2 participants