ES4? I hope you meant ES5.

Yikes, yes.

ECMAScript 6 is a superset of ECMAScript 5, which is a superset of ECMAScript 3. I see only two options: ES6 and JSX.

@nzakas thank you for reconsidering this!

Another library which can provide traversal over es5/es6/jsx/es7 is ast-types which is used by recast.

Please note that we're committed to JSX and in order to make sure that the integration with tooling such as eslint is as easy as possible we created a specification for JSX.

• JSX Specification: http://facebook.github.io/jsx/
• JSX AST Specification: https://github.com/facebook/jsx/blob/master/AST.md

@vjeux good to know! The spec is useful, but not as useful as libraries that we can plug in and have "just work". :) Have you (all) done any work around creating an escope that's compatible with JSX?

@nzakas: It's worth noting that esprima-fb is the esprima harmony branch + JSX (it tracks the upstream harmony branch, we periodically merge in updates from upstream, and we always send harmony-specific PRs to upstream first) -- so if you just always use esprima-fb for ES6, you're supporting both ES6 and JSX. If you were to provide a toggle between esprima-fb and esprima/harmony, the only distinction you'd see (barring bugs) is that esprima/harmony would give a parse error if you threw JSX at it.

On the other hand, you still have the option of rejecting JSX in code if you so desire -- even if the parser parses it (if you see the presence of a JSX node while traversing the syntax tree, you error).

Would it make sense to simply provide a no-JSX lint than a toggle on which parser to use?

The only issue using esprima-fb (or the esprima harmony branch) applied on ES5 JS code is the following:

lib/rules/no-extra-parens.js
  43:12  error  Expected a "break" statement before "case"  no-fallthrough
  53:12  error  Expected a "break" statement before "case"  no-fallthrough

The harmony branch has the fallthrough comment at a different position in the AST. Shouldn't be hard to get the lint rule accept both positions.

In order to lint ES6 or JSX code, there are a few rules that need to be updated. For example the unused variable rule doesn't consider to be a 'use' and marks it as unused.

We're currently playing around switching from jslint after pre-processing to eslint. So far we've got something "working" by disabling a few rules. It doesn't seem that it would require drastic changes and only a handful of pull requests would be needed to support it properly.

In order to lint ES6 or JSX code, there are a few rules that need to be updated. For example the unused variable rule doesn't consider to be a 'use' and marks it as unused.

This is tricky because in React's JSX, writing <Button> counts as a use of the variable Button but since JSX itself doesn't specify semantics, that might not be true with a different transformer.

The harmony branch has the fallthrough comment at a different position in the AST

This sounds like a bug we should just get fixed -- not quite sure why they differ

For example the unused variable rule doesn't consider to be a 'use' and marks it as unused.

This would be a React-specific lint rule. JSX is just syntax -- React applies meaning to that syntax; So given that the lint-rule would be react-specific anyway, specifying the react-specific contextually-local variables in the lint rule makes sense.

@jeffmo ah, that's very helpful information to know! I didn't realize the relationship between esprima and esprima-fb was so well-defined. Your point about erroring on JSX syntax is valid, too. It's kind of nice not to have to do anything and have the parser blow up, but in lieu of that, a hand-crafted approach that just looks for JSX and reports an error is definitely feasible.

A followup question: is ES5 + JSX a valid use case for anyone? I was under the (perhaps mistaken) impression that ES6 was a requirement. If that's not the case, then it would be better to have a JSX opt-in in addition to specifying which ES version you want.

@vjeux that's great. Feel free to file issues for anything that would have to be tidied up to work with esprima-fb. (We require issues for all code-based PRs, so best to get started early. :) )

@jeffmo @vjeux also agree that the comment being in a different place seems like a bug, can one of you follow up on that?

@nzakas: I don't think there's anything that says React+JSX strictly requires ES6 today, but it's likely to start building on more and more newer ES concepts (classes, spread operators, arrows, etc).

On the other hand (as @michaelficarra mentioned), in the same way that esprima-fb is a superset of esprima/harmony, ES6 is a superset of ES5 -- so I'd imagine you could apply the same regressive thoughts around ES5+JSX just working off of the esprima-fb parser (unless you're just relying on the parser to blow up if it hits ES6). Though even if you did want such a parser (ES5+JSX, but no ES6), we don't have any plans to build or support such a parser on our end.

On the comment-node issue: I'll try to take a look tomorrow

Regarding the problem with the comment location, looks like eslint uses esprima 1.2.2. On esprima master (2.0.0-dev) the output differs from esprima 1.2.2. I used following source to test

switch(foo) { case 0: a(); /* falls through */ case 1: b(); }

esprima-fb has the same output as esprima 2.0.0-dev

@frantic yup, that's exactly the problem. There's no reason for the comment location to differ between the two branches.

I took a look at dropping-in esprima-fb and estraverse-fb and basically found two issues:

1. The location of comments isn't just different for the case statement mentioned previously, it's also different in the treating of top-level comments. This basically looks how estraverse.attachComments() deals with comments rather than the way esprima 1.2.x deals with comments, which makes many rules dealing with comments break in those cases.
2. esprima-fb is a bit slower than esprima 1.2.x. I'm not sure if that's because the enhancements from 1.2.x didn't land in the Harmony branch, but it's a non-trivial performance reduction. You can try yourself running npm run perf.

Here's with esprima 1.2.2:

CPU Speed is 3093 with multiplier 7500000
Performance Run #1:  1719.9273899999998ms
Performance Run #2:  1716.118489ms
Performance Run #3:  1708.291122ms
Performance Run #4:  1688.523239ms
Performance Run #5:  1706.549722ms
Performance budget ok:  1708.291122ms (limit: 2424.8302618816683ms)

Here's with esprima-fb:

CPU Speed is 3093 with multiplier 7500000
Performance Run #1:  1893.5033819999999ms
Performance Run #2:  1884.1219959999999ms
Performance Run #3:  1904.216641ms
Performance Run #4:  1897.484768ms
Performance Run #5:  1899.810498ms
Performance budget ok:  1897.484768ms (limit: 2424.8302618816683ms)

I started some investigative work around this on the esprimafb branch. Not anywhere near complete, but gives a decent idea of what I'm thinking.

Filed a ticket with Esprima about the comment attachment problem: https://code.google.com/p/esprima/issues/detail?id=607

Any update on this? I'm seeing commit under the yannickcr/eslint#deezer fork, but can't make it works (I've tried the "valid-syntax" option with jsx to true :/)

What kind of an update are you looking for?

There's no valid-syntax rule. See http://eslint.org/blog/2014/11/es6-jsx-support/ for how to configure correctly and what's going on.

I just found a way (not sure why) to force jsx

{
  "settings": { "jsx": true }, // had to add this
  "valid-syntax": [2, { "jsx": true }],
}

https://github.com/eslint/eslint/tree/es6jsx/docs/configuring#specifying-language-options

Well not sure where I found that haha. Sorry for this
The other part was to do that

$ eslint . --ext=.js --ext=.jsx

But thanks now it's working correctly! Awesome work.

Update: after much deliberation, I've decided the best way to move forward is to fork Esprima and make the changes we need. We have been held back by Esprima bugs for too long. So here is our brand new adding Esprima fork called Espree: https://github.com/eslint/espree

More details on the readme, but basically I'm starting with esprima 1.2.2 and adding in ES6 and JSX features. 1.2.2 is the version that ESLint runs on today, so it provides the best compatibility for us going forward.

Unlike Esprima, Espree will support JSX as an option directly. @vjeux if Facebook folks are willing to help add JSX support to Espree, I'll happily accept the contribution.

For ES6, it's just a matter of pulling in features one by one from the Esprima harmony branch. I've already added a way to specify the ECMAScript version to use, restricted let and const to ES6 more, and added support for the regular expression u and y flags. Overall progress is tracked here: eslint/espree#10

It's disappointing to need to create a new parser, but JSHint and JSLint already have their own parsers, so this brings us inline and allows us to move much faster.

Could you use Facebook's fork of esprima, which already has full support for JSX, instead of forking? I don't have a lot of context here, so forgive my ignorance.

That's what we've been trying to do, but Esprima-FB is based on the Esprima harmony branch, which is where all the problems lie. It's just not worth continuing down that path.

@nzakas Are you sure you're not going with Acorn? Acorn would give you full es6 support, better parsing speed (especially with location info enabled), and error recovery. I'm currently considering changing from jshint to eslint on c9.io, but particularly that lack of error recovery is pretty obvious when all warnings disappear whenever I'm typing and there's a parse error :( (Also, if you would switch to Acorn I would get the benefit of using just one parser for all javascript analysis rather than having two of them ^_^;; But then I might run into #1025, I guess.)

I'd love to use acorn, but it has several incompatibilities with esprima that would require changes to almost every rule we have (not to mention it would break custom rules that people have written).

An alternative is to punt on jsx entirely and use a library like https://github.com/alexmingoia/jsx-transform/ to convert the jsx to hyperscript and then run that file through the linter. The one complexity that remains is making sure that the jsx conversion produces hyperscript code that follows the same rules as your eslint configuration.

@lennartcl The esprima API does allow you to pass in an optional tolerant flag on parse that when true makes parse return "an extra array containing all errors found, attempts to continue parsing when an error is encountered". See http://esprima.org/doc/index.html

Is the esprima tolerant mode sufficient for your needs in c9.io?

@nzakas if you've forked esprima and made bugfixes, why not send a PR back upstream as the esprima-fb developers do, instead of renaming your esprima fork to espree?

@michaelficarra I'm pretty sure the answer (as is stated in the thread) is that the Harmony branch is pretty incompatible with main branch used by ESLint and it's not fair to ESLint rule makers out there to change the AST at this point.

See also: eslint/espree#12

I'd like to see a list of those incompatibilities. They're probably things that esprima would care to fix.

@michaelficarra I've forked off of 1.2.2, which is pretty different than 2.0.0, so it's not going to be easy to reconcile the changes. I've also been filing bugs on the Esprima bug tracker and getting no updates and no indication if a fix is forthcoming. It's not fair to keep holding ESLint back waiting for changes that may or may not happen.

In any event, and to be clear to everyone: this decision has been made. I've been trying to work with Esprima for a while now and have reached the point where I don't see using Esprima as a benefit any longer. ESLint needs to move forward and the best way to do that is to control the parser we're using. Hence Espree.

@nzakas are these outstanding issues eslint has with esprima all related to getting JSX support in eslint or are there unresolved issues with esprima with vanilla javascript linting as well?

@malandrew the Esprima issues have nothing to do with JSX support. The fact that esprima-fb is based on the Esprima harmony branch means any issues from Esprima leak through. That's the problem (more details throughout this thread).

@malandrew I experimented with the "tolerant" option. But for most things I throw at it, it just fails with an exception anyway. It might be nice for eslint to at least try enable that option by default though, if only to show multiple parse errors at the same time.

If you're interested in discussing the tolerant option, please open a separate issue. That topic is orthogonal to this one.

#1602 adds the ability to set the feature flags for Espree, which allows turning on ES6 features and JSX.

#1640 makes sure traversal of JSX nodes works correctly

Closing as 0.12.0 introduced support for JSX and partial ES6 support.

🏆