Bug Tracker

Ticket #14415 (closed feature: fixed)

Opened 13 months ago

Last modified 12 months ago

Remove sourcemap comment

Reported by: dmethvin Owned by: m_gol
Priority: blocker Milestone: 1.11/2.1
Component: build Version: 1.10.2
Keywords: Cc:
Blocking: Blocked by:

Description (last modified by dmethvin) (diff)

In the core meeting on Monday, we discussed how to deal with some of the problems that arose from adding sourcemap support:

  • Chrome spews a 404 on the console if you use the min file and it can't find the map, because sourcemaps are now enabled by default.
  • Web devs don't know that they have to now copy three files (minified, map, unminified) whenever they want to use the minified file or they'll get a 404.
  • Web devs don't realize that renaming one or all of these files is not sufficient to please the sourcemap gods, and they again get a 404.

This problem has become extremely common:

 http://stackoverflow.com/questions/18365315/jquerys-jquery-1-10-2-min-map-is-triggering-a-404-not-found  http://wordpress.org/support/topic/get-wp-adminjquery-1102minmap-404-not-found  http://www.kendoui.com/forums/kendo-ui-complete-for-asp-net-mvc/installer-vs-extensions/jquery-min-map-not-found.aspx  http://blog.clicdata.com/2013/09/12/404-error-on-chrome-when-getting-jquery-minified-map/

Much of this mess is due to limitations in the sourcemap "spec" itself, which isn't a formal W3C spec but just written in this document:

 https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit

Given the confusion above and the fact that many users are actually editing the file to avoid the 404 message, we may consider removing the sourcemap comment in the next version of jQuery.

Here are the things we'd like to see sourcemaps accommodate:

  • Allow files to be renamed in the file system but still have sourcemaps work properly if a consistent naming is used. For example, renaming jquery-1.10.2.min.js to jquery.min.js would be fine as long as the corresponding map was renamed to jquery.min.map. That means the minified and map files should *not* contain full file names in their source.
  • Make it possible to have just a local .min.js file but use public .min.map and uncompressed .js file from a CDN for example. Again this isn't possible at the moment because the map refers back to the minified file, and that will be a local file, not be the one on the CDN.
  • Outside the spec, but dev tools should not simply barf out 404 warnings to the console for this case. If the user tries to view source or set breakpoints there should be a clear message about why things are messed up.
  • Make it possible for users to specify a map file manually via a URL in dev tools. This would allow us to ship a file without a sourcemap comment but the user could add the association themselves if they use maps.

Change History

comment:1 Changed 13 months ago by dmethvin

  • Priority changed from undecided to blocker
  • Component changed from unfiled to build
  • Description modified (diff)
  • Milestone changed from None to 1.11/2.1

comment:2 Changed 13 months ago by m_gol

The list is good but I'd also like to propose a more drastic change (it might not be implemented but I still think we should at least put that out) - it's what Gibson said amongst others - to at least create an alternative way of defining the map & source files - a site would be declaring a link to the manifest file in its HTML, like:

<link rel="sourcemaps" href="path/to/manifest.json">

The manifest file would be a JSON with mappings in a format like:

"path/to/jquery-2.1.0.min.js": [ "path/to/jquery-2.1.0.min.map", "path/to/jquery-2.1.0.js" ]

The site would then download the map & source files for minified files on opening DevTools or on debugging start. The map file should not reference the minified file by name but can do it e.g. via an md5 hash (thx Gibson for the suggestion). The map file would also declare the path (full or relative) to the source file.

In this way we achieve a very important thing - a developer that either doesn't know about sourcemaps or doesn't care about it could copy the files they want and the new site just wouldn't have source map functioning but no errors would be caused by that - since the manifest is missing, DevTools wouldn't even try to find sourcemap files.

Another problem was mentioned by Dave during a meeting: current sourcemap spec breaks one important assumption - that vendor minified files can have clearly defined md5/sha1/etc. hash that lets people quickly check if their vendor file isn't hijacked in any way. Putting comments that depend on file names in the minified file breaks that assumption, a manifest approach avoids this problem completely.

To the list of wins of the approach with the manifest file is also a slight size saving for end users since they download the file without comments that are interested only to devs that do debugging.

Thoughts?

comment:3 follow-up: ↓ 4 Changed 13 months ago by jaubourg

I dunno, if we go with doing stuff in the document itself, why not add a sourcemap attribute to the script tag?

My opinion is that we shouldn't have the info in the source file itself. It involves too much (failing) trickeries when you consider file renaming.

comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 13 months ago by m_gol

Replying to jaubourg:

I dunno, if we go with doing stuff in the document itself, why not add a sourcemap attribute to the script tag?

Didn't think of that. We'd need 2 attributes, though, sourcemap and source (the latter for unminified source) so that the map file doesn't contain any names/paths.

This wouldn't work for dynamically loaded scripts having //# sourceUrl set though I'm not sure how big a concern that is.

My opinion is that we shouldn't have the info in the source file itself. It involves too much (failing) trickeries when you consider file renaming.

100% agreed.

comment:5 in reply to: ↑ 4 Changed 13 months ago by jaubourg

This wouldn't work for dynamically loaded scripts having //# sourceUrl set though I'm not sure how big a concern that is.

It would using script tag injection.

comment:6 follow-up: ↓ 7 Changed 13 months ago by gibson042

The more I think about it, the more I like fingerprinting.

  • source.min.js (or raw compressed source) starts with code indicating the hex-encoded hash of a valid map and optionally a location for it (either absolute or relative to the source location, and in both cases allowing for a "same name" syntax); e.g. //# sourceMap MD5=daef2e232e01d25520708543c48db61f URL={basename .js}.map
    • hash algorithms for which support is required are documented in the spec, and very limited
    • sourceMap parameters can be specified via out-of-band mechanisms like HTTP headers, element attributes, or manual association in developer tools
  • source.min.map is JSON like the current spec but with version > 3 and no file property, and is ignored in the case of hash mismatch
  • uncompressed source files remain unchanged

comment:7 in reply to: ↑ 6 Changed 13 months ago by m_gol

Replying to gibson042:

The more I think about it, the more I like fingerprinting.

  • source.min.js (or raw compressed source) starts with code indicating the hex-encoded hash of a valid map and optionally a location for it (either absolute or relative to the source location, and in both cases allowing for a "same name" syntax); e.g. //# sourceMap MD5=daef2e232e01d25520708543c48db61f URL={basename .js}.map

There's still the location in the file which I wanted to avoid.

  • hash algorithms for which support is required are documented in the spec, and very limited
  • sourceMap parameters can be specified via out-of-band mechanisms like HTTP headers, element attributes, or manual association in developer tools

HTTP headers could work for CDNs but not for regular devs. Manual association is not sth jQuery should care about; I'd go either for the manifest approach or attributes on script tags.

  • source.min.map is JSON like the current spec but with version > 3 and no file property, and is ignored in the case of hash mismatch

+1 for removing the file field.

  • uncompressed source files remain unchanged

comment:8 Changed 13 months ago by pmuellr

Has anyone opened a bug at crbug.com on Chrome's unwanted behaviour? Someone prolly thought this was a great feature; it would still be useful, if I was debugging problems with my sourcemaps or something, but otherwise, you don't want to see those 404's. I'd ask to make the default "quiet" with an option to log the "warnings".

Also, consider embedding the original source in the sourcemap file, in which case you only need the min.js and map file to debug. More thoughts here:  http://pmuellr.blogspot.com/2013/10/sourcemap-best-practices.html

comment:9 Changed 13 months ago by dmethvin

#14433 is a duplicate of this ticket.

comment:10 Changed 13 months ago by dmethvin

If we decide to keep sourcemaps, #14433 would be another impediment.

comment:11 Changed 13 months ago by dmethvin

comment:12 Changed 13 months ago by m_gol

@dmethvin it wouldn't be, that issue is fixed. :)

comment:13 Changed 13 months ago by timmywil

  • Status changed from new to open

comment:14 Changed 13 months ago by m_gol

  • Owner set to m_gol
  • Status changed from open to assigned

comment:15 Changed 12 months ago by m_gol

Pull request:  https://github.com/jquery/jquery/pull/1424

I've kept the source map comment for manually compiled files as they're not causing as problems; manual copying+renaming of some files from CDNs do.

comment:16 Changed 12 months ago by Michał Gołębiowski

  • Status changed from assigned to closed
  • Resolution set to fixed

Fix #14415. Remove the source map comment in the release script. Close gh-1424.

Changeset: 562145e887cc42ce8c9c9cd2c6e946ff01e6731d

Note: See TracTickets for help on using tickets.