Feature: Enable syntax based on reading from .babelrc

[hzoo]

Ref #434 (subset of syntax), #523 (user plugins), #505 (Typescript)

babel-eslint/lib/parse.js

Lines 18 to 22 in bf27f60

	plugins: [
	"flow",
	"jsx",
	"estree",
	"asyncFunctions",

Instead of having to:

• add to the plugin list whenever there is a new syntax and release a new version
• using a wildcard/star * option (found out that was a bad idea due to breaking changes between plugins like decorators/decorators2 so wouldn't be able to easily resolve that)
• expose a user config whitelist

We should be able to just read from a user's Babel config. @babel/core should expose a way for another tool to get relevant information from a user's config file (like return the list of syntax plugins used). babel-eslint could try to read that and just populate the parser options and we can remove the defaults. I think it's safe to assume if you are using babel-eslint you are using babel, and should have a config (can wrap in try/catch or fallback to default list)?

• Less config (re-using config)
• Enable subset of features
• We should be able to remove all deps and just do a peerDep on @babel/core maybe? Then it would use the version of babel in your project rather than it's own dep. And maybe the same for the eslint deps?

babel-eslint/package.json

Lines 14 to 17 in bf27f60

	"@babel/code-frame": "7.0.0-beta.36",
	"@babel/traverse": "7.0.0-beta.36",
	"@babel/types": "7.0.0-beta.36",
	"babylon": "7.0.0-beta.36",

edit: I believe we are only using @babel/types to get the the VISITOR_KEYS, and traverse to do a fast transform of the AST

Thoughts? cc @ljharb @kaicataldo @loganfsmyth @azz @danez @zertosh

[azz]

Makes perfect sense to me. Much better long term solution and it's good for perf, too.

[kaicataldo]

I'd like to work on this.

[kaicataldo]

And agree, I think it should use the new @babel/core API and fallback to the default (potentially outdated) list when it isn't there.

[hzoo]

Awesome, looks like @loganfsmyth was thinking about the babel side in babel/babel#6332 (comment) (didn't realize we had a PR for getting parserOpts earlier)

[ljharb]

fwiw, once this is implemented, the airbnb eslint config can finally switch parsers to babel-eslint, since we’ll be able to get parsing errors on syntax we’re not transforming - this includes enabling async/await and class fields.

[ljharb]

As for a fallback, i would definitely want a parser option that caused the linting run to fail outright if there’s no babel config present.

[kaicataldo]

I have some time to work on this right now, but wanted to make sure we agreed on an API.

@loganfsmyth's suggestion of exposing a parsing method that would do config loading, parse, and return the AST seems like a good suggestion to me. This is similar to how typescript-eslint-parser uses typescript to generate the AST (defining it as a peer dep), converts the AST to ESTree, and passes it to ESLint.

Does this seem like a good way forward here?

cc @JamesHenry if you have any suggestions, having worked with what I believe to be a similar architecture for typescript-eslint-parser

[kaicataldo]

It looks like @xtuc is actually working on this, so I'll unassign myself.

[xtuc]

I had the intention to do so, but missing time unfortunately. Feel free to take it.

Don't hesitate to ping me tho.

[hzoo]

Ok so after babel/babel#7291 I think we would make a peerDep on @babel/core and then make sure the api we use are exposed in @babel/core?

[kaicataldo]

Yeah, though it'll have to have a minimum requirement of a beta version, which is kind of weird?

[hzoo]

I think it would be that way unless it's backported, same with the other integrations unfortunately

[ljharb]

Wait, so that PR is sufficient to close this? Now babel-eslint can automatically read my babelrc and will refuse to parse things i don’t transform?

[loganfsmyth]

That's the first step, looks like this just got closed when that was merged.

[hzoo]

Yeah still need a PR on this end for sure

[kaicataldo]

I have a PR in the works.

Edit: Waiting on the next Babel release, which should be soon!

[kaicataldo]

And sorry, didn't mean to link that. This was the only open issue and I went into autopilot 😅

[kaicataldo]

In the meantime, we can discuss this: babel/babel#7291 (comment)

Copying and pasting my question for ease of discussion:

We can figure that out on the babel-eslint side. If this is the desired API for Babel, I think babel-eslint has a few options:

1. Throw an error and show a helpful warning, killing the linting task entirely.
2. Add the ability to define parserOptions in the ESLint config or to be passed on the command line (we'd have to add the ability for babel-eslint to pass these options to @babel/core#parse) when no filename is found. Not necessarily a big fan of this idea, but we need some way to define plugins for Babylon if there isn't a filename.
3. Fallback to parsing with Babylon with the current default list of plugins (may not be up to date). This would make the fallback be essentially the behavior it has now.
4. Fallback to trying to parse with Espree (ESLint's default parser). Not sure how I feel about this, because a lot of things might work/not work by accident, and I think this will end up being really confusing for users.

My personal preference would be 1 or 3, as I think they're the least confusing.

1 makes it so that running babel-eslint is much more tied to having a working Babel setup to begin with, which I don't think is a bad thing since I think a lot of people don't realize most of the syntax they use is already covered by the default parser. It makes it clear that if if you're not using Babel, you don't need to use babel-eslint!

3 would just be falling back to the current behavior.

It looks like there's general consensus around 1 or 3, with one vote for 1. I'm also leaning towards 1 because I think it provides clarity around when someone should be using babel-eslint vs the default parser, which seems to have been a point of confusion for a long time.

[loganfsmyth]

I'm fine with 1. If users ask we can always add an option to opt into option 3.

[ljharb]

My use case is that i want to specify targets and transforms, and i want the linting task to fail loudly when any other syntax is encountered. Is that option 1?

[nicolo-ribaudo]

I'd like to also add
5. Fallback to parsing with Babylon with no plugins, reporting a warning.

I think that it is easier to reason about than 3, because we avoid the hardcoded and hardly up to date list of plugins. The warning would have the same effect of 1, but users can choose to ignore it.

[kaicataldo]

@ljharb Just to be clear - you mean specify targets and transforms in your .babelrc?

[loganfsmyth]

@ljharb That would be the case either way, this list is specifically for if a file has been ignored in the Babel config.

@kaicataldo I agree with @nicolo-ribaudo here, I'd say if we did have a fallback it should be the no-plugins version.

[kaicataldo]

How does everyone feel about going with option 1 and, if users can ask for it, add option 5?

[lukescott]

As a user, I'm not sure if I understand what default plugins vs no plugins means. The lease surprising thing would be to do what babel does. If that means loading default plugins in absence of a user config, it should probably do that. However, the default list of plugins should be kept in babel, not maintained as a separate list in babel-eslint - I'm assuming that's what "may not be up to date" meant.

I think this extends beyond just babel-eslint does though. There should be a standard set for every tool that uses babel, which IMO should be "use .babelrc; do what babel does". There needs to be a consistent expectation set, no matter what tool is used.

[loganfsmyth]

@lukescott This question only applies if your Babel config for instance had

{
  plugins: ['proposal-class-properties'],
  ignore: ['./lib'],
}

what happens if you try to lint a file in lib? The Babel config explicitly says that Babel should ignore the file, so it definitely shouldn't pare expecting class properties.

So we either

1. Throw an error, potentially allowing users to opt in to Babylon's standard of not expecting proposals using parserOptions in .eslintrc.
2. Just never error and parse using Babylon's standard behavior when Babel didn't know what to do.

[ljharb]

Throw an error. Always better to fail loudly than silently.

[ljharb]

(To clarify, eslint should ignore things via eslintignore, not via babel ignore)

[loganfsmyth]

I think the main thing that made me unsure is that if you forget to put something in .eslintignore and open it in your editor normally you'll get get lots of useless ESLint warnings, but you'll still get them. With this, babel-eslint will throw, so it'll be a difference in behavior that to me at least isn't strictly necessary.

[lukescott]

Oh, got it. Thanks for the clarification. Failing loudly for now sounds good to me 👍

Just out of curiosity, if someone did want to lint ./lib, couldn't they put an.eslintrc file in there and choose a different parser? Meaning there are solutions, even if you went with just option 1.

[kaicataldo]

@lukescott ESLint also has glob-based configuation.

Looks like Babel was released yesterday - will be able to start working on this PR in earnest today.

[kaicataldo]

I've come across a small snag in working on this. The location information is stripped from errors thrown by the parser when calling @babel/core#parse(), which means that location information for syntax errors is not passed to ESLint. It looks like this was added in this commit.

@loganfsmyth I'm happy to chat about this in the Babel Slack in real time if it's easier, but it looks like we'll have to figure out some way to not strip this information out if we want to use the same logic that's used to process files during a full transform run.

[kaicataldo]

As an update - we've landed a PR that adds this location information back in (babel/babel#7314). I'm just waiting on the next Babel release so that I can finish the babel-eslint PR.

[kaicataldo]

Have a WIP PR up here: #594.

I have to dig into testing next, but wanted to see what everyone thought of the implementation.

[hzoo]

Nice, this is fixed by #711 thanks to @kaicataldo, will be in the next release! haha noticed this was a year ago