Skip to content

refactor!: cleanup Asset_Loader and add error checking#458

Merged
dkotter merged 7 commits into
WordPress:developfrom
justlevine:refactor/asset-loader
Apr 22, 2026
Merged

refactor!: cleanup Asset_Loader and add error checking#458
dkotter merged 7 commits into
WordPress:developfrom
justlevine:refactor/asset-loader

Conversation

@justlevine
Copy link
Copy Markdown
Contributor

@justlevine justlevine commented Apr 22, 2026

What?

This PR dedupes the internal logic inside of Asset_Loader and adds both admin notices and _doing_it_wrong() notices when a bad asset is enqueued.

Additionally, tests have been backfilled to provide full coverage for the class.

Why?

  • Improves DX for contributors
  • Provides a guard against breaking changes to the experimental @wordpress/build as we proceed with the eventual migration.

(On a personal note, this happened to me when I was switching branches and took me too long to notice they were still in the old build dir.)

How?

  • The unused $asset_data argument was removed (on ::enqueue_styles it was replaced with string[] $dependencies]. If there's really a need for us to enqueue an asset that's not in our build flow, we can reintroduce it then.
  • Similarly, I kept things as ::enqueue_*() instead of renaming them to ::register_*() and then using native wp_enqueue_*() when the assets are actually required, and skipped introducing an ::enqueue_script_module() since at the moment this also feels like YANGI and can be done with forward-compat.

More specific hardening changes (using consts for reusable strings, making the class final, docblock typo) in the diff.

Use of AI Tools

  • GitHub Copilot Autocomplete
  • GLM-5.1 in Opencode to debug asset registration leaking into other tests and make the necessary fix to the test tearDown()

Testing Instructions

  1. rm -rf build-scripts
  2. visit the dashboard and confirm you see an admin notice. if you have WP_DEBUG_DISPLAY on you'll also see the _doing_it_wrong. Otherwise see it in tests/_output/debug.log.
  3. Run npm run build and refesh the screen, to confirm the notice goes away.

Screenshots or screencast

image

Changelog Entry

Added - New feature.
Changed - Existing functionality.
Deprecated - Soon-to-be removed feature.
Removed - Feature.
Fixed - Bug fix.
Security - Vulnerability.
Development Update - Development related updates.

  • Changed: Refactor Asset_Loader class and add error checking when dependencies are missing.
Open WordPress Playground Preview
@justlevine justlevine requested a review from Copilot April 22, 2026 16:02
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 22, 2026

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: justlevine <justlevine@git.wordpress.org>
Co-authored-by: dkotter <dkotter@git.wordpress.org>

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors Asset_Loader to dedupe asset-loading logic and introduce admin/_doing_it_wrong() feedback when enqueued build assets are missing/invalid.

Changes:

  • Refactor Asset_Loader internals: centralize .asset.php loading, add handle prefix constant, add admin notice + _doing_it_wrong() reporting.
  • Update enqueue_style() signature to accept explicit style dependencies (rather than reading deps from .asset.php).
  • Add new integration tests covering valid/invalid/missing asset metadata scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.

File Description
includes/Asset_Loader.php Refactors enqueue/localize logic, adds centralized asset metadata loading and admin notice/error logging.
tests/Integration/Includes/Asset_LoaderTest.php Adds integration tests for enqueue/localize behavior and error handling around missing/invalid asset metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Integration/Includes/Asset_LoaderTest.php
Comment thread includes/Asset_Loader.php
Comment thread includes/Asset_Loader.php
Comment thread includes/Asset_Loader.php
Comment thread includes/Asset_Loader.php
Comment thread includes/Asset_Loader.php
Comment thread includes/Asset_Loader.php
Comment thread includes/Asset_Loader.php Outdated
Comment thread includes/Asset_Loader.php
Comment thread tests/Integration/Includes/Asset_LoaderTest.php
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.52%. Comparing base (4c9699f) to head (cab45b1).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop     #458      +/-   ##
=============================================
+ Coverage      67.72%   68.52%   +0.80%     
+ Complexity       959      958       -1     
=============================================
  Files             60       60              
  Lines           4836     4861      +25     
=============================================
+ Hits            3275     3331      +56     
+ Misses          1561     1530      -31     
Flag Coverage Δ
unit 68.52% <100.00%> (+0.80%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@dkotter dkotter merged commit bff4c86 into WordPress:develop Apr 22, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from Needs review to Done in WordPress AI Planning & Roadmap Apr 22, 2026
@justlevine justlevine deleted the refactor/asset-loader branch April 22, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants