Page MenuHomePhabricator

Introduce ResourceLoader debug mode v2
Open, MediumPublic

Description

Motivation

As of 2015, debug mode serves makes load.php requests serially for each resource (style, scripts) of each module.

It works this way because when serving a raw JS file from disk, browsers execute the file immediately upon download, which means the only way to ensure code executes in the correct order, is to not request the next file until the previous one has completed.

This is is inefficient and slow, and makes debug mode unsuitable for large features such as VisualEditor (see blocked/parent tasks).

The other issue with this is that the execution scope is changed from local module scope, to global scope. This is the only reason that we have the convention to wrap each JS file in a closure (for files in legacy non-packageFiles modules). Because that way, it has module scope even in debug mode. For production mode the closure is redundant, given the load.php/implement() wrapper already has local scope. This module scope has been part of ResourceLoader since the very beginning in 2010, and is similar to what Node.js, RequireJS, Rollup etc. do nowadays.

Requirements
  • JavaScript code will execute with local scope in debug mode (same as production).
  • Modules that depend on each other will load in parallel in debug mode (same as production).
  • Modules with N JS files will require significantly fewer than N web requests to load in debug mode (close to or equal to production).
  • JavaScript code and CSS code will continue to be served without minification in debug mode, preserving all code comments and whitespace.

Proposal

debug=2 (and later by default for debug=true)

Create a fresh with a new debug mode that is much closer to production.

It will essentially just do two things at first:

  • Disable minification. This means all code comments and whitespace is preserved. It also means the development server won't need to perform as much minification, which helps in slow VM-based environments like MediaWiki-Vagrant.
  • Disable multi-module batching. This means each module will load in its own module=… request.

In practical terms, this means debug mode will behave for all modules similar to how it behaves for packageFiles-modules today already. In other words, the only thing we lose is raw file paths. So instead of an error being attributed to ve.ui.FooBar#onClick() on line 150 of ve/ui/Foo/FooBar.js, it would be attributed to ve.ui.FooBar#onClick() of modules=ext.visualEditor.

Depending on your workflow, this might not make much difference. E.g. in most text editors, a file can be quickly opened by typing cmd-P/ctrl-P, enter "ve ui FooBar" and the relevant file is there, etc.

Also, if you've been using production mode as-is for debugging (like me), then this will have been how you open files anyhow. The difference will be that you'd now get code formatting back without needing to use the in-browser "Prettify" button in DevTools, and get code comments as well within the browser.

Long term, we will get line numbers back with source maps (T47514). But in the interest of moving this forward one step at a time, I've changed this proposal from the original version to not include source maps initially.

debug=1

If some people prefer the old debug mode we have today (with its flaws) over the above proposal, I'd be open to maintaining that a little longer (at least until source maps are ready). Which you could opt-in, and possibly rename to debug=old after flipping the defaut. Let me know in the comments below if this is something you think you'd make use of.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Krinkle raised the priority of this task from Low to Medium.Aug 18 2018, 6:20 AM
Krinkle moved this task from Backlog to Accepted Enhancement on the MediaWiki-ResourceLoader board.

Inspect the code (that causes an error) in non-debug mode is already pretty easy given browser's pretty formatters. But then trying to figure out where it comes from, I usually enable debug mode to get the filename (and a line number). This is also required for meaningful stracktraces (required for bug reports) as the function names are ambiguous without them. Similarly when stepping through code, debug mode is required to better understand what I am looking at, and not having to pretty-format each file. Also file blacklisting doesn't work well in non-debug mode.

Disable minification

It would be nice to have this functionality for projects using Webpack too. One solution is to use debugScripts (example in Popups).

Disable minification

It would be nice to have this functionality for projects using Webpack too. One solution is to use debugScripts (example in Popups).

If the original non-minified source is given to RL, then both the old and new debug mode will not minify it. That'll work out of the box.

What is it that you'd like RL to make work with Webpack?

What is it that you'd like RL to make work with Webpack?

Ship production Webpack-bundled assets by default (this is the current behavior). Ship different, development-only, Webpack-bundled assets when debug=1. These assets, for example, may have special debugging capabilities that could be determined at compile time and stripped from production sources. (Maybe not the best example, but here's a runtime test in Popups.)

If we use packagefiles feature in RL, none of those source files will be listed in devtools(and map to a source file in filesytem) even in debug mode. The current Vue integration without build step relies on packagefiles and not having a proper client side debugging support makes it hard for developers. Is this a known problem and is there any solution?

What is it that you'd like RL to make work with Webpack?

Ship production Webpack-bundled assets by default (this is the current behavior). Ship different, development-only, Webpack-bundled assets when debug=1. These assets, for example, may have special debugging capabilities that could be determined at compile time and stripped from production sources. (Maybe not the best example, but here's a runtime test in Popups.)

Ah okay. I guess I should clarify that debugScripts (to add debug-only files) and packageFiles's closure support (to replace a prod file) are not going anywhere. Do those cover your needs currently?

For packageFiles nothing is planned to change as part of this task. For legacy modules, when using debug=1, the unminified files (or debugScripts files) would now be concatenated and served from load.php instead of directly from /w/extensions/… like today (as requested by developers). And debug=old would behave the same as today even for legacy modules – if there is demand for that option.

I should clarify that debugScripts (to add debug-only files) and packageFiles's closure support (to replace a prod file) are not going anywhere. Do those cover your needs currently?

I think so! However, it would be very nice to avoid the need for custom entry points. It's possible to clumsily workaround but I think it would be a lot nicer if exclusively scripts OR debugScripts assets were shipped instead of both. In general, I want a completely different version of the same code (either a debug or production build) not a production build with debug sprinkled on top. The latter is the current behavior. I would rather intentionally require production sources from debug than intentionally exclude them.

For packageFiles nothing is planned to change as part of this task.

I think packageFiles are less applicable when using an external bundler. A debug-only target seems like it could be useful to add flexibility. I don't have any need for it at the moment but wanted to mention it.

Edit: similar PHP workaround for debugScripts:

	'vue' => [
		'packageFiles' => [
			'resources/src/vue/index.js',
			'resources/src/vue/i18n.js',
			[
				'name' => 'resources/lib/vue/vue.js',
				'callback' => function ( ResourceLoaderContext $context, Config $config ) {
					// Use the development version if development mode is enabled, or if we're in debug mode
					$file = $config->get( 'VueDevelopmentMode' ) || $context->getDebug() ?
						'resources/lib/vue/vue.common.dev.js' :
						'resources/lib/vue/vue.common.prod.js';
					return new ResourceLoaderFilePath( $file );
				}
			],

		],
		'targets' => [ 'desktop', 'mobile' ],
	],

^It'd be awesome to be able to keep configuration data as JSON files strictly in skins and extensions.

Change 597912 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Add internal handling for debug=2

https://gerrit.wikimedia.org/r/597912

Change 597912 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Add internal handling for debug=2

https://gerrit.wikimedia.org/r/597912

Change 715295 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] resourceloader: Skip version hash calculation in debug mode

https://gerrit.wikimedia.org/r/715295

Change 715296 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] resourceloader: Always split requests in debug mode

https://gerrit.wikimedia.org/r/715296

Change 721287 had a related patch set uploaded (by Krinkle; author: Kosta Harlan):

[mediawiki/core@master] resourceloader: Add setting for controlling minification

https://gerrit.wikimedia.org/r/721287

Krinkle renamed this task from ResourceLoader: Redesign how debug mode operates to Introduce ResourceLoader debug mode v2.Sep 15 2021, 5:34 PM

Change 715295 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Skip version hash calculation in debug mode

https://gerrit.wikimedia.org/r/715295

Change 721287 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] ResourceLoader: Add setting for controlling minification

https://gerrit.wikimedia.org/r/721287

Change 723620 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] resourceloader: Preserve new 'debug' param in getScriptURLsForDebug()

https://gerrit.wikimedia.org/r/723620

Change 723620 merged by jenkins-bot:

[mediawiki/core@master] resourceloader: Preserve new 'debug' param in getScriptURLsForDebug()

https://gerrit.wikimedia.org/r/723620