shapesecurity / shift-scope-js

scope analyser for the Shift AST
http://shift-ast.org/scope.html
Apache License 2.0
11 stars 6 forks source link

little more specific document? #4

Closed beatak closed 9 years ago

beatak commented 9 years ago

First of all, this is great. I've been using Esprima for a while and I'm a big fan of parser to transform other JS files. Thank you!

I'm just trying to run what it says on https://github.com/shapesecurity/shift-scope-js/blob/master/README.md but it does not seem to be working. here's my code:

https://gist.github.com/beatak/aad8f4dc3410a241205a

The results are:

> % node es6.js --harmony
> import parse from "shift-parser";
> ^^^^^^
> SyntaxError: Unexpected reserved word

and

> % node es5.js --harmony
> */node_modules/shift-scope/lib/scope.js:46
>    for (var _iterator = this.variables.values()[Symbol.iterator](), _step; !(
> 
> ReferenceError: Symbol is not defined
>    at GlobalScope.Scope (*/node_modules/shift-scope/lib/scope.js:46:50)

Am I doing something totally off? I'm using Node v0.10.32 on Darwin Kernel Version 13.4.0/Mac OS X 10.9.5.

Thanks again!

michaelficarra commented 9 years ago

What happens when you just require('shift-scope') from the REPL? No --harmony flag needed.

The code in the README is an ES6 example and would need to be compiled to ES5 with a tool like 6to5 in order to run.

beatak commented 9 years ago

Awesome. Thanks for your quick response.

If I use 6to5-node it runs as it's expected: https://gist.github.com/beatak/f4d53896c062fc93c123

But if I just convert it by 6to5, the result is:

var program = "var a = 42;";
var parse = require("shift-parser")["default"];
var analyzeScope = require("shift-scope")["default"];
var ast = parse(program);
var scopeTree = analyzeScope(ast);
console.log(JSON.stringify(scopeTree, "\n", 2));

This code raises Symbol is not defined error. How do you usually test this?

michaelficarra commented 9 years ago

I test by simply running npm run build && npm test. Clone the repo, run npm install, npm run build, npm test, then node -pe 'require(".")'. If any of those fail, we can investigate further.

michaelficarra commented 9 years ago

@beatak: #16 probably fixes this issue. Can you try out master (also published as 1.0.2) and let me know if it is working for you now?

beatak commented 9 years ago

I tested it on my machine (xnu 13.4.0 x86_64 / node v0.10.32) and dev machine (Linux 2.6 x86_64 / node v0.10.16) and it works great. Yay to ditch 6to5-node. Thank you @michaelficarra

michaelficarra commented 9 years ago

Thanks for confirming.