Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source maps #42

Open
guybedford opened this issue Jan 14, 2013 · 43 comments
Open

Source maps #42

guybedford opened this issue Jan 14, 2013 · 43 comments

Comments

@guybedford
Copy link
Contributor

This is not an urgent request - just some consideration as to how this could be implemented.

CoffeeScript Redux supports source maps generation (Ryan Florence has a good write up here http://ryanflorence.com/2012/coffeescript-source-maps/)

It does support the browser environment as well, but needs the source-maps dependency to generate the source map. The source-map module is fully AMD compatible in the browser which is a big bonus.

So it may take some work to get CoffeeScriptRedux to work with source map generation in the browser within AMD, but it looks like it may be possible, perhaps with a CoffeeScriptRedux pull request or two.

Then, the question is how to populate the source map within eval'd code generated from CoffeeScript. Looking through the spec, I can't see if this is straightforward or not.

One way might be to set the source map as a data-url within the sourceMap comment. This is worth testing to see if it is supported.

Getting this source maps support is quite crucial to long-term buy in for require-cs. I'm very much a fan of the dynamic compilation that AMD implies for a development environment. If we miss crucial features like this, static compilation may win by adoption though.

@jrburke
Copy link
Member

jrburke commented Jan 15, 2013

Yeah, I don't quite understand how a multi-stage source map (CS -> JS -> minified JS) would work. I will likely need someone to research this and figure it out, and do pull requests. The source map support in 2.1.3 is what I would consider experimental, and it really just came along for the ride with the uglify2 work.

So while I am open to getting this work, someone else will need to do the heavy lifting as I have not had a personal use case that needed source maps.

@guybedford
Copy link
Contributor Author

The hard part is providing a source map to eval'ed code I think - I will do some tests and see if I can get this to work with data-uris.

In terms of the bigger picture, I'm now using CoffeeScript quite heavily in a dev environment with cs! requires all over the place. I'd be interested to hear if you think this is a good approach, or if it would be more sensible to have a compiler running as a process in the background and then using js requires on the compiled files?

@jrburke
Copy link
Member

jrburke commented Jan 16, 2013

For the evaled code, maybe the answer there is @sourceURL that also as an @sourcemap thing.

Supporting transpiled languages is very important for a module system, something that should work without needing a precompile step. So I'm glad to have the cs plugin getting use, and I do want to work out any wrinkles with its use (requirejs/requirejs#497 for example), if for no other reason to work out the right flow for ES modules later. So, I am still committed to making it work. Source maps in general are just a hard sell for me, as they introduce a lot of complexity, and they still seem new (I don't hear much about the multi-stage maps for example).

If a precompiled cs step can work out the sourceMap translation from CS -> JS -> minified JS, then maybe we can use that knowledge to figure out the pathway here.

@guybedford
Copy link
Contributor Author

I tested the eval method as described above in a test repo here - https://github.com/guybedford/cs-source-maps (to see what is going on see https://raw.github.com/guybedford/cs-source-maps/master/test-eval.html)

I noticed that webkit seems to support the sourceMapperURL with a data-uri, only the original source isn't displayed. I've filed a bug here https://bugs.webkit.org/show_bug.cgi?id=63940#c13.

If this is sorted out, then the entire method I described above will work perfectly for debugging in a development environment. It is specifically getting full source mapping in the development environment that is the focus here.

@guybedford
Copy link
Contributor Author

I filed a proper bug here - https://bugs.webkit.org/show_bug.cgi?id=107939

@guybedford
Copy link
Contributor Author

A patch has been submitted here - https://bugs.webkit.org/show_bug.cgi?id=107939

This means that we just need to do:

  • Use CoffeeScript Redux for CoffeeScript compilation
  • Run through the process of getting it to generate a source map in the browser with the source maps module
  • Re-encode the source map as a data-uri to add to the @SourceMappingURL property comment in the eval code, along with the @SourceURL comment

Then with the above patch in webkit, full debugging would be possible during development (note it is specifically the development environment I am focusing on).

Firefox still doesn't support the @SourceURL comment in evalled code as far as I can tell (https://bugzilla.mozilla.org/show_bug.cgi?id=833744). So once that is through it will be again necessary to check how it works with the test repo at https://github.com/guybedford/cs-source-maps (test-eval.html) is the file. If debugging isn't supported yet it would be worth chasing up the issues there as well.

@jrburke
Copy link
Member

jrburke commented Jan 28, 2013

Not sure if this helps, but meant to mention this earlier.

This tutorial:

http://net.tutsplus.com/tutorials/tools-and-tips/source-maps-101/

mentions that uglifyjs supports an --in-source-map property that could take a cs-to-js source map and use that as part of generating the minified source map. So maybe just have a way to tell the optimizer to look for [file name].coffee.js.map and if there feed to the --in-source-map property.

@tomjakubowski
Copy link

the standard implementation of CS supports source maps as of 1.6.0, by the way

http://coffeescript.org/#source-maps

@esamattis
Copy link

They got source maps working with the <script type="text/coffeescript"> tag.

jashkenas/coffeescript#2780
https://github.com/hden/coffee-script/blob/c24e957f1714e67240447deb643dff6b59b56ff6/src/browser.coffee#L15

I think we could do something similar here too.

@guybedford
Copy link
Contributor Author

Amazing, that looks like it would adapt perfectly here. Will check it out when I have a moment too.

@esamattis
Copy link

Also coffeeify from Browserify v2 can be used as a reference: https://github.com/substack/coffeeify/commit/9fb0ab24e923e66039304c1b5b7a84ec14a1ed86

@jrburke
Copy link
Member

jrburke commented Mar 19, 2013

I have an in-progress branch for source map support that goes from separate .js files to concatenated files, and then to uglify2 minification here:
https://github.com/jrburke/r.js/tree/sourcemap

I'm still working out the file naming, in particular so that the uglified maps point back to non-uglified code, so that tree is probably not interesting to the outside world yet. I also want to allow transpiler plugins to work. However, I have not worked out how a loader plugin would trigger its source map generation and saving. r.js allows building to a string output, so the plugin should not write to disk directly, but perhaps pass the data back somehow in the "write" loader plugin method. For writeFile, it probably should have a way to write directly to disk.

So I could use some help working out those mechanics. I have been very busy with day job stuff, but I do hope to get back to finishing up the sourcemap tree later this week, and if I get really lucky doing a 2.1.6 release with its changes before next week. Depending on how much churn it is for loader plugins, that capability may have to wait for the release after 2.1.6, but I would like to try for 2.1.6 if we can cleanly sort out the details.

@guybedford
Copy link
Contributor Author

The source map support in https://github.com/hden/coffee-script/blob/c24e957f1714e67240447deb643dff6b59b56ff6/src/browser.coffee#L15 seems ideal for loader plugin support. It uses a generated data-uri to describe the source map, so that it doesn't need to be saved on the server, which is what I was initially attempting above. Between the sourcemap branch, and a loader addon like this, that would cover the production and development environment source maps respectively.

I'm just getting into my open source time for this week... have a few other things to do, but will give the dev environment loader source maps an attempt if I can.

@guybedford
Copy link
Contributor Author

I attempted the source maps today and got quite far. CoffeeScript is generating the map, and the original CoffeeScript is displaying in the Chrome debugger.

But as soon as I change the path to a full URL everything falls over, and debugging doesn't work!

I've tried quite a few permutations of options, but can't get this to work, so either I am misreading the docs or Chrome has a few issues to work through yet. Any help would be really appreciated here.

Code is here - https://github.com/guybedford/require-cs/blob/master/cs.js#L147

@jrburke
Copy link
Member

jrburke commented Mar 20, 2013

I thought if sourceMappingURL was in play then sourceURL would not be needed.

By using a data URL that works around the need for exterior files, but I wonder if this would be desirable since it means the compiled JS would include a big source map in the source. There would be better performance by allowing the source map to be saved as a separate file.

Although maybe this approach can be used, and r.js would scan for @sourceMappingURL and if a data URL then pull it out and save as a separate file.

@guybedford
Copy link
Contributor Author

I believe the @sourceMappingURL is needed in order to display the file in the Chrome inspector. Info here - https://bugs.webkit.org/show_bug.cgi?id=107939#c8.

But no matter what permutations of the config I do this doesn't seem to be working.

The main use for this is modules compiled on the client. For the server, extracting out this sourceMappingURL should work well though.

Let me know if you know anyone who may be able to assist with the source maps config. Otherwise I will keep following this up.

@guybedford
Copy link
Contributor Author

Just to update where I am stuck at the moment:

The current source map I am using looks like:

var map = {
  version : 3,
  file: "/module.js",
  sourceRoot : "",
  sources: ["/module.coffee"],
  sourcesContent: ["define [] () -> \n   test: 'coffee'"],
  mappings: "(as calculated by CoffeeScript compiler)"
};

I'm giving the name "module.js", but have also tried leaving this out.

I am then converting this into a base64 data-uri and including it as the following:

eval(source + '\n//@ sourceMappingURL=data:application/json;base64,' + btoa(map) + '\n//@ sourceURL=/module.coffee);

This displays the correct file in sources in the current version of Chrome, and the debug points seem to correspond to the correct lines, but when debugging it crashes the browser.

In Canary, it reverts to displaying the JavaScript instead of the CoffeeScript as it does in Chrome.

The project is located at https://github.com/guybedford/require-cs, and the applicable code is here - https://github.com/guybedford/require-cs/blob/master/cs.js#L129.

Any tips would be really helpful, as I've hit a dead end here.

@guybedford
Copy link
Contributor Author

@lavrton
Copy link
Contributor

lavrton commented Apr 20, 2013

Hello, do someone have any updates of this issue?
I was playing with source map. It is works good, and also works with r.js and uglify2.
https://gist.github.com/lavrton/5425945

@guybedford
Copy link
Contributor Author

Thanks for looking into this. It nearly works, but if you try and use the debugger in Chrome, it crashes the browser. Then if you try it in Chrome Canary, you won't even see the source map!

Hence my webkit bugs which are getting no response currently. So close!!

@lavrton
Copy link
Contributor

lavrton commented Apr 21, 2013

I updated the gist: https://gist.github.com/lavrton/5425945
For me debugger works, also with page reload. I Have no Canary(os linux), so can't test it.
For use with r.js you should add this to buid configuration:

useSourceUrl: true

@guybedford
Copy link
Contributor Author

That's very strange - I still get the browser crash and it doesn't work on Canary. I am running on Mac. Perhaps a Windows test could be worthwhile I suppose.

@guybedford
Copy link
Contributor Author

@lavrton by the way thanks for your help - makes a big difference having someone check this as well. I've been stuck on this way too long!

@zr40
Copy link

zr40 commented May 13, 2013

For what it's worth, these are my results when using @lavrton's gist.

On Mac, neither Chrome Stable (26.0.1410.65) nor Chrome Canary (29.0.1506.0 canary) crash.
Without r.js, I'm seeing main.coffee, containing debuggable CoffeeScript.
With r.js, I'm seeing ../../../../../../../../cs!main, containing the compiler output for main.coffee. That output includes //@ sourceMappingURL and //@ sourceURL.

@guybedford
Copy link
Contributor Author

Thanks for looking into this - the key thing is to check the debug points. I have manage to get the source to show, but being able to place debug points is the key source map functionality that was crashing for me.

@zr40
Copy link

zr40 commented May 13, 2013

Breakpoints appear to be working in both browsers with and without r.js, though so far I have only tested this on simple CoffeeScript sources.

One oddity, though: without r.js, when I set a breakpoint and reload the page, the breakpoint appears to trigger on the first source line in addition to the line I actually set the breakpoint on. When I reload the page again, it only triggers on the intended line. This happens on both Stable and Canary.

@guybedford
Copy link
Contributor Author

The thing I noticed was the real points corresponding to the original source lines instead of the coffeescript. Basically as if it was showing the right source, but not applying the source map. Could that r the case in your tests as well?

@zr40
Copy link

zr40 commented May 13, 2013

No, that doesn't appear to be the case.

Using this CoffeeScript input:

'use strict'

require [], ->
    console.log 'test'
    console.log 'test'
    console.log 'test'

This JavaScript source is generated:

// Generated by CoffeeScript 1.6.2
(function() {
  'use strict';  require([], function() {
    console.log('test');
    console.log('test');
    return console.log('test');
  });

}).call(this);

//@ sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAiZmlsZSI6ICJtYWluLmNvZmZlZSIsCiAgInNvdXJjZVJvb3QiOiAiIiwKICAic291cmNlcyI6IFsKICAgICJtYWluLmNvZmZlZSIKICBdLAogICJuYW1lcyI6IFtdLAogICJtYXBwaW5ncyI6ICI7QUFBQTtDQUFBLENBQUEsQ0FFWSxJQUFaLEVBQVksR0FGWjtDQUdDLEVBQUEsQ0FBQSxFQUFBLENBQU87Q0FBUCxFQUNBLENBQUEsRUFBQSxDQUFPO0NBQ0MsRUFBUixHQUFBLENBQU8sSUFBUDtDQUhELEVBQVk7Q0FGWiIsCiAgInNvdXJjZXNDb250ZW50IjogWwogICAgIid1c2Ugc3RyaWN0J1xuXG5yZXF1aXJlIFtdLCAtPlxuXHRjb25zb2xlLmxvZyAndGVzdCdcblx0Y29uc29sZS5sb2cgJ3Rlc3QnXG5cdGNvbnNvbGUubG9nICd0ZXN0J1xuIgogIF0KfQ==
//@ sourceURL=./main.coffee

Notice that the console.log calls coincidentally happen to have the same line numbers in both.

After setting the breakpoint:
screen shot 2013-05-13 at 22 48 03

First reload:
screen shot 2013-05-13 at 22 48 27

Continue:
screen shot 2013-05-13 at 22 49 22

Continue:
screen shot 2013-05-13 at 22 48 03

Second reload:
screen shot 2013-05-13 at 22 49 22

@guybedford
Copy link
Contributor Author

Yes the above case has the same line numbers in both anyway, so it doesn't prove anything yet. But if it does work when the line numbers don't correspond then this is amazing.

@zr40
Copy link

zr40 commented May 13, 2013

I see what you mean. Using this source:

'use strict'

require [], ->
    ->
        4 for a in b
    console.log 1
    console.log 2
    console.log 3

This is generated:

// Generated by CoffeeScript 1.6.2
(function() {
  'use strict';  require([], function() {
    (function() {
      var a, _i, _len, _results;

      _results = [];
      for (_i = 0, _len = b.length; _i < _len; _i++) {
        a = b[_i];
        _results.push(4);
      }
      return _results;
    });
    console.log(1);
    console.log(2);
    return console.log(3);
  });

}).call(this);

//@ sourceMappingURL=data:application/json;base64,ewogICJ2ZXJzaW9uIjogMywKICAiZmlsZSI6ICJtYWluLmNvZmZlZSIsCiAgInNvdXJjZVJvb3QiOiAiIiwKICAic291cmNlcyI6IFsKICAgICJtYWluLmNvZmZlZSIKICBdLAogICJuYW1lcyI6IFtdLAogICJtYXBwaW5ncyI6ICI7QUFBQTtDQUFBLENBQUEsQ0FFWSxJQUFaLEVBQVksR0FGWjtDQUdDLEVBQUEsQ0FBQSxLQUFBO0NBQ0MsU0FBQSxXQUFBOztBQUFBLENBQUE7WUFBQSw0QkFBQTttQkFBQTtDQUFBO0NBQUE7dUJBREQ7Q0FBQSxJQUFBO0NBQUEsRUFFQSxDQUFBLEdBQU87Q0FGUCxFQUdBLENBQUEsR0FBTztDQUNDLEVBQVIsSUFBTyxJQUFQO0NBTEQsRUFBWTtDQUZaIiwKICAic291cmNlc0NvbnRlbnQiOiBbCiAgICAiJ3VzZSBzdHJpY3QnXG5cbnJlcXVpcmUgW10sIC0+XG5cdC0+XG5cdFx0NCBmb3IgYSBpbiBiXG5cdGNvbnNvbGUubG9nIDFcblx0Y29uc29sZS5sb2cgMlxuXHRjb25zb2xlLmxvZyAzXG4iCiAgXQp9
//@ sourceURL=./main.coffee

This is the result:
screen shot 2013-05-13 at 23 01 42

@guybedford
Copy link
Contributor Author

I filed a WebKit bug but have had no response. If you are able to put a note there as well it would help.

https://bugs.webkit.org/show_bug.cgi?id=63940#c13

@lavrton
Copy link
Contributor

lavrton commented May 31, 2013

I tested my gist on windows chrome canary. It works good with breakpoints and no bugs founded.

@ichernev
Copy link

@lavrton I tested on mac with chrome canary. It works, but if you set a breakpoint, and refresh the page, it first stops on the wrapper function. Otherwise it seems flawless. I also had to change //@ to //# as suggested by chrome (but that's minor).

@lavrton
Copy link
Contributor

lavrton commented Jun 21, 2013

#50

@guybedford
Copy link
Contributor Author

I've merged and tested the changes from #50 now, and it is all working great. @jrburke let me know if you're happy to do a 0.4.5 / 0.5.0 release based on the current changes.

It sounds like the issue with the base 64 encoding was mainly affecting the Russian utf-8 character encodings. @lavrton could you confirm this was the issue?

We really shouldn't have to include custom base64 encoding in this module, so it would be good to have an exact test case to know when window.btoa is working correctly.

In terms of handling builds, how about adding a third argument to load.fromText, which takes the JavaScript source map object as a parameter allowing source map composition for plugins?

@jrburke
Copy link
Member

jrburke commented Jun 26, 2013

I still do not think adding the map in the actual file is a good idea in general. I created requirejs/r.js#470 to track r.js changes related to connecting maps from transpiled languages, and mention your idea about load.fromText.

@guybedford
Copy link
Contributor Author

I would consider including the source map in the file in the same way as including the CoffeeScript compiler in the browser - it allows for development without a build, even if it doesn't seem particularly ideal. Still not sure where I stand on these browser compilation considerations.

@jrburke
Copy link
Member

jrburke commented Jun 26, 2013

I think the difference is the CS compiler is needed for the code to even run, where the source map is not. For me, the nice thing about source maps is that they do not impact production code, but if the developer needs to pop into the dev tools, then the annotations allow bootstrapping the debugger into something useful. This is particularly useful for minified content, where trying to debug a production problem with just the minified code would be hard.

@cmwelsh
Copy link

cmwelsh commented Jul 13, 2013

I don't think this feature, as implemented in #50, is relevant to production builds - there needs to be a completely separate source map strategy used when the RequireJS optimizer runs.

As long as this doesn't cause any regressions with the RequireJS optimizer I think it'd be great to ship this out as soon as possible. I have been eagerly anticipating it for a while.

@lavrton
Copy link
Contributor

lavrton commented Aug 14, 2013

@guybedford window.btoa throws exception if input contain any non ASCII chars:

btoa("Hello")
"SGVsbG8="
btoa("Hello! Привет!")
Error: InvalidCharacterError: DOM Exception 5
btoa("Hello! 汉!")
Error: InvalidCharacterError: DOM Exception 5

@dtilchin
Copy link

This is basically working now, at least in the browser. I had to make a couple fixes (#62) but with those it works pretty well - except for exceptions, where it always maps to one statement above the statement that caused the error, at the same indentation level. It's basically good enough for me, and it makes the plugin a lot nicer to work with. A shame if this project continued to languish :/

@huntc
Copy link

huntc commented Mar 27, 2014

Unfortunately rjs appears to break the chain when chaining source maps. In summary I propose that if rjs (or indeed Uglify2) finds an existing source map then it should somehow be considered in the final source map product. It shouldn't matter where the existing source map comes from whether it be from CoffeeScript, TypeScript, ScalaJs, etc.

Some background: we've built some tooling for sbt known as sbt-web. In particular we have developed an rjs plugin that integrates rjs with our asset pipeline. sbt-web will be an integral part of the Play Framework.

I have created an archive that fully reproduces the problem. To invoke rjs cd to the archive's expanded folder and type node r.js -o app.build.js. For convenience I have already run rjs. To observe the issue:

  1. cd to the appdir/build folder.
  2. open index.html in Chrome.
  3. inspect sources.
  4. ideally, they'd an x.coffee file in the list of sources.

I hope that this helps.

@crisu83
Copy link

crisu83 commented Dec 2, 2015

What's the reasoning behind disabling sourcemaps for builds? I'm referencing to this if-statement https://github.com/requirejs/require-cs/blob/master/cs.js#L313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests