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

Investigate opening more files in inferred JS projects based on package.json presence #45558

Open
andrewbranch opened this issue Aug 23, 2021 · 1 comment
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.
Milestone

Comments

@andrewbranch
Copy link
Member

andrewbranch commented Aug 23, 2021

Separating out some of the discussion in #45109 since that issue encompasses both a bug and an effective feature request that needs to be explored.

Background

When you open a TS or JS file in an editor, TS Server tries to figure out what project to put that file in. It searches up the directory tree for tsconfig/jsconfig files (up until the folder opened by the editor, I believe), and if there is a project config file that includes the file in question, a “configured project” is created (or an already-open one is used). The TS Program created for these configured projects contains all the files that would normally be part of the compilation with tsc -p path/to/tsconfig.json. Since the Program contains everything, auto-imports and other language features will be aware of other files that are not yet open in the editor, as expected.

On the other hand, if no containing tsconfig/jsconfig is found, the open file goes into an “inferred project” that contains just the files that are currently open in the editor (that aren’t part of another project), along with all their recursive imports/references. This can create a confusing experience, since open editor tabs feel like UI state, but actually affect program state: e.g., globals defined in other files will only be found when those files are open, and auto imports will only be available from files that are open or already imported from open files. This is the default experience for JS users who have no idea what a jsconfig.json is or why they would want one, but often the solution to their flaky IntelliSense issues is just to add a jsconfig.json file containing nothing but {} to their project root.

Investigation

I hypothesized that we could give a better default experience basically by pretending that every package.json file had an empty jsconfig.json file alongside it, causing other TS/JS files recursively down the directory to be added to the project. Unsurprisingly, this is a huge oversimplification of how this could work, and after half a day or so of poking around in the code, I see a number of questions/problems deciding when to trigger this behavior and what files to include.

  • Obvious invariant: a file should not be part of an inferred project if it should be included by a configured project (findable within the opened folder). An inferred project should be the last resort for every file it contains.
    • Scenario: open folder /project and file /project/index.js, no config file found, but /project/package.json is present, so we want to include additional files in the folder. When we see /project/a/b/c/d/e/f/g/h/foo.js, we have to check each of those subfolders for tsconfig/jsconfig files. Could this be expensive? (We may already have readdir results cached in the process of finding the file?)
  • What if there is a configured project already open with many files included, but an additional, unincluded file is open that creates an inferred project. Is it still worthwhile to search the directory tree for additional files to go into the inferred project?
    • Default assumption is no—if a workspace mostly uses configured projects, the behavior of inferred projects for random additional files isn't what we're trying to target here.
    • But if we don’t include additional files in the presence of configured projects, it might become complicated to maintain the state of the inferred project if a configured project gets opened or closed—if a configured project closes, does the inferred project suddenly gain files? If a configured project is opened, does the inferred project suddenly lose files? This sounds undesirable.
    • Maybe we shouldn’t include additional files if a tsconfig/jsconfig is found (while searching for files to include) anywhere in the opened folder (recursive)?
  • Should this behavior be enabled for inferred projects containing TS, or only JS? What about a mix? What about JS and .d.ts?
  • What files should be excluded from wildcard inclusion?
    • .min.js
    • {dist,build,out}/?
    • Should/can we take a cue from other kinds of config files? .gitignore, .vscode, package.json fields?
    • If you have a ton of compiled/vendor code we included because our heuristics didn't filter it out, you end up with a worse perf experience, and the solution is manual config. We might be trading one non-obvious problem solved by a jsconfig for another.
  • Still, the happy path scenario for pure unconfigured JS projects is compelling. Most tempting approach is to try it only when we're pretty sure the workspace will never open a configured project.
    • Should the JS/TS VS Code extension just detect this and prompt to add a jsconfig file instead?
  • Not yet explored: how to detect and what to do for monorepo-like projects
@andrewbranch andrewbranch self-assigned this Aug 23, 2021
@andrewbranch andrewbranch added the Needs Investigation This issue needs a team member to investigate its status. label Aug 23, 2021
@andrewbranch andrewbranch added this to the TypeScript 4.5.0 milestone Aug 23, 2021
@DanielRosenwasser
Copy link
Member

Should the JS/TS VS Code extension just detect this and prompt to add a jsconfig file instead?

I think there's a big question on this one. We've also had similar issues around TypeScript with misconfigured projects, where too many files get included in some way and we've floated the idea of having the editor suggest creating a tsconfig.json. The hesitation is that this could get annoying at some point. We don't want editors to provide too many pop-ups while they're trying to work.

Perhaps there's an argument to be made for making things better for smaller unconfigured projects, and when it hits a tipping point where we say "this isn't going to work, you need to create a jsconfig.json". I'm not 100% sold on that idea to be honest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

3 participants