Skip to content

Enhancement: Allow project service to provide type information for out-of-project files #7871

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
4 tasks done
JoshuaKGoldberg opened this issue Nov 2, 2023 · 3 comments · Fixed by #7752
Closed
4 tasks done

Enhancement: Allow project service to provide type information for out-of-project files #7871

JoshuaKGoldberg opened this issue Nov 2, 2023 · 3 comments · Fixed by #7752
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Nov 2, 2023 •
edited
Loading

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

typescript-estree

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

Splitting out of #7435: that issue indicates that the error message for trying to generate type information for a file not recognized by the tsconfig.json is not ideal. But as we see in #7752, if we don't check to make sure that the file is recognized... type information still works! This issue tracks: can we allow "unknown" (not included in the TSConfig) files to have type info generated by the project service?

Not needing to explicitly include files in the TSConfig would be ideal from the perspective of simple configs. It'd mean we can get rid of tsconfig.eslint.json-style strategies of having a different TSConfig for your linted code. Those are commonly used to enable allowJs for root-level config files (e.g. eslint.config.js) not included in the project's TS-only tsconfig.json.

We can't enable that kind of "config-less typed JS linting" with traditional project: true styles of approaches because of the following combination of tech restrictions:

  1. The default program in TypeScript is a "builder": each time you add a new file to it, it recreates & recomputes a large swathe of nodes and types.
  2. ESLint does not provide us with APIs around when files are added, updated, or removed. We're restricted to just setting everything up once in the parser. See feat: parsing session objects eslint/rfcs#102.
  3. As a result, what was renamed to DEPRECATED__createDefaultProgram in v6 is known to cause significant slowdowns in non-trivial uses

If we can use the project service in a way that doesn't suffer from the same drawback, then that'd be a massive win for users.

Additional Info

Shoutout+thanks as always @jakebailey for helping us with the TypeScript side!
@JoshuaKGoldberg JoshuaKGoldberg added enhancement New feature or request triage Waiting for team members to take a look labels Nov 2, 2023
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Nov 2, 2023
Merged
3 tasks
@jakebailey
Copy link
Collaborator

The default program in TypeScript is a "builder": each time you add a new file to it, it recreates & recomputes a large swathe of nodes and types.

Definitely want to do perf testing here to make sure that this actually the case.

@bradzacher
Copy link
Member

Would need to dig into it but in the past when I've looked it seemed like the only way to have an "mutable" program that allows you to change things like adding files or changing contents was a builder program. So I'd assume that the language service uses a builder program as it's backing tooling so that it can mutate the program in response to the IDE changes. Maybe it uses something else?

Some backstory into past perf work:

Since the genesis of type-aware linting we have used a builder program in the parser. Eventually we found (or rather Ben Lichtman first discovered) that if you passed a plain old ts.Program through and skipped the builder program step - everything was much faster.

Ben had a usecase at MSFT iirc where he'd already generated a ts.Program in some tooling and wanted to pass that to our parser to save duplicated effort. Once he implemented an API for that he found the math didn't add up - the total lint runtime (external ts.Program generation plus lint run duration) was lower than before.

Profiling lead us to determine that essentially when you ask the builder to provide you with the latest program it had to do a bunch of work to check for changes and such before it could give you the ts Program. This work was done even if nothing actually changed and was a source of massive slowness in the CLI runs where all code is immutable.

James prototyped a solution that created a ts.Program and skipped the builder code paths if some heuristics said we were in a CLI run and we saw massive perf wins.

@JoshuaKGoldberg
Copy link
Member Author

JoshuaKGoldberg commented Nov 13, 2023 •
edited
Loading

Marking as accepted as a follow-up to #7761 (comment): that we could take in fallback out-of-project file support. As in, for the specific case of some random root-ish-level config file (eslint.config.js, etc.), it'd be fine to have a potentially slightly slower program API call if that file allowlisted by some opt-in setting. For example, allowDefaultProjectForFiles: "./*.js".

That's not the same as allowing out-of-project file information by default - even with EXPERIMENTAL_useProjectService: true. We haven't yet figured out how to wrestle the combination of ESLint and TypeScript APIs into submission to do that efficiently. See the comment just before this one.

@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Dec 6, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request
Projects
None yet
Milestone
No milestone
3 participants
@JoshuaKGoldberg @jakebailey @bradzacher