Wrong sourceRoot handling?

[sokra]

From the spec:

Line 4: An optional source root, useful for relocating source files on a server or removing repeated values in the “sources” entry. This value is prepended to the individual entries in the “source” field.

The spec say that the sourceRoot is prepended, but mozilla/source-map concat it with an additional slash /.

[fitzgen]

Just want to note, I am at Front Trends this week, and am taking the following week off, so it might be a little while until I look into this.

[fitzgen]

Sorry I forgot about this.

Doing a quick glance, it looks like you are correct that we aren't following the spec, but I think this is a case where we should change the spec. I think that users expect that if their sourceRoot is http://example.com/js and they include a app/foo.js they should end up with http://example.com/js/app/foo.js. I will make a proposal to the source map list in the next couple days and postpone changing any of this code until we figure out what's the right thing to do.

[lydell]

I think that the spec should say that each source should be resolved against the sourceRoot.

Since #93, sourceRoot: "http://example.com/js", sources: ["/foo/bar.js"] would result in "http://example.com/foo/bar.js".

[michaelficarra]

Ah, this is very related: https://bugzilla.mozilla.org/show_bug.cgi?id=977463

[lydell]

Resolving sources against the sourceRoot is far more flexible, in my opinion:

sourceRoot: "http://example.com/static/coffee/",
sources: [
  "one.coffee", "two.coffee", ..., "eleven.coffee",
  "../js/legacy.js",
  "/version.js",
  "http://jquery.com/jquery.js"
]

would become:

• http://example.com/static/coffee/one.coffee
• http://example.com/static/coffee/two.coffee
• ...
• http://example.com/static/coffee/eleven.coffee
• http://example.com/static/js/legacy.js
• http://example.com/version.js
• http://jquery.com/jquery.js

instead of

• http://example.com/static/coffee/one.coffee
• http://example.com/static/coffee/two.coffee
• ...
• http://example.com/static/coffee/eleven.coffee
• http://example.com/static/coffee/../js/legacy.js
• http://example.com/static/coffee//version.js
• http://example.com/static/coffee/http://jquery.com/jquery.js

Simply prepending the sourceRoot also allows for really weird stuff. Assume that the source map is located at http://example.com/test.js.map.

sourceRoot: "/",
sources: ["/file.com"]

would result in http://file.com.

[michaelficarra]

@lydell: No, you're forgetting that you resolve the result of prepending sourceRoot, with all relative paths after the concatenation (be it by / or just regular string concatenation) being resolved relative to the source map's directory. So {sourceRoot: "/", sources: ["/file.ext"]} would not resolve to http://file.ext, but http://example.com/file.ext (after path normalisation).

[sokra]

@lydell is correct because the prefix // means protocol-relative url, so //abc will resolve to http://abc. /abc will resolve to http://example.com/abc

see RFC3986

A relative reference that begins with two slash characters is termed
a network-path reference; such references are rarely used. A
relative reference that begins with a single slash character is
termed an absolute-path reference. A relative reference that does
not begin with a slash character is termed a relative-path reference.

[michaelficarra]

Ah, I was thinking normalisation would happen first, but that makes sense. I guess that pretty much decides the concatenation issue, then. If paths are joined by /, this issue would come about any time someone used "/" or "" as sourceRoot and started a source with "/" in an attempt to give an absolute path. Path concatenation must be plain old string concatenation.

[lydell]

Note that my examples do use plain old string concatenation.

I still think that the spec should be changed to say that each source should be resolved by doing url-resolve(absolute-url-to-source-map, source-root, source).

[lydell]

However, I don’t think that the source root should be able to point to a file: sourceRoot: "/static/subdir" should point to a directory called “subdir”, not a file called “subdir”, so that the sources become /static/subdir/<source>, not /static/<source>. That is how this module currently works, but it needs to be pointed out in the spec if it is updated. I myself messed that up in a project: lydell/source-map-resolve@46ec603.

[tromey]

See https://bugzilla.mozilla.org/show_bug.cgi?id=977463#c10. What Chrome does is:

    var sourceRoot = sourceMap.sourceRoot || '';
    if (sourceRoot && !sourceRoot.endsWith('/'))
      sourceRoot += '/';
    for (var i = 0; i < sourceMap.sources.length; ++i) {
      var href = sourceRoot + sourceMap.sources[i];
      var url = Common.ParsedURL.completeURL(this._sourceMappingURL, href) || href;

I tend to think this library should do this as well. Also, it seems to me that it would be good for SourceMapConsumer to take the source map URL as a parameter, so this computation can be done by the library, and not by the consumers. I already had to work around this problem in the devtools-source-map in order to get some existing code working.

[tromey]

If you're still following this issue, I encourage you to look at the PR.

[tromey]

Well, that PR didn't work out. It tried to cut a corner but that can't work.

It's possible not to cut corners, and implement what the spec says. However this affects the API quite a bit.

My first idea was to only keep absolute source URLs. But, there are tests that indicate that users of the API expect to be able to look up relative URLs; for example the test for issue #64.

Maybe a more complicated idea is possible: keep the storage as-is, and absolutize URLs when handing them out -- but then sourceContentFor has to try to look the URL up twice.