ES6 module import tests relies on a version of esprima that doesn't follow estree spec
If you look at https://github.com/estools/escope/blob/master/test/es6-import.coffee#L25 you see that the tests for es6 uses an esprima version that does not generate https://github.com/estree/estree spec compliant ast
For example:
ast.body[0].specifiers for code 'import v from "mod"' is expected to output:
[ { type: 'ImportDefaultSpecifier', local: { type: 'Identifier', name: 'v' } } ]
and not
[ { type: 'ImportDefaultSpecifier', id: { type: 'Identifier', name: 'v' } } ]
by the way, there is no version of esprima at the moment that follows estree, the harmony branch is old and master is expected to merge https://github.com/eslint/espree changes soon according to comments on Freenode's IRC channel #esprima
related: https://github.com/estools/escope/issues/33
and the proper esprima issue might be this one: https://github.com/jquery/esprima/issues/1000
Yeah, Espree is the first parser to implement the ESTree representation of ES6 modules.
estraverse, esrecurse are now upgraded to suppert those types :)
So next is escope :D
Yay!
if replaced with espree, lines such as this one https://github.com/estools/escope/blob/master/src/referencer.js#L117 must be updated to use node.local instead of node.id
one possible fix to this bug:
diff --git a/package.json b/package.json
index f9ea500..9b45783 100644
--- a/package.json
+++ b/package.json
@@ -28,6 +28,7 @@
"browserify": "^9.0.3",
"chai": "^2.1.1",
"coffee-script": "^1.9.1",
+ "espree": "^1.11.0",
"esprima": "~1.2.2",
"gulp": "~3.8.10",
"gulp-babel": "^4.0.0",
diff --git a/src/referencer.js b/src/referencer.js
index 7b592ce..a237e43 100644
--- a/src/referencer.js
+++ b/src/referencer.js
@@ -108,20 +108,20 @@ class Importer extends esrecurse.Visitor {
}
ImportNamespaceSpecifier(node) {
- if (node.id) {
- this.visitImport(node.id, node);
+ if (node.local) {
+ this.visitImport(node.local, node);
}
}
ImportDefaultSpecifier(node) {
- this.visitImport(node.id, node);
+ this.visitImport(node.local, node);
}
ImportSpecifier(node) {
if (node.name) {
this.visitImport(node.name, node);
} else {
- this.visitImport(node.id, node);
+ this.visitImport(node.local, node);
}
}
}
diff --git a/test/es6-import.coffee b/test/es6-import.coffee
index 11d8a6b..6e524b3 100644
--- a/test/es6-import.coffee
+++ b/test/es6-import.coffee
@@ -22,7 +22,7 @@
# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
expect = require('chai').expect
-harmony = require '../third_party/esprima'
+harmony = require 'espree'
escope = require '..'
describe 'import declaration', ->
It would probably be easier to replace node.id with node.local || node.id
true, here is a pull request: https://github.com/estools/escope/pull/52
Other nodes, (exportXXX should be handled).
@Constellation do you have a plan to release this fix?
@nzakas, @ai
What do you think of updating the major number with these ES6 commits?
@Constellation it will be more difficult for me personally, because I will be wait for eslint update (this issue is critical for me).
But we must do it, if we change the API. But maybe it is only a fix, because new behaviour was expected from current API?
It looks like these changes are backwards compatible, so I don't think a major version bump is necessary.
@nzakas, @ai
Ah, my concern is that, to align to estree that is actively developed, we will need to change/add/drop many nodes. So I'm worrying about that backporting becomes difficult. Maybe, this change can be easily ported to 2.0.x. So I'll pick it to 2.0.x now :)
Oh yeah, I was just thinking about the changes for this issue, specifically. With all the node changes, it's probably best to do a major version bump.
And I'll pick super support and I'll publish it as 2.0.7 soon! :D
Awesome!
Released as 2.0.7 :)
Thanks!
@Constellation I'm not sure what happened, but 2.0.7 seems to have a regression related to AssignmentPattern. See: https://github.com/eslint/eslint/issues/2001
@nzakas
Ah, current escope is locked with estraverse 1.9.x because there's no exportxxx node support in estree.
We need to implement all ExportXXX node before updating estraverse to 2.0.x.
@nzakas
Maybe, to support ExportXXX, only modifying ExportSpecifier is sufficient I think.
I'll create a patch and update the estraverse contained in escope.
AssignmentPattern is done in b713c53fc5b23c8df3a947af8f91f58e93b78751.
This is initial support of AssignmentPattern and TDZ scope issue is not fixed yet.