Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow registering parsers with Node.js API #13694

Closed
molisani opened this issue Sep 16, 2020 · 7 comments
Closed

Allow registering parsers with Node.js API #13694

molisani opened this issue Sep 16, 2020 · 7 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@molisani
Copy link

The version of ESLint you are using.
7.9.0

The problem you want to solve.
When using the ESLint class, it is possible to directly specify options.plugins. These plugins then get stored in the additionalPluginPool map inside that ESLint instance (src). When loading a plugin, if it is found in the additionalPluginPool, it does not trigger the ModuleResolver (src). This provides the ability to bundle this code in a such a way that does not rely on eslint's internal module resolution.

However, no such additional pool exists for parsers. When loading a parser, it immediately triggers the ModuleResolver (src). Therefore, it appears that there is no way to use custom parsers in ESLint without relying on the internal ModuleResolver.

Your take on the correct solution to problem.
Apply the same strategy used for plugins to parsers. Create a new, optional parsers property on ESLint.Options that would populate an additionalParserPool map. Then on parser load, check this additionalParserPool before moving to module resolution.

Are you willing to submit a pull request to implement this change?
Yes, but I'd have to check with my current employer first in regards to the CLA.

@molisani molisani added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Sep 16, 2020
@mdjermanovic mdjermanovic added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Sep 16, 2020
@mdjermanovic
Copy link
Member

Hi @molisani, thanks for the issue!

This provides the ability to bundle this code in a such a way that does not rely on eslint's internal module resolution.

Can you please provide more details about your use case, is the intention to use ESLint in browsers?

@molisani
Copy link
Author

molisani commented Sep 16, 2020

This provides the ability to bundle this code in a such a way that does not rely on eslint's internal module resolution.

Can you please provide more details about your use case, is the intention to use ESLint in browsers?

Sure, I can explain. This not for use in the browser. We currently have an eslint plugin + parser setup for projects that use one of our internal JS environments. Right now, users are required to install and configure these manually in order to run our custom lint rules (with the parser). However, we also ship a cli as our primary interface (they're how users build/run projects for this environment). We are investigating the solution of including eslint and these plugins and parsers as dependencies of the cli tool, so that users don't have to set up their own eslint environment. From there, we could use webpack or some other bundler to reduce the deployed size.

I first noticed this when I was attempting to use the ESLint Node API to lint a project in another directory. When I set the cwd to that directory, I noticed that it always tried to load parsers from that directory, rather than the one I was running in. It didn't quite seem like a bug, and after looking through the internals and discovering the strategy for plugins, it seemed like that would be a reasonable enhancement for parsers as well.

@nzakas
Copy link
Member

nzakas commented Sep 16, 2020

It's worth noting that this plugin strategy is going to change completely with the new config system in development. As such, we aren't making any further changes to the current config system (it will make achieving backwards compatibility a lot harder). With the new config system, ESLint will no longer be doing any module resolving and you'll be able to slide in any object from anywhere you so choose.

@molisani
Copy link
Author

As such, we aren't making any further changes to the current config system (it will make achieving backwards compatibility a lot harder).

Is the entire internal config system locked down now? Is there any way that this feature could be landed just for the Node.js API (ESLint) without making "public" (i.e. counting towards backwards compatibility) changes elsewhere in the config system?

It doesn't seem like to much code to implement, but if you're not making any changes to this part of the code I understand.

@mdjermanovic
Copy link
Member

Is the entire internal config system locked down now? Is there any way that this feature could be landed just for the Node.js API (ESLint) without making "public" (i.e. counting towards backwards compatibility) changes elsewhere in the config system?

It doesn't seem like to much code to implement, but if you're not making any changes to this part of the code I understand.

It doesn't look like a trivial change, because it should also account for context.parserPath field that ESLint provides to rules, which can then require and use the parser if needed (I think eslint-plugin-import is using this to build the dependency graph). The documentation for context.parsePath is a bit misleading, it's actually a full absolute path to the parser module if an external parser was specified.

If we add a feature to pass a parser object through the ESLint API, then we should probably somehow make sure that require(context.parserPath) from a rule returns the same object, and that seems like a big change for a config system that will be replaced soon. With the new config system, context.parserPath will be replaced with context.parser object.

We are investigating the solution of including eslint and these plugins and parsers as dependencies of the cli tool, so that users don't have to set up their own eslint environment. From there, we could use webpack or some other bundler to reduce the deployed size.

There are cli tools like standard and xo that use ESLint and plugins as runtime dependencies. I'm not sure if there are tools that successfully bundle them instead.

@nzakas
Copy link
Member

nzakas commented Sep 17, 2020

@molisani the new config system is fairly well defined. You can get more details by visiting the issue I linked to. I really don’t want to add something that will be deprecated once the new config system is complete.

@mdjermanovic we are actually deprecating context.parserPath as part of the config changes. It will be replaced by context.parser.

@molisani
Copy link
Author

There are cli tools like standard and xo that use ESLint and plugins as runtime dependencies. I'm not sure if there are tools that successfully bundle them instead.

After looking through xo it appears that they use require.resolve (src) to pull in parser dependencies. I believe that webpack (for example) supports bundling static usage of require.resolve so that should work. With that workaround I'll close this out. Thanks @mdjermanovic and @nzakas for the information and insight!

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Mar 18, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

No branches or pull requests

3 participants