From 9316f2e3c38cade1213aa9060f5bc26516d8fb77 Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Thu, 21 Feb 2019 17:26:23 -0800 Subject: [PATCH] resourceloader: Change 'packageFiles' format to be JSON-compatible The module definition format for 'packageFiles', as initially designed, mixes sequential and associative arrays. This works in PHP, but not in JSON. To make the format JSON compatible, use a 'name' key instead of using the key in the main array. Leave backwards compatibility in place so that extensions using the old format can be migrated. This will be removed in the next commit. Before: 'packageFiles' => [ 'script.js', 'script2.js', 'config.json' => [ 'config' => [ 'Foo', 'Bar' ] ], 'data.json' => [ 'callback' => function () { ... } ], ], After: 'packageFiles' => [ 'script.js', 'script2.js', [ 'name' => 'config.json', 'config' => [ 'Foo', 'Bar' ] ], [ 'name' => 'data.json', 'callback' => function () { ... } ], ], This can then be written in extension.json as: "packageFiles": [ "script.js", "script2.js", [ "name": "config.json", "config": [ "Foo", "Bar" ] ], [ "name": "data.json", "callback: [ "MyExtHooks", "getData" ] ] ] Change-Id: Ic566a1cd7efd075c380bc50ba0cc2c329a2041d7 --- .../ResourceLoaderFileModule.php | 48 +++++++++---------- resources/Resources.php | 14 +++--- .../ResourceLoaderFileModuleTest.php | 30 +++++------- 3 files changed, 44 insertions(+), 48 deletions(-) diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php index 4444b1357e..66807778f1 100644 --- a/includes/resourceloader/ResourceLoaderFileModule.php +++ b/includes/resourceloader/ResourceLoaderFileModule.php @@ -204,12 +204,11 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { * // If 'packageFiles' is set, 'scripts' cannot also be set * 'packageFiles' => [ * [file path string], // or: - * [file alias] => [file path string], // or: - * [file alias] => [ 'file' => [file path string], 'type' => 'script'|'data' ], // or: - * [file alias] => [ 'content' => [string], 'type' => 'script'|'data' ], // or: - * [file alias] => [ 'callback' => [callable], 'type' => 'script'|'data' ], // or: - * [file alias] => [ 'config' => [ [config var name], ... ], 'type' => 'data' ], // or: - * [file alias] => [ 'config' => [ [JS name] => [PHP name] ], 'type' => 'data' ], + * [ 'name' => [file name], 'file' => [file path], 'type' => 'script'|'data' ], // or: + * [ 'name' => [name], 'content' => [string], 'type' => 'script'|'data' ], // or: + * [ 'name' => [name], 'callback' => [callable], 'type' => 'script'|'data' ], + * [ 'name' => [name], 'config' => [ [config var name], ... ], 'type' => 'data' ], + * [ 'name' => [name], 'config' => [ [JS name] => [PHP name] ], 'type' => 'data' ], * ], * // Modules which must be loaded before this module * 'dependencies' => [module name string or array of module name strings], @@ -1091,25 +1090,25 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { $mainFile = null; foreach ( $this->packageFiles as $alias => $fileInfo ) { - // Alias is optional, but only when specfiying plain file names (strings) - if ( is_int( $alias ) ) { - if ( is_array( $fileInfo ) ) { + if ( is_string( $fileInfo ) ) { + $fileInfo = [ 'name' => $fileInfo, 'file' => $fileInfo ]; + } elseif ( !isset( $fileInfo['name'] ) ) { + // Backwards compatibility + if ( !is_numeric( $alias ) ) { + $fileInfo['name'] = $alias; + } else { $msg = __METHOD__ . ": invalid package file definition for module " . - "\"{$this->getName()}\": key is required when value is not a string"; + "\"{$this->getName()}\": 'name' key is required when value is not a string"; wfDebugLog( 'resourceloader', $msg ); throw new MWException( $msg ); } - $alias = $fileInfo; - } - if ( !is_array( $fileInfo ) ) { - $fileInfo = [ 'file' => $fileInfo ]; } // Infer type from alias if needed - $type = $fileInfo['type'] ?? self::getPackageFileType( $alias ); + $type = $fileInfo['type'] ?? self::getPackageFileType( $fileInfo['name'] ); $expanded = [ 'type' => $type ]; if ( !empty( $fileInfo['main'] ) ) { - $mainFile = $alias; + $mainFile = $fileInfo['name']; if ( $type !== 'script' ) { $msg = __METHOD__ . ": invalid package file definition for module " . "\"{$this->getName()}\": main file \"$mainFile\" must be of type \"script\", not \"$type\""; @@ -1126,15 +1125,15 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { if ( is_callable( $fileInfo['callback'] ) ) { $expanded['content'] = $fileInfo['callback']( $context ); } else { - $msg = __METHOD__ . ": invalid callback for package file \"$alias\"" . + $msg = __METHOD__ . ": invalid callback for package file \"{$fileInfo['name']}\"" . " in module \"{$this->getName()}\""; wfDebugLog( 'resourceloader', $msg ); throw new MWException( $msg ); } } elseif ( isset( $fileInfo['config'] ) ) { if ( $type !== 'data' ) { - $msg = __METHOD__ . ": invalid use of \"config\" for package file \"$alias\" in module " . - "\"{$this->getName()}\": type must be \"data\" but is \"$type\""; + $msg = __METHOD__ . ": invalid use of \"config\" for package file \"{$fileInfo['name']}\" " . + "in module \"{$this->getName()}\": type must be \"data\" but is \"$type\""; wfDebugLog( 'resourceloader', $msg ); throw new MWException( $msg ); } @@ -1144,16 +1143,17 @@ class ResourceLoaderFileModule extends ResourceLoaderModule { } $expanded['content'] = $expandedConfig; } elseif ( !empty( $fileInfo['main'] ) ) { - // 'foo.js' => [ 'main' => true ] is shorthand - $expanded['filePath'] = $alias; + // [ 'name' => 'foo.js', 'main' => true ] is shorthand + $expanded['filePath'] = $fileInfo['name']; } else { - $msg = __METHOD__ . ": invalid package file definition for \"$alias\" in module " . - "\"{$this->getName()}\": one of \"file\", \"content\", \"callback\" or \"config\" must be set"; + $msg = __METHOD__ . ": invalid package file definition for \"{$fileInfo['name']}\" " . + "in module \"{$this->getName()}\": one of \"file\", \"content\", \"callback\" or \"config\" " . + "must be set"; wfDebugLog( 'resourceloader', $msg ); throw new MWException( $msg ); } - $expandedFiles[$alias] = $expanded; + $expandedFiles[$fileInfo['name']] = $expanded; } if ( $expandedFiles && $mainFile === null ) { diff --git a/resources/Resources.php b/resources/Resources.php index ec7da1cf21..1edfdd3eb7 100644 --- a/resources/Resources.php +++ b/resources/Resources.php @@ -1191,7 +1191,7 @@ return [ 'remoteBasePath' => "$wgResourceBasePath/resources/src", 'packageFiles' => [ 'mediawiki.ForeignStructuredUpload.js', - 'config.json' => [ 'config' => [ 'UploadDialog' ] ], + [ 'name' => 'config.json', 'config' => [ 'UploadDialog' ] ], ], 'dependencies' => [ 'mediawiki.ForeignUpload', @@ -1344,7 +1344,7 @@ return [ 'remoteBasePath' => "$wgResourceBasePath/resources/src", 'packageFiles' => [ 'mediawiki.util.js', - 'config.json' => [ 'config' => [ 'FragmentMode' ] ], + [ 'name' => 'config.json', 'config' => [ 'FragmentMode' ] ], ], 'dependencies' => [ 'jquery.accessKeyLabel', @@ -1591,7 +1591,7 @@ return [ 'remoteBasePath' => "$wgResourceBasePath/resources/src/mediawiki.jqueryMsg", 'packageFiles' => [ 'mediawiki.jqueryMsg.js', - 'parserDefaults.json' => [ 'callback' => function ( ResourceLoaderContext $context ) { + [ 'name' => 'parserDefaults.json', 'callback' => function ( ResourceLoaderContext $context ) { $tagData = Sanitizer::getRecognizedTagData(); $allowedHtmlElements = array_merge( array_keys( $tagData['htmlpairs'] ), @@ -1636,7 +1636,7 @@ return [ 'remoteBasePath' => "$wgResourceBasePath/resources/src/mediawiki.language", 'packageFiles' => [ 'mediawiki.language.names.js', - 'names.json' => [ 'callback' => function ( ResourceLoaderContext $context ) { + [ 'name' => 'names.json', 'callback' => function ( ResourceLoaderContext $context ) { return Language::fetchLanguageNames( $context->getLanguage(), 'all' ); } ], ], @@ -1823,7 +1823,7 @@ return [ 'dm/ItemModel.js', 'dm/SavedQueriesModel.js', 'dm/SavedQueryItemModel.js', - 'config.json' => [ 'config' => [ 'StructuredChangeFiltersLiveUpdatePollingRate' ] ], + [ 'name' => 'config.json', 'config' => [ 'StructuredChangeFiltersLiveUpdatePollingRate' ] ], ], 'dependencies' => [ 'mediawiki.String', @@ -1877,7 +1877,7 @@ return [ 'ui/RclTargetPageWidget.js', 'ui/RclToOrFromWidget.js', 'ui/WatchlistTopSectionWidget.js', - 'config.json' => [ 'callback' => 'ChangesListSpecialPage::getRcFiltersConfigVars' ], + [ 'name' => 'config.json', 'callback' => 'ChangesListSpecialPage::getRcFiltersConfigVars' ], ], 'styles' => [ 'styles/mw.rcfilters.mixins.less', @@ -2423,7 +2423,7 @@ return [ 'remoteBasePath' => "$wgResourceBasePath/resources/src/mediawiki.legacy", 'packageFiles' => [ 'protect.js', - 'config.json' => [ 'config' => [ 'CascadingRestrictionLevels' ] ], + [ 'name' => 'config.json', 'config' => [ 'CascadingRestrictionLevels' ] ], ], 'dependencies' => 'jquery.lengthLimit', 'messages' => [ 'protect-unchain-permissions' ] diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php index fbddfb6f72..0a4cf1e108 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php @@ -405,7 +405,7 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { $base + [ 'packageFiles' => [ 'script-comment.js', - 'script-nosemi.js' => [ 'main' => true ] + [ 'name' => 'script-nosemi.js', 'main' => true ] ], 'deprecated' => 'Deprecation test', 'name' => 'test-deprecated' @@ -431,8 +431,8 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { [ $base + [ 'packageFiles' => [ - 'init.js' => [ 'file' => 'script-comment.js', 'main' => true ], - 'nosemi.js' => 'script-nosemi.js' + [ 'name' => 'init.js', 'file' => 'script-comment.js', 'main' => true ], + [ 'name' => 'nosemi.js', 'file' => 'script-nosemi.js' ], ] ], [ @@ -452,13 +452,13 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { [ $base + [ 'packageFiles' => [ - 'foo.json' => [ 'content' => [ 'Hello' => 'world' ] ], + [ 'name' => 'foo.json', 'content' => [ 'Hello' => 'world' ] ], 'sample.json', - 'bar.js' => [ 'content' => "console.log('Hello');" ], - 'data' => [ 'type' => 'data', 'callback' => function ( $context ) { + [ 'name' => 'bar.js', 'content' => "console.log('Hello');" ], + [ 'name' => 'data.json', 'callback' => function ( $context ) { return [ 'langCode' => $context->getLanguage() ]; } ], - 'config' => [ 'type' => 'data', 'config' => [ + [ 'name' => 'config.json', 'config' => [ 'Sitename', 'wgVersion' => 'Version', ] ], @@ -478,11 +478,11 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { 'type' => 'script', 'content' => "console.log('Hello');", ], - 'data' => [ + 'data.json' => [ 'type' => 'data', 'content' => [ 'langCode' => 'fy' ] ], - 'config' => [ + 'config.json' => [ 'type' => 'data', 'content' => [ 'Sitename' => $config->get( 'Sitename' ), @@ -507,7 +507,7 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { [ $base + [ 'packageFiles' => [ - 'foo.json' => [ 'callback' => 'functionThatDoesNotExist142857' ] + [ 'name' => 'foo.json', 'callback' => 'functionThatDoesNotExist142857' ] ] ], false @@ -515,7 +515,7 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { [ $base + [ 'packageFiles' => [ - 'foo' => [ 'type' => 'script', 'config' => [ 'Sitename' ] ] + 'foo.json' => [ 'type' => 'script', 'config' => [ 'Sitename' ] ] ] ], false @@ -523,7 +523,7 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { [ $base + [ 'packageFiles' => [ - 'foo.js' => [ 'config' => 'Sitename' ] + [ 'name' => 'foo.js', 'config' => 'Sitename' ] ] ], false @@ -548,11 +548,7 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase { $base + [ 'packageFiles' => [ 'script-nosemi.js', - 'foo.json' => [ - 'type' => 'data', - 'content' => [ 'Hello' => 'world' ], - 'main' => true - ] + [ 'name' => 'foo.json', 'content' => [ 'Hello' => 'world' ], 'main' => true ] ] ], false -- 2.20.1