From 9840116ab2b3fa23f1eda490128e0b93e6c8743b Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Thu, 30 Jul 2026 12:18:50 +0200 Subject: [PATCH 1/2] Route component modules to the matching ResourceLoader queue `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 `` output. Route each module to the queue matching its type. Styles-only modules keep their render-blocking ``; 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) --- src/HooksHandler.php | 20 +++++++++- tests/phpunit/Unit/HooksHandlerTest.php | 51 +++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/HooksHandler.php b/src/HooksHandler.php index c942c4d..6cd1cf7 100644 --- a/src/HooksHandler.php +++ b/src/HooksHandler.php @@ -16,6 +16,8 @@ use MediaWiki\Hook\SetupAfterCacheHook; use MediaWiki\MediaWikiServices; use MediaWiki\Parser\Parser; +use MediaWiki\Parser\ParserOutput; +use MediaWiki\ResourceLoader\Module; use SMW\Utils\File; use StripState; @@ -246,13 +248,27 @@ public function onParserAfterParse( $parser, &$text, $stripState ): bool { continue; } foreach ( $this->getComponentLibrary()->getModulesFor( $activeComponent, 'vector' ) as $module ) { - $parser->getOutput()->addModuleStyles( [ $module ] ); - $parser->getOutput()->addModules( [ $module ] ); + $this->addModuleToMatchingQueue( $parser->getOutput(), $module ); } } return true; } + /** + * ResourceLoader discards anything but a styles-only module from the styles queue, so a module + * carrying scripts or dependencies has to go to the general queue, which delivers its styles too. + */ + private function addModuleToMatchingQueue( ParserOutput $parserOutput, string $module ): void { + $registeredModule = MediaWikiServices::getInstance()->getResourceLoader()->getModule( $module ); + + if ( $registeredModule && $registeredModule->getType() === Module::LOAD_STYLES ) { + $parserOutput->addModuleStyles( [ $module ] ); + return; + } + + $parserOutput->addModules( [ $module ] ); + } + /** * Hook: ParserFirstCallInit * diff --git a/tests/phpunit/Unit/HooksHandlerTest.php b/tests/phpunit/Unit/HooksHandlerTest.php index aa1fb6c..7b78864 100644 --- a/tests/phpunit/Unit/HooksHandlerTest.php +++ b/tests/phpunit/Unit/HooksHandlerTest.php @@ -7,9 +7,11 @@ use MediaWiki\Extension\BootstrapComponents\HooksHandler; use MediaWiki\Extension\BootstrapComponents\NestingController; use MediaWiki\Extension\BootstrapComponents\Tests\Fixtures\TestConfig; +use MediaWiki\MediaWikiServices; use MediaWiki\Output\OutputPage; use MediaWiki\Parser\Parser; use MediaWiki\Parser\ParserOutput; +use MediaWiki\ResourceLoader\Module; use PHPUnit\Framework\TestCase; use Skin; @@ -83,6 +85,55 @@ public function testOnParserAfterParseAddsOnlyTheStylesFix() { $this->assertNotContains( 'ext.bootstrap.scripts', $parserOutput->getModules() ); } + public function testOnParserAfterParseKeepsGeneralModulesOutOfTheStylesQueue() { + $parserOutput = $this->parserOutputAfterParsingWith( 'modal' ); + + $this->assertNotEmpty( $parserOutput->getModules(), 'the active component contributed no modules' ); + $this->assertSame( [], $this->modulesOfType( $parserOutput->getModuleStyles(), Module::LOAD_GENERAL ) ); + } + + public function testOnParserAfterParseKeepsStylesOnlyModulesOutOfTheGeneralQueue() { + $parserOutput = $this->parserOutputAfterParsingWith( 'modal' ); + + $this->assertNotEmpty( $parserOutput->getModules(), 'the active component contributed no modules' ); + $this->assertSame( [], $this->modulesOfType( $parserOutput->getModules(), Module::LOAD_STYLES ) ); + } + + private function parserOutputAfterParsingWith( string $componentName ): ParserOutput { + $bootstrapComponentsService = new BootstrapComponentsService( new TestConfig() ); + $bootstrapComponentsService->registerComponentAsActive( $componentName ); + $parserOutput = new ParserOutput(); + $parser = $this->createMock( Parser::class ); + $parser->method( 'getOutput' )->willReturn( $parserOutput ); + $text = ''; + + $hooksHandler = new HooksHandler( + $bootstrapComponentsService, + new ComponentLibrary(), + $this->createMock( NestingController::class ), + ); + $hooksHandler->onParserAfterParse( $parser, $text, null ); + + return $parserOutput; + } + + /** + * @param string[] $moduleNames + * @return string[] + */ + private function modulesOfType( array $moduleNames, string $type ): array { + $resourceLoader = MediaWikiServices::getInstance()->getResourceLoader(); + + return array_values( array_filter( + $moduleNames, + static function ( string $moduleName ) use ( $resourceLoader, $type ): bool { + $module = $resourceLoader->getModule( $moduleName ); + + return $module !== null && $module->getType() === $type; + } + ) ); + } + public function testOnBeforePageDisplayLoadsExtensionBootstrapOnNonProviderContentPage() { $out = $this->createMock( OutputPage::class ); $out->method( 'getModuleStyles' )->willReturn( [ 'ext.bootstrapComponents.bootstrap.fix' ] ); From e7504ed649d25ee671e7b43404fd8cea54b93480 Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Thu, 30 Jul 2026 13:14:20 +0200 Subject: [PATCH 2/2] Pin the module split by expected queue contents 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) --- docs/release-notes.md | 7 ++++ src/HooksHandler.php | 17 +++++----- tests/phpunit/Unit/HooksHandlerTest.php | 43 +++++++------------------ 3 files changed, 28 insertions(+), 39 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 08cf344..36086b9 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -1,5 +1,12 @@ ## Release Notes +### BootstrapComponents 6.1.1 + +Released on TBD + +Fixes: +* stop logging `Unexpected general module ... in styles queue.` on pages using a modal, carousel, tooltip or popover + ### BootstrapComponents 6.1.0 Released on 21-July-2026 diff --git a/src/HooksHandler.php b/src/HooksHandler.php index 6cd1cf7..dadb1dc 100644 --- a/src/HooksHandler.php +++ b/src/HooksHandler.php @@ -247,8 +247,8 @@ public function onParserAfterParse( $parser, &$text, $stripState ): bool { if ( !$this->getComponentLibrary()->isRegistered( $activeComponent ) ) { continue; } - foreach ( $this->getComponentLibrary()->getModulesFor( $activeComponent, 'vector' ) as $module ) { - $this->addModuleToMatchingQueue( $parser->getOutput(), $module ); + foreach ( $this->getComponentLibrary()->getModulesFor( $activeComponent, 'vector' ) as $moduleName ) { + $this->addModuleToMatchingQueue( $parser->getOutput(), $moduleName ); } } return true; @@ -256,17 +256,18 @@ public function onParserAfterParse( $parser, &$text, $stripState ): bool { /** * ResourceLoader discards anything but a styles-only module from the styles queue, so a module - * carrying scripts or dependencies has to go to the general queue, which delivers its styles too. + * that also carries scripts, dependencies, messages or templates has to go to the general + * queue, which delivers its styles too. */ - private function addModuleToMatchingQueue( ParserOutput $parserOutput, string $module ): void { - $registeredModule = MediaWikiServices::getInstance()->getResourceLoader()->getModule( $module ); + private function addModuleToMatchingQueue( ParserOutput $parserOutput, string $moduleName ): void { + $module = MediaWikiServices::getInstance()->getResourceLoader()->getModule( $moduleName ); - if ( $registeredModule && $registeredModule->getType() === Module::LOAD_STYLES ) { - $parserOutput->addModuleStyles( [ $module ] ); + if ( $module !== null && $module->getType() === Module::LOAD_STYLES ) { + $parserOutput->addModuleStyles( [ $moduleName ] ); return; } - $parserOutput->addModules( [ $module ] ); + $parserOutput->addModules( [ $moduleName ] ); } /** diff --git a/tests/phpunit/Unit/HooksHandlerTest.php b/tests/phpunit/Unit/HooksHandlerTest.php index 7b78864..eb60356 100644 --- a/tests/phpunit/Unit/HooksHandlerTest.php +++ b/tests/phpunit/Unit/HooksHandlerTest.php @@ -7,11 +7,9 @@ use MediaWiki\Extension\BootstrapComponents\HooksHandler; use MediaWiki\Extension\BootstrapComponents\NestingController; use MediaWiki\Extension\BootstrapComponents\Tests\Fixtures\TestConfig; -use MediaWiki\MediaWikiServices; use MediaWiki\Output\OutputPage; use MediaWiki\Parser\Parser; use MediaWiki\Parser\ParserOutput; -use MediaWiki\ResourceLoader\Module; use PHPUnit\Framework\TestCase; use Skin; @@ -85,21 +83,21 @@ public function testOnParserAfterParseAddsOnlyTheStylesFix() { $this->assertNotContains( 'ext.bootstrap.scripts', $parserOutput->getModules() ); } - public function testOnParserAfterParseKeepsGeneralModulesOutOfTheStylesQueue() { - $parserOutput = $this->parserOutputAfterParsingWith( 'modal' ); + public function testOnParserAfterParseSplitsComponentModulesByType() { + $parserOutput = $this->parserOutputWithActiveComponent( 'modal' ); - $this->assertNotEmpty( $parserOutput->getModules(), 'the active component contributed no modules' ); - $this->assertSame( [], $this->modulesOfType( $parserOutput->getModuleStyles(), Module::LOAD_GENERAL ) ); - } - - public function testOnParserAfterParseKeepsStylesOnlyModulesOutOfTheGeneralQueue() { - $parserOutput = $this->parserOutputAfterParsingWith( 'modal' ); - - $this->assertNotEmpty( $parserOutput->getModules(), 'the active component contributed no modules' ); - $this->assertSame( [], $this->modulesOfType( $parserOutput->getModules(), Module::LOAD_STYLES ) ); + $this->assertSame( + [ + 'ext.bootstrapComponents.bootstrap.fix', + 'ext.bootstrapComponents.button.fix', + 'ext.bootstrapComponents.modal.vector-fix', + ], + $parserOutput->getModuleStyles() + ); + $this->assertSame( [ 'ext.bootstrapComponents.modal.fix' ], $parserOutput->getModules() ); } - private function parserOutputAfterParsingWith( string $componentName ): ParserOutput { + private function parserOutputWithActiveComponent( string $componentName ): ParserOutput { $bootstrapComponentsService = new BootstrapComponentsService( new TestConfig() ); $bootstrapComponentsService->registerComponentAsActive( $componentName ); $parserOutput = new ParserOutput(); @@ -117,23 +115,6 @@ private function parserOutputAfterParsingWith( string $componentName ): ParserOu return $parserOutput; } - /** - * @param string[] $moduleNames - * @return string[] - */ - private function modulesOfType( array $moduleNames, string $type ): array { - $resourceLoader = MediaWikiServices::getInstance()->getResourceLoader(); - - return array_values( array_filter( - $moduleNames, - static function ( string $moduleName ) use ( $resourceLoader, $type ): bool { - $module = $resourceLoader->getModule( $moduleName ); - - return $module !== null && $module->getType() === $type; - } - ) ); - } - public function testOnBeforePageDisplayLoadsExtensionBootstrapOnNonProviderContentPage() { $out = $this->createMock( OutputPage::class ); $out->method( 'getModuleStyles' )->willReturn( [ 'ext.bootstrapComponents.bootstrap.fix' ] );