Page MenuHomePhabricator

Popups use of sourceMappingURL breaks production debugging
Closed, ResolvedPublic2 Estimated Story Points

Description

Currently, any page on which Popups loads has most of its debuggability compromised due to the inclusion of an invalid //# sourceMappingURL statement half-way in the combined debug=false bundle pointing to a file that doesn't exist (/w/index.js.json).

I understand its purpose is to, in debug mode, allow source maps to work (which is pretty useful). However for all other code in production that is not Popups, no source maps are involved, and generally in day-to-day work, production mode is a better debug mode (Yeah, T85805 for more).

Please ensure this comment is removed in production mode. Right now this is not happening because of the inclusion of a @nomin instruction in the dist build (added in 7bd29bb). I'm not sure why that is, as it appears to directly (and only) lead to this bug :-)

See also:

Event Timeline

Since this is interfering with other teams ability to debug and easy to fix we should fix this...

Jdlrobson set the point value for this task to 2.Oct 4 2017, 5:49 PM

As mentioned in the description, removing the @nomin banner could also work well giving the sourcemap comment on debug=true and having jsmin remove it on debug=false.

Change 383705 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Do not include @nomin instruction in dist build

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

Change 383705 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Do not include @nomin instruction in dist build

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

🎉🎉🎉

I can confirm that:

  • I see the //DevTools failed to parse SourceMap: https://en.wikipedia.org/wiki/index.js.json// message on enwiki (currently on the -wmf.3 branch)
  • I don't see the message on cawiki (group 1, currently on the -wmf.4 branch)
  • I can navigate the Popups source, set breakpoints etc, when in debug mode