michaelficarra / commonjs-everywhere

:rainbow: minimal CommonJS browser bundler with aliasing, extensibility, and source maps
BSD 3-Clause "New" or "Revised" License
158 stars 21 forks source link

Why is there leading slash in generated sourcemap file #97

Closed tiye closed 10 years ago

tiye commented 10 years ago
cjsify -o build/build.js -s build/build.map -r . coffee/live.coffee

build/build.map

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

/coffee/live.coffee will not get the correct source file.

michaelficarra commented 10 years ago

Has this caused you any actual problems, or are you just asking why it is that way? Your source map should have a sourceRoot field that is prepended to all source entries. 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

Resolving Sources

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

tiye commented 10 years ago

I also tried this one,

cjsify -o build/build.js -s build/build.map coffee/live.coffee

its results was using absolute path too.

➤➤ cat build/build.map
{"version":3,"sources":["/coffee/live.coffee","/coffee/parser.coffee"],

As I debug my code in a folder on Nginx, I do have problem in debugging this code.

michaelficarra commented 10 years ago

Your source map should still have a sourceRoot field. The default value is ., which means all files will be resolved relative to the source map. It looks like, with your directory structure, you want the -r flag to be .. or $(pwd), depending on whether you are debugging remotely or locally hosted code.

tiye commented 10 years ago

It returns error on .., and for PWD, it also generates absolute path. Neither worked.

The entire file structure can be found here(sorry for bothering you with such trival details): https://github.com/Cirru/cirru-parser.coffee http://raw.tiye.me/cirru-parser.coffee/

michaelficarra commented 10 years ago

Sorry, I was mistaken. I always use inline source maps, so I don't usually have to deal with this. I believe you should have had it right the first time. According to /src/command.coffee line 144 (inline below)

sourceMapRoot: if options.sourceMap? then (path.relative (path.dirname options.sourceMap), root) or '.'

the sourceRoot value should be .. in your case, since root is just your working directory. And indeed, when I run

cjsify a.coffee --source-map dist/a.js.map -o a.js

the source map has a sourceRoot value of ... I'm going to close this, but feel free to re-open if you can provide a simple repro case.

tiye commented 10 years ago

I'd love to use

cjsify -o build/build.js --inline-source-map -w coffee/live.coffee

instead of making a build.map file.

But the absolute path is always preventing me from mapping to the source. I don't know how to figure it out, or what's your solution debugging such code? screen shot 2014-02-21 at 9 37 04 am

michaelficarra commented 10 years ago

It's not an absolute path.

The browser should look for http://repo/cirru-parser.coffee/build/../coffee/live.coffee, which is the directory containing the source map, combined with the the sourceRoot, combined with the source path. That normalises to http://repo/cirru-parser.coffee/coffee/live.coffee. To me, it looks like a bug in your browser. Are you absolutely sure sourceRoot is being set to ..?

edit: formatting

tiye commented 10 years ago

More details here:

cjsify -o build/build.js -r ./ -s build/build.map -w coffee/live.coffee

part of build.map:

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

I found modifying sources property makes differences.

Chrome 33.0.1711.3 dev Chrome 35.0.1850.0 canary Firefox Nightly 30.0a1 (2014-02-18) Safari 7.0.1 (9537.73.11) OS X 10.9.1

Works in Safari, but Chrome and Firefox Firefox says:

Error loading source:
Could not load the source for http://repo/coffee/live.coffee.
Error: "http://repo/coffee/live.coffee" is not in the SourceMap.
Stack: SourceMapConsumer_sourceContentFor@resource://gre/modules/devtools/SourceMap.jsm:391
SourceActor.prototype._getSourceText@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/server/actors/script.js:2455
resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/promise.js:118
then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/promise.js:43
then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/promise.js:153
SourceActor.prototype.onSource@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/server/actors/script.js:2481
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1024
LDT_send/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/server/transport.js:258
makeInfallible/<@resource://gre/modules/devtools/DevToolsUtils.jsm -> resource://gre/modules/devtools/DevToolsUtils.js:80
Line: 391, column: 6

Works in Firefox and Safari, but not Chrome

Works in Firefox and Safari, but not Chrome. It's ["/coffee/live.coffee", "/coffee/parser.coffee"] by default. So there's problems in Chrome. While Safari looks quite cmopatiable... Are you using Safari?

michaelficarra commented 10 years ago

Yeah, it seems sourceRoot support is still somewhat lacking in Chrome/Firefox. We can probably compensate for these issues by stripping leading slashes from our sources and adding a trailing slash to sourceRoot if it's non-empty. This should make up for the failings of current browser source map support. I'll re-open this issue as a task to do that. We should still open/track issues on the browsers' respective issue trackers. @fitzgen: Can you help point us in the right direction for Firefox? Are you aware of the above issue?

michaelficarra commented 10 years ago

@jiyinyiyong: Try out master and let me know how that works for you.

tiye commented 10 years ago

Now it's working in Firefox, in Chrome it still fails, as expected.

michaelficarra commented 10 years ago

Seems chrome has zero sourceRoot support whatsoever. I guess we could prepend the sourceRoot onto all of our sources ourselves, always leaving sourceRoot empty, but I don't really care to do that unless someone requests it. I will either look up or create issues for Firefox/Chrome tomorrow.

tiye commented 10 years ago

In the case of using Nginx, prepeding sourceRoot to sources might be a bad idea, any code using absolute path might fail since the code doesn't know which root Nginx has set.

There's another exmaple I made with souce map compiled/demo.js.map

{
  "version": 3,
  "sources": [
    "../cirru/demo.cr"
  ],
  "sourceRoot": "./"

but it works in the same Chrome, while it appears to me that there's no difference between these two .map files.

fitzgen commented 10 years ago

If you have a test case of sourceRoot not working in firefox's devtools it would be very awesome of you to file a bug here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox&component=Developer%20Tools%3A%20Debugger

tiye commented 10 years ago

@fitzgen Bugzilla appears to be for more complicated the GitHub. Issue opened at https://bugzilla.mozilla.org/show_bug.cgi?id=977463

fitzgen commented 10 years ago

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.

tiye commented 10 years ago

@fitzgen How about this comment here? https://github.com/michaelficarra/commonjs-everywhere/issues/97#issuecomment-35626341