Few questions

[mrjoelkemp]

1. Why do we need to monkeypatch estraverse? Can we not use babel's traversal mechanism which includes the es6 types?
2. Instead of doing a parent search, why don't we just make jscs a dependency of babel-jscs, and the loc of jscs could be found via require.resolve('jscs')? There's room for trouble since jscs will depend on babel-jscs which depends on jscs. But from my local tests, I haven't seen an issue yet.
3. What if we just pull in babel's types into tree-iterator.js and extend our custom supplied keys to estraverse? From my local tests, that plus the use of jscs within babel-jscs gets us closer to a stable integration (some tests still fail, but there's progress).

[mrjoelkemp]

Will ask more questions as I continue to tinker.

[hzoo]

Cool thanks for checking it out!

I guess firstly I'l just say I didn't do a lot of thinking of any improvements and it's mostly just babel-eslint and then changed some things to work for jscs - it's possible some things are only used in eslint and not required for jscs (I removed stuff like escope tracking, etc).

1. Yeah the only reason to monkeypatch is to do assign(estraverse.VisitorKeys, t.VISITOR_KEYS); in tree-iterator.js

There's room for trouble since jscs will depend on babel-jscs which depends on jscs

1. Yeah I don't know other than what you described. I accidentally added eslint to the dependencies and got an issue but I'm not sure if it's the same babel/babel-eslint@8815982
2. Are you saying we just pull in the newer types into tree-iterator in jscs itself? I guess we did that for JSX already. Yeah then we wouldn't need the whole monkeypatch method and no dependency on that. But then we would need to update the estraverse.VisitorKeys when babel updates it's keys.

Yeah the last time I tested it it seemed like none of the rules failed other than the ones related to quotes and other tests related to the cli. The only other failing tests were the function declaration one which I already fixed in jscs master (export default function with no node.id) jscs-dev/node-jscs/issues/1376

[mrjoelkemp]

I think we'll still need acorn and the esprima to acorn work for a parser that supports the new node types.

In regards to the monkeypatch, I'd rather not hardcode the new visitor keys into jscs (we'd then have to maintain the list), but pull in babel-core and lodash.assign the keys into our tree iterator.

Im also not sure (yet) whether we need the comment modifications provided here.

[hzoo]

Yeah acorn-to-esprima will be needed. There's an issue to move it out to a separate repo too (babel/babel-eslint#24) if we think that's a good idea.

I mentioned in the other issue jscs-dev/node-jscs#1446 that importing babel-core might make jscs bigger? If that's not the case or not a big deal that sounds good.

I think we need the comments since esprima.parse gives you the ast including the comments unless we are doing that separately? We probably want to parse once though.

[mrjoelkemp]

There's an issue to move it out to a separate repo too (babel/babel-eslint#24) if we think that's a good idea.

For sure.

I mentioned in the other issue jscs-dev/node-jscs#1446 that importing babel-core might make jscs bigger?

I personally don't care about the download size. Alternatively, can babel just move its types definition to another module like babel-types?

I think we need the comments since esprima.parse gives you the ast including the comments unless we are doing that separately?

You're right. We need it.

Will close and ping when I get closer to making the jscs test suite stable with babel-jscs.