Skip to content

Refactor: Remove createIsolatedProgram call from getProgramAndAST #5856

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
JoshuaKGoldberg opened this issue Oct 20, 2022 · 4 comments · Fixed by #8834
Closed

Refactor: Remove createIsolatedProgram call from getProgramAndAST #5856

JoshuaKGoldberg opened this issue Oct 20, 2022 · 4 comments · Fixed by #8834
Labels
accepting prs Go ahead, send a pull request that resolves this issue breaking change This change will require a new major version to be released locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. refactor PRs that refactor code only
Milestone
8.0.0

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Oct 20, 2022 •
edited
Loading

Overview

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

Lines 90 to 91 in 09c04fb

createDefaultProgram(code, extra)) ||
createIsolatedProgram(code, extra)

That last || createIsolatedProgram(code, extra) should never be hit. We believe it was a just-in-case fallback added long ago that is no longer (and maybe never was) needed.

Removing that case may make #5855 easier.

Marking as accepting PRs with the caveat that everything still has to work as expected. Which may or may not be doable. Good luck! 😄

This issue is from a pairing I had with @bradzacher going over the parser internals.
@JoshuaKGoldberg JoshuaKGoldberg added refactor PRs that refactor code only accepting prs Go ahead, send a pull request that resolves this issue labels Oct 20, 2022
@bradzacher
Copy link
Member

I remembered why it was there.
The isolated program allows us to provide a program always which allows us to power the parser services always.

Most of the time this is going to be useless as you can only get types for the current file and no dependencies, but it does enable you to access diagnostics for the file (variable usage, non type semantic errors, etc).

I added this to power the "no-unused-vars-experimental" rule way back so that I could leverage TS's unused variable logic, but unfortunately it took an average of 50ms per file to calculate diagnostics - which is too slow across an entire codebase.

So I think we should breaking change remove this. I can't think of a reason that someone would want this now.

To double check - does the TS language server API need a program? Or does it create its own?

@JoshuaKGoldberg JoshuaKGoldberg added the breaking change This change will require a new major version to be released label Oct 20, 2022
@JoshuaKGoldberg
Copy link
Member Author

Language server or language services?

In TypeStat I generate a language service without a program: https://github.com/JoshuaKGoldberg/TypeStat/blob/a43cac385b8a4fcb2263d94e28ce5059de29f159/src/services/language.ts#L42-L54

@bradzacher
Copy link
Member

service is what I meant yeah.
so if it's not needed there then we can definitely remove this from the APIs

@JoshuaKGoldberg JoshuaKGoldberg changed the title Refactor: Investigate removing createIsolatedProgram call from getProgramAndAST Refactor: Remove createIsolatedProgram call from getProgramAndAST Nov 9, 2022
@JoshuaKGoldberg JoshuaKGoldberg added this to the 7.0.0 milestone Jul 27, 2023
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Apr 4, 2024
Merged
3 tasks
@JoshuaKGoldberg
Copy link
Member Author

#8834 was merged into v8. ✅

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024
@bradzacher bradzacher added the locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. label Apr 14, 2025
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 breaking change This change will require a new major version to be released locked due to age Please open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing. refactor PRs that refactor code only
Projects
None yet
Milestone
8.0.0
2 participants
@JoshuaKGoldberg @bradzacher