Topic on Talk:ResourceLoader/Architecture

I was surprised by this detail listed in the "debug mode" section:

> Script resources: No longer minified, concatenated and loaded from load.php. Instead, load.php will instruct the client to request each source file directly. This makes debugging scripts easier with your browser's developer tools. Transformations such as closure don't apply, so scripts may execute in the global scope.

Relatedly, the JS coding conventions suggest that a namespace closure is mandatory around each file.

However, reading the implementation in ResourceLoader#makeLoaderImplementScript, I discovered that each file is automatically surrounded by a closure regardless of the debug flag. Also, some MediaWiki-core modules such as mediawiki.Title/Title.js are missing the closure. Experimenting with a local wiki confirms that the closure is always added around every file.

Based on this, shall we update the documentation to recommend no closures, to save a few bytes and a level of indentation?

Yes, and also no. It's complicated currently! In short:

• Yes, files of package modules don't need file closures.
• No, files of legacy modules may still need file closures.

The longer version:

[…] in ResourceLoader#makeLoaderImplementScript, I discovered that each file is automatically surrounded by a closure regardless of the debug flag.

The above is true, but the following is also true:

Debug mode: No longer […] loaded from load.php. […] the client request each file directly. This makes debugging easier. Transformations such as closure don't apply, so scripts may execute in the global scope.

Let's load the cookie module in debug mode. Here is what happens:

mw.loader.implement( "jquery.cookie@13ckt", [
    "/w/resources/lib/jquery.cookie/jquery.cookie.js?86bfb"
] );

This instructs the browser to load cookie.js directly from the source directory. It does not involve makeLoaderImplementScript or its closure maker. Debug mode promises that for a legacy module, the file names and line numbers in the browser will match your source code. This means we can't easily add a closure. This is why legacy files have global scope in debug mode. And for this reason, we have the convention of wrapping each file in their own closure. (For production this is indeed not needed, as you say.)

The mediawiki.Title module was ported to a Package module. These have can use relative file imports, virtual files, and per-file exports. These features require at least a basic form of encapsulation to work. This means we had to let go of the "debug mode is completely raw" here. It also has the happy side-effect that these files don't need a file closure. See what happens there:

mw.loader.implement( "mediawiki.Title@…", {
  main: "Title.js",
  files: {
    "Title.js": function ( require, module ) {
        …
    },
    "phpCharToUpper.json": {
        …
    }
  }
} );

See also T50886 and T85805, in which I propose to straight out this inconsistency.

Thanks for the helpful explanation. I'll drop all closures after converting a module to packageFiles, and will follow T85805 because I'm also interested in the Webpack use case. This might be worth mentioning in the coding conventions doc, but it could also wait until everything is consistent.