feat!: Support async parsing #87
Conversation
| ## Open Questions | ||
|
|
||
| * Do we want to explore parallel parsing-linting as described in https://github.com/eslint/eslint/discussions/14733 in this RFC or separately? | ||
| * Assuming that we plan to implement both `async parse()` support and Flat Config for v9.0.0, can we split `FlatLinter` out of the `Linter` now? The work on `async parse()` overlaps a lot with the work on flat config. With the current dual-mode `Linter`, it doesn't seem feasible to start implementing support for `async parse()` before the final v9.0.0 flat config changes are merged. If we split out `FlatLinter`, most of the work could be finished much earlier and we could have a working support for async parsers in the "flat" mode (the mode that uses `FlatESLint` class). |
There was a problem hiding this comment.
This is an idea worth exploring. I’m a bit concerned about the effects on the ecosystem of changing formerly sync methods to be async. We don’t really know all the places that Linter is used, so such a change could break a lot of places we just aren’t aware of yet. (Maybe @bmish could modify his GitHub search he used for rule formats to see where Linter is being used?)
Tying the new async support to flat config could be a nice transitional phase to smooth over this change. There would be the open question of how long to keep the old Linter class around in this case.
Note that we ultimately don’t want the word “flat” in any interface names as flat config will be the only config eventually.
There was a problem hiding this comment.
My guess would be that it makes it prohibitively difficult to maintain an eslint plugin that’s compatible with both 8 and 9 (assuming 9 is what adds this).
There was a problem hiding this comment.
@ljharb do you know of any eslint plugins that use the Linter class? That sounds like a very unusual use case to me.
There was a problem hiding this comment.
There was a problem hiding this comment.
Keep in mind that the ecosystem is more than just plugins. We need to consider other tools that may wrap ESLint in some way, too.
There was a problem hiding this comment.
- https://github.com/sarbbottam/eslint-find-rules/blob/6dabfb759697e7d946bcfd485be0b7f584f2c0a0/src/lib/rule-finder.js#L74
- https://github.com/yannickcr/eslint-plugin-react/blob/9e12d2b5054230e366f18fdaa9ad44d952a4b950/tests/lib/rules/jsx-uses-react.js#L39
are all i found in a quick search.
Linter#getRules and Linter#defineRule wouldn't be affected by this change (they will be removed by eslint/eslint#13481 though, but that's off-topic for this RFC).
I should have been more precise, the question is which plugins and tools use Linter#verify or Linter#verifyAndFix, as those are the only public methods that will be changed by this RFC. The change would be especially problematic for plugins because rules and processors cannot be async.
There was a problem hiding this comment.
In the first several pages of a GitHub search for linter.verify() in code, all of the results are in test files. There are a few instances of linter.verifyAndFix() outside of tests.
| ## Alternatives | ||
|
|
||
| ### Alternatives to `async parse()` / `async parseForESLint()` | ||
|
|
There was a problem hiding this comment.
At the moment I have function configs disabled in flat config due to the complexity of getting it working.
There was a problem hiding this comment.
Another option: we could actually do the async loading inside of the ESLint class as part of preprocessing and then stick the loaded module directly into the config that is passed to Linter.
There was a problem hiding this comment.
Another option: we could actually do the async loading inside of the ESLint class as part of preprocessing and then stick the loaded module directly into the config that is passed to Linter.
I've added this alternative and another one with top-level await.
async parse() / async parseForESLint()|
|
||
| ## Motivation | ||
|
|
||
| Per the [original issue](https://github.com/eslint/eslint/issues/15475), this feature will allow for the use of ESLint with custom parsers written in languages that compile to Wasm or other binary formats. |
There was a problem hiding this comment.
This motivation is speculative since there is no such asynchronous parser for now. SWC is a candidate but it has performance problem passing AST to JS. swc-project/swc#2175
There was a problem hiding this comment.
I’d say anticipatory rather than speculative. People won’t write parsers that require async loading until ESLint supports async loading. We want to allow for the possibility sooner rather than later.
There was a problem hiding this comment.
the import plugin would likely benefit massively from an async parser, that can make parsing multiple files concurrent across the whole codebase.
|
|
||
| * In a preprocessing step that we could add to the `ESLint` class. | ||
| * In an async function inside `eslint.config.js`, if the new flat config system supports [async function configs](https://github.com/eslint/rfcs/tree/main/designs/2019-config-simplification#can-a-config-be-an-async-function). | ||
| * With top-level `await` in `eslint.config.js`, if the new flat config system supports ESM configs. |
There was a problem hiding this comment.
ESM configs are fully supported. 👍
|
@btmills thoughts on this? |
|
|
||
| Most changes are straightforward. Functions that operate on results from other async functions will become `async` functions themselves, and they'll `await` those results. Functions that just `return` results from other async functions don't need to `await` those results, but we'll nevertheless convert them to `async` functions to ensure consistent signature. | ||
|
|
||
| `_verifyWithProcessor` will lint code blocks one by one, as before this change. The linting of the second code block will start after the linting of the first code block has finished, and so on. |
There was a problem hiding this comment.
Is this a note to simplify initial implementation and something that we could change later, or is this somehow a constraint of the design?
|
|
||
| Breaking changes for plugins are only related to `RuleTester`: | ||
|
|
||
| * `RuleTester` will require a testing framework or custom `it` handler. |
There was a problem hiding this comment.
Should we add a deprecation warning when itDefaultHandler is used to encourage people to customize it prior to v9.0.0?
|
The more I’ve thought about it, the more I don’t believe that we can do a cutover from sync methods to async methods. We just don’t know all of the places that the Linter methods are used assuming they are sync, and I keep finding more examples. For instance, the JavaScript editor for CodeMirror allows you to pass in a Linter instance: So, I think the only suitable path forward likely involves leaving the sync methods as-is and either creating async methods alongside or else creating a new class that only has async methods. |
|
If we shouldn't expect the ecosystem to implement an async API, is it worth having async equivalents at all? We know of two use cases for async parsing: using wasm parsers and allowing |
|
@typescript-eslint would also benefit from this by being able to parallelise the "project"-based (type-aware) parsing using TS. I've already done some investigation into this by imementing a custom CLI and it showed some tangible perf wins (typescript-eslint/typescript-eslint#4359) |
|
For wasm parsers, there are other ways mentioned in this RFC that we could do the async initialization. If we took that alternative and resurrected the discussion started in eslint/eslint#14139, would we need async parsing anymore? |
|
For @typescript-eslint - the goal would be parallel initialisation into multiple worker threads (one per TS project config). We would need async parsing so that we could communicate with the worker threads after the fact. |
I don’t think this is necessarily the right question. My point is that we can’t just replace sync methods with async ones because that will make it difficult for folks to migrate, as some circumstances currently don’t allow async calls (like inside of rules). That’s a harsh cutover without any migration path. Supporting both would give us a migration path while also allowing experimentation. It seems that people expect to be able to use async functions in places where regular functions are used. We’ve had numerous requests for both async parsing and async rules, and I think there’s good argument to do async all the way down to bring ESLint up to date with the rest of the JS ecosystem. So while we can punt on this for now, I fully expect that down the line we will have more requests and we will eventually need to solve it. |
|
While I’m not thrilled about maintaining both sync and async implementations, you’ve persuaded me that it’s the best route toward where we’d like to end up long term. Between async methods or an all-async class, right now I’d lean toward async methods to learn about the use cases. Eventually we’ll probably want an all-async class (perhaps using parallel linting as a carrot), but I doubt we know enough today to design that class’s complete API. If we get these methods right, we could include compatible versions in a new class in the future. |
|
Closing, as we will address this as part of the larger rewrite. |
Summary
This RFC proposes adding support for asynchronous parser API.
Related Issues