Route component modules to the matching ResourceLoader queue - #126
Draft
malberts wants to merge 2 commits into
Draft
Route component modules to the matching ResourceLoader queue#126malberts wants to merge 2 commits into
malberts wants to merge 2 commits into
Conversation
`onParserAfterParse` added every active component's modules to both queues. ResourceLoader accepts only styles-only modules in the styles queue, so `modal.fix`, `carousel.fix`, `tooltip.fix` and `popover.fix` each logged `Unexpected general module "..." in styles queue.` on every page view that activated their component, and had their stylesheet dropped from the render-blocking `<head>` output. Route each module to the queue matching its type. Styles-only modules keep their render-blocking `<link>`; modules carrying scripts or dependencies go to the general queue, which delivers their styles too. Simply dropping the `addModuleStyles()` call also silences the error, but it demotes the styles-only modules to JS-delivered CSS, trading log noise for a flash of unstyled content on elements visible at first paint. Splitting the combined modules into separate styles and script modules would additionally restore render-blocking CSS for the four affected components. Deferred: #90 is set to redesign this loading mechanism and #118 flags dead Bootstrap 3 selectors in several of these stylesheets, so the split is better made once that audit has settled. The hardcoded `'vector'` argument and the unused `$skin` on the adjacent lines are likewise left to #90. The tests assert the queue-type invariant in both directions rather than naming individual modules, so a later module split leaves them passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two queue-type invariants asserted absence only, so they stayed green when the styles-only modules were dropped altogether. Assert the expected contents of each queue instead, which pins delivery as well as routing. The invariant form also claimed more than holds: `AbstractComponent::augmentParserOutput` puts every module of a rendered component into the general queue regardless of type, so "no styles-only module in the general queue" is true of this hook alone, not of a real render. Name the module parameter for what it holds, compare against null explicitly, and widen the note to the other options that make a module non-styles-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
onParserAfterParseadded every active component's modules to both queues. ResourceLoader accepts onlystyles-only modules in the styles queue, so
modal.fix,carousel.fix,tooltip.fixandpopover.fixeachlogged
Unexpected general module "..." in styles queue.on every page view that activated their component.The first three also lost their stylesheet from the render-blocking
<head>output;popover.fixcarries nostylesheet, so for it only the log line was affected.
Route each module to the queue matching its type. Styles-only modules keep their render-blocking
<link>;modules carrying scripts or dependencies go to the general queue, which delivers their styles too.
Dropping the
addModuleStyles()call instead also silences the error, but it demotes the styles-only modulesto JS-delivered CSS, trading log noise for a flash of unstyled content on elements visible at first paint.
Splitting the four combined modules into separate styles and script modules is what would restore
render-blocking CSS for them. That is not done here and is not currently tracked: #90 covers the per-skin
resolution mechanism and #118 the stylesheet contents, so neither carries it. The hardcoded
'vector'argument and the unused
$skinon the adjacent lines do belong to #90.No remaining call site can put a general module into the styles queue, so the logged error is gone for
good: the only other additions there are
ext.bootstrap.stylesandext.bootstrapComponents.bootstrap.fix,both styles-only.
The inverse case is untouched.
Hooks/OutputPageParserOutputqueuesext.bootstrapComponents.vector-fixwithaddModules(), and that module is declared for no component, so no path ever puts it in the styles queue. Itsrules are page-wide (
html { font-size },body.skin-vector *, *::before, *::after { box-sizing: initial }),so on Vector they take effect only once the module has loaded.
AbstractComponent::augmentParserOutput()alsoadds a rendered component's modules with
addModules()regardless of type, through aParserOutputHelperbound to whichever parser reached
ParserFirstCallInitfirst; those additions are not visible in the queues ofa rendered page.