Conversation
WordPress provides no straightforward way to control asset output order. Further more, this library abstract enqueue related actions where priority isn't reachable. When multiple scripts or styles are enqueued, their order in the HTML depends on registration order, which is hard to manage when scattered accross plugins, themes, etc. This adds a priority property to assets (lower = earlier, WP style). The `AssetCollection` now sorts assets by priority before returning them, ensuring predictable output order regardless of registration timing.
|
Hi @nlemoine I get your concerns and the implementation is not so complex in terms of maintanance but I do have some problems to the fact that the Priority implementation reach the Another concerns is the false assumption that scripts would load in that particular order due to the The order in case of elements of the same priority isn't guarantee in php version lower than 8.0.0. In particular, related to the I'm not against the changes but I think make sense to document the fact that the order cannot be guaranteed due to the aforementioned characteristics of browsers, php and WordPress. Other than that, if the changes are fine for @Chrico are also fine for me. |
|
Thank you @widoz for your thorough review! I hear your concerns and I thought I anticipated most of them with the "preamble" in my PR comment but I realize it wasn't crystal clear, sorry about that. Let me address each point more clearly.
To clarify: this feature controls HTML output order (the order These are distinct concepts. A developer may want critical CSS to appear before other styles in the markup, or need specific tag ordering for non-deferred scripts. That's a valid use case independent of how browsers handle However, I recognize the naming can be confusing and lead to misunderstanding. Would renaming the method/prop help clarifying the intent?
This is true-WordPress may reorder scripts based on registered dependencies. However, that's a WordPress-level concern that happens after registration. This feature controls what we can control: the order assets are registered via For assets without cross-dependencies (or with dependencies already satisfied), registration order directly affects output order.
The current implementation handles this correctly. In
PHP 7.x reached EOL in November 2022. That said, if the library still supports PHP 7.x, we can make the sort deterministic by adding a tiebreaker: return $priorityA <=> $priorityB
?: $assetA->handle() <=> $assetB->handle();This ensures assets with equal priority are sorted alphabetically by handle—predictable across all PHP versions.
Script modules are subject to the same WordPress-level reordering as dependencies. But if this is a concern, we could limit the I'll try to come with some real use cases as soon as I can. It's always a better starting point to discuss implementation details. Let me know what you think, I'll be happy to refactor it according to your preferences. |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
Preambule
This feature is not to be confused with:
async,defer, etc.This must be considered all asset location/attributes equal.
Problem
WordPress provides no straightforward (besides hook priority) way to control asset output order. Further more, this library abstract enqueue related actions where hook priority isn't reachable.
When multiple scripts or styles are enqueued, their order in the HTML depends on registration order, which can be hard to manage when scattered across plugins, themes, etc.
Example:
Although adding different priorities to
AssetManager::ACTION_SETUPwill work, it's not very convenient and will eventually fail or become unmaintainable when registering multiple assets with different priorities within the same hook.Having priority defined per asset feels way more natural and maintainable approach.
What is the new behavior (if this is a feature change)?
This PR adds a priority property to assets (lower = earlier, WP style). The
AssetCollectionnow sorts assets by priority before returning them, ensuring predictable output order regardless of registration timing.Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No. I created a
PrioritizedAssetinterface to avoid changing theAssetinterface. So this feature is completely backward compatible.Other information
You can take a look at Capo.js for ideal head order as a reason for this feature.