add preset and plugins to parser options

[jharris4]

Q	A
Fixed Issues?	No
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	Yes
Tests Added + Pass?	No
Documentation PR Link
Any Dependency Changes?	No
License	MIT

As described in babel/babel-eslint#784 and #10752 (comment) this PR adds support for the presets and plugins babel options to the babel-eslint-parser package.

[kaicataldo]

My concern with this is that adding this means that the team is committed to maintaining use cases for @babel/eslint-parser outside of it being a tool that allows ESLint to lint files compiled with Babel, which I believe is out of scope for the project. This happened to be a case that was supported before because the plugins were hard coded into the source code (please note that plugins were not configurable before the v11 prereleases).

Given the size of the team, I'm not convinced we should be committing to spending resources that way. I'm curious what the rest of the team thinks regarding this.

[nicolo-ribaudo]

@kaicataldo Could we just forward the babelOptions object to Babel's config loading utility function?

[jharris4]

@kaicataldo Could we just forward the babelOptions object to Babel's config loading utility function?

I would say that that's basically exactly what this PR is doing, forwarding more options to Babel's config loading utility...

[kaicataldo]

@nicolo-ribaudo I think we could. For historical context, here is the discussion where we decided what to include.

Are you envisioning we allow all configuration options? As I mentioned above, I believe @babel/eslint-parser should really be only used when someone is actually using Babel (rather than a catch-all experimental syntax parser for ESLint), because that adds a lot of maintenance overhead (and I don't think serves the Babel community well, since we end up having to support a number of different use cases).

That being said, I also think it's confusing to cherry pick options that we pass through like we have in the current iteration.

[kaicataldo]

I'm definitely happy to defer to consensus on this.

[jharris4]

My concern with this is that adding this means that the team is committed to maintaining use cases for @babel/eslint-parser outside of it being a tool that allows ESLint to lint files compiled with Babel, which I believe is out of scope for the project. This happened to be a case that was supported before because the plugins were hard coded into the source code (please note that plugins were not configurable before the v11 prereleases).

Given the size of the team, I'm not convinced we should be committing to spending resources that way. I'm curious what the rest of the team thinks regarding this.

The sole use case for this PR is to simplify linting code that is being compiled with Babel using the same configuration options that Babel itself provides.

As described in the babel options documentation, the main options that you use to configure babel are presets and plugins. Additionally, you can specify a configFile option as a file that contains those presets and plugins inside it. Without this PR, the configFile is the only option the babel-eslint-parser package supports for configuring which presets & plugins are used by Babel.

Pretty much every babel config out there is primarily just presets and plugins. If the PR was adding some of the more obscure options I could better understand the hesitation. But all this PR is doing is allowing people to choose whether the 2 main options are passed directly, or via a file, while using core Babel code to do so.

I know lots of people here are very attached to having Babel configuration always be loaded from a self contained file, but please don't block those of us that want to pass the same Babel configuration programatically from other JavaScript code! :-)

[jharris4]

@nicolo-ribaudo I think we could. For historical context, here is the discussion where we decided what to include.

Are you envisioning we allow all configuration options? As I mentioned above, I believe @babel/eslint-parser should really be only used when someone is actually using Babel (rather than a catch-all experimental syntax parser for ESLint), because that adds a lot of maintenance overhead (and I don't think serves the Babel community well, since we end up having to support a number of different use cases).

That being said, I also think it's confusing to cherry pick options that we pass through like we have in the current iteration.

Sorry, I hadn't seen this comment as it came in while I was writing my own comment.

I could see concern at maintaining every option under the sun and having to keep track of them all and keep them in sync. But like I said in my last comment, presets and plugins are pretty foundational to how you configure Babel, and I'd really like to see this PR finally get merged so I can stop having to fork this package (and writing the same PR over and over haha) just to be able to pass in primary/standard babel config options :-)

[kaicataldo]

Would it make sense to allow for a full configuration object instead of passing through individual options? So configuration precedence could then be:

1. If a full configuration object is given, pass it right through and use that.
2. If a config file path is given, resolve the module and load that configuration object.
3. Otherwise, use the standard Babel config-loading logic (which I believe would be the majority use case).

Also, thanks for participating in this discussion. Now is the perfect time to get this kind of feedback and figure these APIs out before we finalize them for the next major release.

[kaicataldo]

What I would really like to avoid is the confusion of having configuration loaded through multiple mechanisms and merged in hard-to-debug ways. i.e. I don't think configuration passed through an .eslintrc.js should override or be merged into the config resolved by Babel's internals, because this could get unwieldy really quickly from a maintenance perspective.

[jharris4]

While presets & plugins are all that my use case requires, I'd lean more towards giving more (all?) options versus limiting to just one (configFile).

I totally get what you're saying though about wanting to avoid confusion. In fact for a wide range of (closed source) projects that I maintain for work, we have devised one common configuration file for babel, webpack, eslint, jest etc that makes it easy to configure & update lots of projects at once.

The outline of the config file looks like this:

export const config = {
  babel: {
    presets: [ /*...*/ ],
    plugins: [ /*... */ ]
  },
  eslint: {
    rules: {
      /* ... */
    },
    plugins: [ /*... */ ]
  }
}

We then have a eslint helper binary that just loads this config file, parses the babel section & eslint section and passes those to the real eslint binary as configuration options.

It's pretty cool to build tools that foster/encourage integration between different tools like this, and furthering this aim is what motivated me to make this PR. ;-)

[nicolo-ribaudo]

What I would really like to avoid is the confusion of having configuration loaded through multiple mechanisms and merged in hard-to-debug ways. i.e. I don't think configuration passed through an .eslintrc.js should override or be merged into the config resolved by Babel's internals, because this could get unwieldy really quickly from a maintenance perspective.

I see what you mean, but that's how currently other Babel integrations work. If you use @babel/core, babel-loader or rollup-plugin-babel, you can directly pass a Babel config that is merged to the other configs.

Currently, the config precedence is:

• Options directly passed to @babel/core, babel-loader or rollup-plugin-babel
• .babelrc.json
• babel.config.json's env and overrides sections
• babel.config.json's top-level section

(I can't find docs for this; we should write them)

[kaicataldo]

Understood. In that case, it sounds like passing through a configuration object directly to Babel seems like the option that's the most in line with other tools that integrate with Babel. If the rest of the team is on board with that, I'm 👍 to that. I think I allowing for all options makes more sense than cherry-picking specific configuration options and passing them through.

[jharris4]

Shall I modify this PR to pass through all options, or do you think it makes sense to do that in an entirely separate PR?

[nicolo-ribaudo]

@hzoo @existentialism @JLHwung What's your opinion on this?

[kaicataldo]

Sorry for my absence - I've been sick. I'm still of the mind that we should pass through the options.

[vikr01]

@kaicataldo Could we just forward the babelOptions object to Babel's config loading utility function?

This makes sense. The parser can export a "recommended" (or maybe "legacy") config that uses the current default options for the parser.

[kaicataldo]

This makes sense. The parser can export a "recommended" (or maybe "legacy") config that uses the current default options for the parser.

I don't think we should we do this. Folks should pass through the options they want.

[nicolo-ribaudo]

We should only force-enable the estree parser plugin.

[kaicataldo]

So, just to be completely clear, I don't think this iteration of this PR is the correct solution. I'm going to take a look at this today and see what this would actually entail.

@jharris4 Sorry for all the back and forth. Appreciate you bringing this to our attention!

[jharris4]

@kaicataldo Thanks for considering this!

It will be great to have all the babel options available for passthrough.

I'm happy to help craft the PR or review if that'd be helpful.

[kaicataldo]

Closing this now that #11639 has been released.

[kaicataldo]

Thanks for contributing!