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

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index 4444b13..6680777 100644 (file)
--- 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 ec7da1c..1edfdd3 100644 (file)
--- 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 fbddfb6..0a4cf1e 100644 (file)
--- 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