Page MenuHomePhabricator

Spike: Is it possible to get source maps for Extension:Popups in RL production mode?
Closed, DeclinedPublic

Description

Context

Initially Extension:Popups added a nomin banner to the JS in a module to avoid minification to maintain the source-maps comment on the JS file.

We had to remove it because apparently it caused problems with other RL modules in production mode:

As a result, we get source maps in development mode debug=true, but not in production because the source-map comment is removed.

Question

  • Is it possible to get source maps for Extension:Popups in RL production mode?

Possible investigations, areas to research, questions to answer

  • Can we disable minification (JSMinifier) on just one RL module without impacting other modules?
  • Can we improve RL to understand source-map comments and leave them alone?
  • Are the source maps comments correct in production mode or would they need their line numbers shifted?

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 23 2018, 10:30 AM

Can we disable minification (JSMinifier) on just one RL module without impacting other modules?

No.
The problem stems from the load.php URL which can string together any number of modules e.g. load.php?modules=module1|module2
Check out ResourceLoader::applyFilter to see how minification of assets works!

Can we improve RL to understand source-map comments and leave them alone?

Maybe, but given we are the only user, this might be hard. I'd also want buy-in from Timo.

Can we improve RL to understand source-map comments and leave them alone?

Ideally RL would implement its own source maps (basically the suggestion in T47514) with an optional override. Then for Popups we'd override it like so:

'module'=> [ 'sourcemap' => '/path/to/actual/sourcemap/file.map' ]

Can we disable minification (JSMinifier) on just one RL module without impacting other modules?

No.

Actually, yes, we can. The minification happens per-module, not per web response. Minification can be disabled for a module through the /*@nomin*/ instruction (typically prepended). Popups used to do this, and in itself actually worked just fine. Minification of other modules wasn't affected.

The problem with Popups was that its source file ends with //# sourceMappingURL=index.js.json. This is a problem because Source Maps, as implemented in web browsers, are a way of associating pieces of a script with different pieces of source code. "Script" in this context, refers to anything loaded through <script>, so basically the web server response. These mapping pieces are typically identified by an arbitrary string offset within the current script's source code. For example var a,b=1; could map offset 0-5 to var a;, and map offset 6-9 to var b = 1, or something like that.

Somewhat similar to HTTP headers, there can only be one set of mappings for one web response. And when the popups code is included with the larger load.php response, the source map reference ends up being a part of that (unless the sourceMappingURL comment instruction is stripped by minification). When this happens, debugging becomes a mess because while the json file claims to provide mappings for the "current script", they don't make sense. The result is that console log messages and stack traces get attributed to seemingly random code. E.g. code from before Popups loaded gets attribute to Popups, code from Popups gets attributes to code loaded after Popups, and code loaded after Popups gets attributes to -1/null or some other error (beyond last mappable offset).

Similar to other forms of aggregation (again, e.g. HTTP headers), we'd need SourceMap-specific code to scan the source files for existing source maps, and combine these maps into one larger maps, store it somewhere, and expose it through some url. Ideas for that are part of T47514, but this is a fairly complex thing, and we have yet to find out whether it is even doable within our performance constraints.

I suspect what we'll end up doing is keep production mode as-is, but replace our debug mode with something that is basically production mode + source maps.

Jhernandez updated the task description. (Show Details)Feb 26 2018, 11:23 AM

Actually, yes, we can. The minification happens per-module, not per web response. Minification can be disabled for a module through the /*@nomin*/ instruction (typically prepended). Popups used to do this, and in itself actually worked just fine. Minification of other modules wasn't affected.
The problem with Popups was that its source file ends with //# sourceMappingURL=index.js.json. This is a problem [...]
there can only be one set of mappings for one web response. [...]

Right, thanks for clarifying!

I'll add a comment on the codebase to keep track of why we shouldn't have the @nomin banner and close this task for the moment.

Change 414644 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
[mediawiki/extensions/Popups@master] Hygiene: Add comment about @nomin and source-maps in production

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

Jhernandez closed this task as Declined.Feb 26 2018, 11:40 AM

I'm going to decline this as we have talked about it and there isn't much we can do right now.

Change 414644 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: Add comment about @nomin and source-maps in production

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