977463 - SourceMaps generated from commonjs-everywhere not working

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P3)

Description

[jiyinyiyong]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/34.0.1847.3 Safari/537.36

Steps to reproduce:

I was trying to debug my code with the sourcemaps generated with commonjs-everywhere
https://github.com/Cirru/cirru-parser



Actual results:


But Chrome and firefox tries to fetch the wrong mapping files.
As talked in the issue, the problem was located at the leading slash of sources property:

https://github.com/michaelficarra/commonjs-everywhere/issues/97

Probably you need to read that isssue for details.


Expected results:


In this example

```js
{
  "version": 3,
  "sources": ["/coffee/live.coffee", "/coffee/parser.coffee"],
  "sourceRoot": "..",
```

`/coffee/live.coffee` should always behave like `coffee/live.coffee` according to the protocol of sourcemaps 3

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Having '..' as the source root works fine for me. The leading / is an absolute path, so it is resolved as such. Don't think there is a bug in Firefox here.

Comment 2

[Michael Ficarra]

I disagree with this resolution. Please see [my comment][0] from [michaelficarra/commonjs-everywhere#97][1], reproduced in part here:

From the source map specification,

> 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.

and

> If the sources are not absolute URLs after prepending
> of the “sourceRoot”, the sources are resolved relative
> to the SourceMap (like resolving script src in a html
> document).

[0] https://github.com/michaelficarra/commonjs-everywhere/issues/97#issuecomment-35626341
[1] https://github.com/michaelficarra/commonjs-everywhere/issues/97

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Ah ok, yeah this is slightly more complicated; I misread before.

The spec does say to prepend, but we've always been doing path joining / url resolution because it is what is generally expected and seen in the wild. I thought I started a thread on the source map list to see if we couldn't change the spec but I can't find it so maybe I didn't.

Comment 4

[Michael Ficarra]

At the very least, the spec needs to be clarified with respect to joining of paths such as "prepending". Does that mean simple string concatenation or joining by the path separator? I've always assumed the former, which was part of the reason this bug arose.

Comment 5

[Michael Ficarra]

See also this issue in the mozilla/source-map library: https://github.com/mozilla/source-map/issues/62

Comment 6

[ovidiu boca[:Ovidiu]]

Hi,

Can you reproduce this issue with the new version of Firefox?

Comment 7

[ovidiu boca[:Ovidiu]]

Meanwhile,  James Long can you take a look at this?
Thank you.

Comment 8

[James Long (:jlongster)]

I've tried to change how we use `sourceRoot` before but it broke some other stuff. I'm not sure I'm the best person to actively work on this. Nick, is this still an issue? I can help talk through what we might be able to change.

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

"Web compatibility" (or, source-maps-in-the-wild compatibility) requires doing URI resolving.

Comment 10

[Tom Tromey :tromey]

From reading the source, Chrome seems to follow what the spec says with

https://src.chromium.org/viewvc/blink/trunk/Source/devtools/front_end/sdk/SourceMap.js#l290

That 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;


It took me a while but I managed to set up a test case locally where chrome fetches the
original sources but firefox does not.

Given that: (1) this is what the spec says, (2) firefox's source-map story has generally been
behind the times, and (3) chrome implemented the spec, I think our best choice now is to
change our implementation.

Comment 11

[Tom Tromey :tromey]

This is going to be fixed upstream, then landed in M-C via one of the bundle updates.

Comment 12

[Tom Tromey :tromey]

I believe this is fixed now.