resourceloader: Remove support for raw modules

Being a raw module means that when it is requested from load.php with
"only=scripts" set, then the output is *not* wrapped in an
'mw.loader.implement' closure *and* there no 'mw.loader.state()' appendix.
Instead, it is served "raw".

Before 2018, the modules 'mediawiki' and 'jquery' were raw modules.
They were needed before the client could define 'mw.loader.implement', and
could never be valid dependencies. Module 'mediawiki' merged to 'startup',
and 'jquery' became a regular module (T192623). Based on the architecture
of modules being deliverable bundles, it doesn't make sense for there to
ever be raw modules again. Anything that 'startup' needs should be bundled
with it. Anything else is a regular module.

On top of that, we never actually needed this feature because specifying
the 'only=scripts' and 'raw=1' parameters does the same thing.

The only special bit about marking modules (not requests) as "raw" was that
it allowed the client to forget to specify "raw=1" and the server would
automatically omit the 'mw.loader.state()' appendix based on whether the
module is marked as raw. As of Ie4564ec8e26ad53f2, the two remaining use
cases for raw responses now specify the 'raw=1' request parameter, and we
can get rid of the "raw module" feature and all the complexity around it.

== Startup module

In the startup module there was an interesting use of isRaw() that has
little to do with the above. The "ATTENTION" warning there applies to the
startup module only, not raw modules in general. This is now fixed by
explicitly checking for StartupModule.

Above that warning, it talked about saving bytes, which was an optimisation
given that "raw" modules don't communicate with mw.loader, they also don't
need to be registered there because even if mw.loader would try to load
them, the server would never inform mw.loader about the module having
arrived. There are now no longer any such modules.

Bug: T201483
Change-Id: I8839036e7b2b76919b6cd3aa42ccfde4d1247899

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 387e344..ec376e3 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -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 );

diff --git a/includes/resourceloader/ResourceLoaderClientHtml.php b/includes/resourceloader/ResourceLoaderClientHtml.php
index 7f2f85f..e324d04 100644 (file)
--- a/includes/resourceloader/ResourceLoaderClientHtml.php
+++ b/includes/resourceloader/ResourceLoaderClientHtml.php
@@ -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
                                                        ] );

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index 7093ab1..017b399 100644 (file)
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -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;
        }

diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php
index a7fee85..c376fa7 100644 (file)
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -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.
         *

diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php
index b90b618..64dfc94 100644 (file)
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -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

diff --git a/resources/Resources.php b/resources/Resources.php
index b90ead4..20fc129 100644 (file)
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -2822,7 +2822,6 @@ return [
                'scripts' => [
                        'resources/lib/html5shiv/html5shiv.js'
                ],
-               'raw' => true,
        ],
 
        /* EasyDeflate */

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
index 99f5e1b..bc7cb69 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderStartUpModuleTest.php
@@ -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}"
-    ]
 ] );',
                        ] ],
                        [ [

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
index 1171ebc..544afae 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -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,

diff --git a/tests/phpunit/structure/ResourcesTest.php b/tests/phpunit/structure/ResourcesTest.php
index 78e5763..a762884 100644 (file)
--- a/tests/phpunit/structure/ResourcesTest.php
+++ b/tests/phpunit/structure/ResourcesTest.php
@@ -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;
                        }
                }