Skip to content

[Public API request] make parseForESLint accept ts.sourceFile #774

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.

Sign up for GitHub

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

Jump to bottom
Closed
Quramy opened this issue Jul 29, 2019 · 6 comments
Closed

[Public API request] make parseForESLint accept ts.sourceFile #774

Quramy opened this issue Jul 29, 2019 · 6 comments
Assignees
@JoshuaKGoldberg
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: parser Issues related to @typescript-eslint/parser package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@Quramy
Copy link

Quramy commented Jul 29, 2019

I want a public API for TypeScript language service plugin ( this is related to #254 (comment) ).

API proposal

Signature

parseForESLintFromSourceFile(
    sourceFile: ts.sourceFile,
    options?: ParserOptions | null,
): ParseForESLintResult

Expected behavior

This new function should works almost the same as parseForESLint. However, the following points are different:

  • result.ast should be converted by @typescript-eslint/typescript-estree/src/ast-converter directly.
  • result.services.program should be undefined ( I want to skip to call createProgram)

Motivation

I develop a TypeScript language service plugin to integrate tsserver and ESLint, https://github.com/Quramy/typescript-eslint-language-service.

In tsserver, ts.LanguageService does:

  • tokenize source text and create an AST node. And the parsing is so fast because of ts.IncrementalParser.
  • provide ts.Program ( So ts-estree don't need call createProgram )

Of course, I can provide a language service plugin using only public @typescript-eslint/parser APIs. But they accept only string text. So source code's text would be tokenized twice if using these public APIs. It's so silly.

Now I use some internal APIs ( see https://github.com/Quramy/typescript-eslint-language-service/blob/v1.2.3/src/ast-converter.ts#L187 ) for the above performance reason. But depending on them is too fragile.

Please consider it.

Quramy
@Quramy Quramy added package: parser Issues related to @typescript-eslint/parser triage Waiting for team members to take a look labels Jul 29, 2019
@Quramy Quramy changed the title [Public API Request] make parseForESLint accept ts.sourceFile [Public API request] make parseForESLint accept ts.sourceFile Jul 29, 2019
@bradzacher bradzacher added enhancement New feature or request package: typescript-estree Issues related to @typescript-eslint/typescript-estree and removed triage Waiting for team members to take a look labels Jul 29, 2019
@ajafff ajafff mentioned this issue Aug 13, 2019
Open
@octogonz
Copy link
Contributor

I'm working on a toolchain that invokes the TypeScript compiler and also ESLint. Currently both of these tasks perform the full steps of: load source files, tokenize, parse, and perform semantic analysis. This is an expensive operation, so it's unfortunate to do it twice.

It seems that we could cut the time in half by sharing the ts.program object between the compiler and ESLint.

@Quramy Is that the same thing you are proposing here? (I'm unfamiliar with parseForESLint because currently I'm invoking ESLint directly, which loads the TypeScript parser indirectly.)

@bradzacher
Copy link
Member

bradzacher commented Oct 21, 2019 •
edited
Loading

Happy to accept a PR for both cases!
It's not something we'll actively work on ourselves, as there's little call for it for the majority of the userbase, and we've got our hands full with everything else.

Quramy has built a ts language server plugin to coordinate linting via the language server instance, instead of installing an eslint plugin.

For the case of accepting a ts.SourceCode file, you'd essentially be wanting a thin wrapper on top of the conversion logic. I.e. you essentially want a function which does this, without the SourceCode creator:

typescript-eslint/packages/typescript-estree/src/parser.ts

Lines 299 to 309 in 611dff3

/**
* Create a ts.SourceFile directly, no ts.Program is needed for a simple
* parse
*/
const ast = createSourceFile(code, extra);
/**
* Convert the TypeScript AST to an ESTree-compatible one
*/
const { estree } = astConverter(ast, extra, false);
return estree as AST<T>;

For the case of sharing a ts.Program, this is a bit of a larger problem which doesn't really fit into our codebase as well.
Our codebase is built around using ts.createWatchProgram, because it gives us a BuilderProgram, which is internally designed to be more amenable to the "incremental change" use case.

Our code (as of the commits I merged on the weekend) does a lot of manipulation of this program as well (in order to handle the IDE's persistent state usecase).

So you'd probably want something a bit different - a thin wrapper which accepts a set of programs ahead of the lint, and then when asked to parse a file, it iterates over the programs until it finds one that has the file, and then uses the same API above to convert the SourceFile.

@octogonz
Copy link
Contributor

Cool. Should I open a separate issue for my scenario? I might work on this if I get some time.

@bradzacher
Copy link
Member

You can if you want - there's no huge need to open a new issue.

@bradzacher bradzacher mentioned this issue Jan 13, 2020
Closed
@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@JoshuaKGoldberg JoshuaKGoldberg self-assigned this Oct 25, 2022
@JoshuaKGoldberg
Copy link
Member

I'll tackle the original request of making parseForESLint also accept a source file once #5834 is in.

@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Oct 26, 2022
Merged
3 tasks
@JoshuaKGoldberg
Copy link
Member

#5892 was merged into the v6 branch. This'll be available as part of our upcoming v6 major version! 🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@JoshuaKGoldberg JoshuaKGoldberg

Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: parser Issues related to @typescript-eslint/parser package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Quramy @JoshuaKGoldberg @octogonz @bradzacher