Merge "resourceloader: Remove support for raw modules"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 27 Jun 2019 07:02:38 +0000 (07:02 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 27 Jun 2019 07:02:38 +0000 (07:02 +0000)
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderClientHtml.php
includes/resourceloader/ResourceLoaderFileModule.php
includes/resourceloader/ResourceLoaderModule.php
includes/resourceloader/ResourceLoaderStartUpModule.php
resources/Resources.php
tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
tests/phpunit/structure/ResourcesTest.php

index 387e344..ec376e3 100644 (file)
@@ -1047,9 +1047,6 @@ MESSAGE;
                        $states[$name] = 'missing';
                }
 
-               // Generate output
-               $isRaw = false;
-
                $filter = $context->getOnly() === 'styles' ? 'minify-css' : 'minify-js';
 
                foreach ( $modules as $name => $module ) {
@@ -1128,12 +1125,11 @@ MESSAGE;
                                $states[$name] = 'error';
                                unset( $modules[$name] );
                        }
-                       $isRaw |= $module->isRaw();
                }
 
                // Update module states
-               if ( $context->shouldIncludeScripts() && !$context->getRaw() && !$isRaw ) {
-                       if ( count( $modules ) && $context->getOnly() === 'scripts' ) {
+               if ( $context->shouldIncludeScripts() && !$context->getRaw() ) {
+                       if ( $modules && $context->getOnly() === 'scripts' ) {
                                // Set the state of modules loaded as only scripts to ready as
                                // they don't have an mw.loader.implement wrapper that sets the state
                                foreach ( $modules as $name => $module ) {
@@ -1142,7 +1138,7 @@ MESSAGE;
                        }
 
                        // Set the state of modules we didn't respond to with mw.loader.implement
-                       if ( count( $states ) ) {
+                       if ( $states ) {
                                $stateScript = self::makeLoaderStateScript( $states );
                                if ( !$context->getDebug() ) {
                                        $stateScript = self::filter( 'minify-js', $stateScript );
index 7f2f85f..e324d04 100644 (file)
@@ -348,7 +348,7 @@ JAVASCRIPT;
        private static function makeContext( ResourceLoaderContext $mainContext, $group, $type,
                array $extraQuery = []
        ) {
-               // Create new ResourceLoaderContext so that $extraQuery may trigger isRaw().
+               // Create new ResourceLoaderContext so that $extraQuery is supported (eg. for 'sync=1').
                $req = new FauxRequest( array_merge( $mainContext->getRequest()->getValues(), $extraQuery ) );
                // Set 'only' if not combined
                $req->setVal( 'only', $type === ResourceLoaderModule::TYPE_COMBINED ? null : $type );
@@ -434,12 +434,6 @@ JAVASCRIPT;
                                                        );
                                                }
                                        } else {
-                                               // See if we have one or more raw modules
-                                               $isRaw = false;
-                                               foreach ( $moduleSet as $key => $module ) {
-                                                       $isRaw |= $module->isRaw();
-                                               }
-
                                                // Special handling for the user group; because users might change their stuff
                                                // on-wiki like user pages, or user preferences; we need to find the highest
                                                // timestamp of these user-changeable modules so we can ensure cache misses on change
@@ -455,9 +449,15 @@ JAVASCRIPT;
                                                // Decide whether to use 'style' or 'script' element
                                                if ( $only === ResourceLoaderModule::TYPE_STYLES ) {
                                                        $chunk = Html::linkedStyle( $url );
-                                               } elseif ( $context->getRaw() || $isRaw ) {
+                                               } elseif ( $context->getRaw() ) {
+                                                       // This request is asking for the module to be delivered standalone,
+                                                       // (aka "raw") without communicating to any mw.loader client.
+                                                       // Use cases:
+                                                       // - startup (naturally because this is what will define mw.loader)
+                                                       // - html5shiv (loads synchronously in old IE before the async startup module arrives)
+                                                       // - QUnit (needed in SpecialJavaScriptTest before async startup)
                                                        $chunk = Html::element( 'script', [
-                                                               // In SpecialJavaScriptTest, QUnit must load synchronous
+                                                               // The 'sync' option is only supported in combination with 'raw'.
                                                                'async' => !isset( $extraQuery['sync'] ),
                                                                'src' => $url
                                                        ] );
index 7093ab1..017b399 100644 (file)
@@ -140,9 +140,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
        /** @var bool Link to raw files in debug mode */
        protected $debugRaw = true;
 
-       /** @var bool Whether mw.loader.state() call should be omitted */
-       protected $raw = false;
-
        protected $targets = [ 'desktop' ];
 
        /** @var bool Whether CSSJanus flipping should be skipped for this module */
@@ -305,7 +302,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                                        break;
                                // Single booleans
                                case 'debugRaw':
-                               case 'raw':
                                case 'noflip':
                                        $this->{$member} = (bool)$option;
                                        break;
@@ -513,13 +509,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                return $contents;
        }
 
-       /**
-        * @return bool
-        */
-       public function isRaw() {
-               return $this->raw;
-       }
-
        /**
         * Disable module content versioning.
         *
@@ -620,7 +609,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        'templates',
                        'skipFunction',
                        'debugRaw',
-                       'raw',
                ] as $member ) {
                        $options[$member] = $this->{$member};
                }
@@ -1004,7 +992,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        || $this->dependencies
                        || $this->messages
                        || $this->skipFunction
-                       || $this->raw
                );
                return $canBeStylesOnly ? self::LOAD_STYLES : self::LOAD_GENERAL;
        }
index a7fee85..c376fa7 100644 (file)
@@ -335,17 +335,6 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface {
                return 'local';
        }
 
-       /**
-        * Whether this module's JS expects to work without the client-side ResourceLoader module.
-        * Returning true from this function will prevent mw.loader.state() call from being
-        * appended to the bottom of the script.
-        *
-        * @return bool
-        */
-       public function isRaw() {
-               return false;
-       }
-
        /**
         * Get a list of modules this module depends on.
         *
index 2e7c869..2959b22 100644 (file)
@@ -254,10 +254,11 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                                continue;
                        }
 
-                       if ( $module->isRaw() ) {
-                               // Don't register "raw" modules (like 'startup') client-side because depending on them
-                               // is illegal anyway and would only lead to them being loaded a second time,
-                               // causing any state to be lost.
+                       if ( $module instanceof ResourceLoaderStartUpModule ) {
+                               // Don't register 'startup' to the client because loading it lazily or depending
+                               // on it doesn't make sense, because the startup module *is* the client.
+                               // Registering would be a waste of bandwidth and memory and risks somehow causing
+                               // it to load a second time.
 
                                // ATTENTION: Because of the line below, this is not going to cause infinite recursion.
                                // Think carefully before making changes to this code!
@@ -341,13 +342,6 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                return $out;
        }
 
-       /**
-        * @return bool
-        */
-       public function isRaw() {
-               return true;
-       }
-
        /**
         * @private For internal use by SpecialJavaScriptTest
         * @since 1.32
index 9c26986..4446d0d 100644 (file)
@@ -2825,7 +2825,6 @@ return [
                'scripts' => [
                        'resources/lib/html5shiv/html5shiv.js'
                ],
-               'raw' => true,
        ],
 
        /* EasyDeflate */
index 99f5e1b..bc7cb69 100644 (file)
@@ -182,23 +182,6 @@ mw.loader.register( [
         "util",
         "{blankVer}"
     ]
-] );',
-                       ] ],
-                       [ [
-                               'msg' => 'Omit raw modules from registry',
-                               'modules' => [
-                                       'test.raw' => new ResourceLoaderTestModule( [ 'isRaw' => true ] ),
-                                       'test.blank' => new ResourceLoaderTestModule(),
-                               ],
-                               'out' => '
-mw.loader.addSource( {
-    "local": "/w/load.php"
-} );
-mw.loader.register( [
-    [
-        "test.blank",
-        "{blankVer}"
-    ]
 ] );',
                        ] ],
                        [ [
index 1171ebc..544afae 100644 (file)
@@ -171,9 +171,8 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
                        'simple scripts' => [ true,
                                [ 'scripts' => 'example.js' ]
                        ],
-                       'simple scripts, raw and targets' => [ true, [
+                       'simple scripts with targets' => [ true, [
                                'scripts' => [ 'a.js', 'b.js' ],
-                               'raw' => true,
                                'targets' => [ 'desktop', 'mobile' ],
                        ] ],
                        'FileModule' => [ true,
index 78e5763..a762884 100644 (file)
@@ -45,9 +45,9 @@ class ResourcesTest extends MediaWikiTestCase {
        }
 
        /**
-        * Verify that nothing explicitly depends on raw modules (such as "query").
+        * Verify that nothing depends on "startup".
         *
-        * Depending on them is unsupported as they are not registered client-side by the startup module.
+        * Depending on it is unsupported as it cannot be loaded by the client.
         *
         * @todo Modules can dynamically choose dependencies based on context. This method does not
         * test such dependencies. The same goes for testMissingDependencies() and
@@ -58,7 +58,7 @@ class ResourcesTest extends MediaWikiTestCase {
 
                $illegalDeps = [];
                foreach ( $data['modules'] as $moduleName => $module ) {
-                       if ( $module->isRaw() ) {
+                       if ( $module instanceof ResourceLoaderStartUpModule ) {
                                $illegalDeps[] = $moduleName;
                        }
                }