Source Mapping

[tarqd]

It'd be nice if node-theseus generated a source-map after instrumenting the file, so if I were to debug with node-inspector concurrently I could set view the original source and still set breakpoints/step through my code. Imagining being able to see all my functions calls etc with theseus with all the power of v8s debugger makes me all giddy

[alltom]

This is on my to-do list. :)

node-theseus would be most of the way there if falafel automatically generated a source map when you used its update() function (https://github.com/substack/node-falafel). That's probably the route I will take when I look into this.

[tarqd]

Definitely, I created a feature request there. The only problem I see is that falafel would need to be modified to keep track of the line/column numbers of the new data (it would be a lot easier if source maps just let you use the index/position, like the range property on the AST nodes)

[tarqd]

It's actually really easy to do. I haven't tested this code (beside the fact that it does generate the inlined source-map and it seems to have the correct mappings but don't quote me on that).

Maybe you can test out this branch with theseus and see if it works https://github.com/ilsken/node-falafel/commit/b45ff626e2ebbcbe097209350f7d4cc3d975d334

[alltom]

I assume I am just supposed to pass debug: true as an option to falafel?

The map I get out is just "[object Object]" encoded as base64, but it seems close enough to working that I'm going to poke around and see if I can get it the rest of the way...

[alltom]

I won't be able to look at this again for a bit, but I just pushed a version of fondue (https://github.com/adobe-research/fondue/commit/5713cb6621aea29778bc68ac04e3f8f9cff6e541) that does all the transformations from within falafel so that the source maps ought to work when they work.

Also, from reading around online, it looks like the line where you append the source map should look like this:

result.chunks.push('\n//@ sourceMappingURL=data:application/json;base64,' + base64(sourceMap.map.toString()));

I generated test files with bin/fondue and included them from an HTML file in Chrome, but the Web Inspector doesn't seem to recognize the maps. I'm not sure how to debug that.

[tarqd]

Damn I was really hoping it was just going to be that simple :(. I'm going to look into how coffeescript and uglify handle it when I get a chance.

As for chrome not recognizing the file, maybe try //# sourceMappingURL= (spec says they should prefer //# but accept both. Could have something to do with the source/generated name not being accurate when passed to new SourceNode. The mapping object (.map) looks fine and I'm pretty sure new Buffer(str).toString('base64') is good to use in a data uri

Edit: Actually it could be from not passing the name parameter to the SourceNode.

[tarqd]

I got it working! I tested the output in chrome and it worked (run example-sourcemap.js) The CodeGenerator which now handles manipulating nodes and creating the source map implements a Transform stream so it requires node v0.10.x

ilsken/node-falafel@source-maps

[alltom]

1. Check out your branch of node-falafel.
2. Run node example-sourcemap.js > foo.js
3. Make foo.html that has <script src="foo.js"></script>.
4. Open foo.html in Chrome.
5. Open Web Inspector, go to Sources tab, open foo.js.

I see this:

function foo(){
    var bazzle = 'baz';
    return bazzle;
}

//@ sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZm9vLmpzIiwic291cmNlcyI6WyJmb28uanMiXSwibmFtZXMiOlsiYmFyIl0sIm1hcHBpbmdzIjoiQUFBQSxDQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FBQTtBQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FDS0EsTUFETCxDQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FBQTtBQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FBQSxDQUFBLENBQUEsQ0FFUUEsTUFGUixDQUFBO0FBQUEsQ0FBQSIsInNvdXJjZXNDb250ZW50IjpbImZ1bmN0aW9uIGZvbygpe1xuXHR2YXIgYmFyID0gJ2Jheic7XG5cdHJldHVybiBiYXI7XG59Il19

Did I do this any different from you? I'm running Chrome 27 on OS X with "Enable source maps" set in the Web Inspector preferences.

[tarqd]

When I put into jsfiddle it seems to work fine (Chrome Version 28.0.1500.71 ).

[alltom]

It works for me there, too! I guess I'll figure out why it doesn't work on file://localhost and http://localhost later.

I'm now debugging some mangling that occurs in your branch with or without mapping enabled. For example, if I modify your script to do this identity operation:

    if(node.type == 'BlockStatement'){
        node.update(node.source());
    }

then the output is:

function foo(){
;
;

[tarqd]

Ah I found the issue. Apparently if you have the same name sourceName and generatedName chrome will ignore the source map entirely. It works if you provide a different generatedName from the sourceName https://github.com/ilsken/node-falafel/commit/7f2ca9ec61e9062a548f0542ad7340f17e7b8277

[tarqd]

So close..I really need to write new tests for the code generator

[tarqd]

Fixed in https://github.com/ilsken/node-falafel/commit/8b4265a40c29a60f0bc77004f0c538a4a7be97ab I was using line.length - 1 when I should of been using line.length, doh!

[alltom]

There may be off-by-one errors going the other way now. I think they're uncovered in some of the falafel tests that are now failing (such as parent.js), but here are two that are relevant to fondue:

Error 1

if (node.type === "ReturnStatement" && node.argument) {
    var src = node.source().slice(6);
    var semicolon = src.slice(-1) === ";";
    if (semicolon) src = src.slice(0, -1);
    node.update("return traceReturnValue(" + src + ")" + (semicolon ? ";" : "") + "\n");
}

produces

function foo(){
    var bar = 'baz';
    return traceReturnValue( bar;
)

}

Error 2

if(node.type == 'FunctionDeclaration'){
    var oldBody = node.body.source().slice(1, node.body.source().length - 1);
    node.body.update('{ try { ' + oldBody + '\n } catch (e) { /* ... */ } finally { /* ... */ } }');
}

produces

function foo(){ try { 
    var bar = 'baz';
    return bar;
}
 } catch (e) { /* ... */ } finally { /* ... */ } }

[tarqd]

Looks like the problem is the appendChunks function always appends a line break (which leads to an extra one that shouldn't be there) which seems to break most of those tests and cause those errors (for example in the first on the last character wasn't the semicolon but the extra \n. All these cases should work now and I ran the tests and only one is failing now (test/async which fails because the stream is closed when the AST has been walked, I suppose I'll need to fix that but it may cause some changes how you need to use it because eventually something need to close the stream)

https://github.com/ilsken/node-falafel/commit/02176122b7ae3e03a3b33a860f9f88ead10de542

[tarqd]

Actually since there's no actual IO it wouldn't be so bad if the stream wasn't explicitly closed, would you be against changing how the actual API for this works? I doubt this is going upstream anyway because it changes how falafel generates code entirely and I don't think substack would want to pull it (and on the issues they seemed against using streams2 because it wouldn't work browser side without some shims)

[alltom]

Seeing as falafel is just esprima + visitor pattern, it doesn't seem like a big deal to fork it or make something entirely new. :)

Also, it doesn't seem to make sense for the code generator to be a stream. From what I understood of the code, you could rename _transform() as update() and flush() as toString() and remove all the stream parts and it would be much easier to understand, and more compatible. I don't think it's legit to pass anything except Buffers and strings to a stream's write() function, either.

Also, I played with it a little more and it seems like fondue actually needs a node.splice(index, [length, [text]]) method instead of update(text). What happens now is that if fondue rewrites a return statement, then it rewrites the containing function, the mapping of the return function is overwritten because all the rewrites look like node.update("text" + node.source() + "text") and there's no information about how that maps to what was there before. (Try setting a breakpoint in a function in the Web Inspector.) With such a splice method, fondue could add and remove bits of the source without losing that information.

I should also start trying to use the source map library to make sure I'm getting line/col mappings that are going to be useful for Theseus, hmm...

[tarqd]

Yeah one of the main problems with how it's setup right now is it's very easy to lose mappings :(. I was hoping that it wouldn't affect it too much. It'll take some work to make sure changes to containing nodes don't overwrite mappings for changes of any child nodes due how it's organized at the moment. If you have any ideas let me know because I definitely think the method it's using to generate the code/source map could be better

I actually thought I could make it more stream-like but later on I found it wouldn't work out easily so I'd be all for removing the stream2 dependency

Originally I wanted something like this

fs.createReadStream('foo.js').pipe(new CodeGenerator(function(node){
// rewrite stuff
}).pipe(fs.createWriteStream('foo.generated.js')) // or wherever you wanted the output to go

[alltom]

I took a pass at it: https://github.com/alltom/node-falafel-map (relevant diff here: https://github.com/alltom/node-falafel-map/commit/55e931440ab19bbc0b51e52f15de6acc6ac2fc7a#diff-1)

Basically:

1. update() now creates a SourceNode containing the array of arguments passed to it. Thus, it accepts strings or other SourceNodes.
2. I added node.sourceNodes() which is like node.source(), but returns the underlying array of strings and SourceNodes.
3. If update() is never called for a given node, it is automatically called by falafel-map with node.sourceNodes().

I've lightly tested it, to the extent that the tests still pass and the output makes sense in this source map visualizer. I also have a locally-modified fondue which uses the new API, but I haven't verified that it works at all, only that it doesn't crash and the resulting code looks fine. :) I'm testing that next.

[tarqd]

Looks great! I think that should do it, all we need now is the same level of integration cloud9 has with node.js' debugger and multi-cursor support in brackets and it'll be my #1 editor (sorry sublimetext)

[alltom]

I took out that third change and pushed it as the falafel-map npm module. fondue master uses it and it's looking like it will be in the next release.

Thanks a ton, ilsken, for all the code and energy!