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

TSServer responses requesting further files to be opened #45315

Open
DanielRosenwasser opened this issue Aug 3, 2021 · 5 comments
Open

TSServer responses requesting further files to be opened #45315

DanielRosenwasser opened this issue Aug 3, 2021 · 5 comments
Labels
Domain: TSServer Issues related to the TSServer In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 3, 2021

Background

Back in #37713, I suggested that TSServer could produce several look-up locations in go-to-definition in the absence of a full project; part of the concern was that in an environment where the TypeScript server is on the same machine as the file system, it's way cheaper to perform that operation. We never went that route, though @sheetalkamat did try to create miniature projects in partial semantic mode in #39476, and backed that out in #40026.

Since then, partial semantic mode has been generalized to work in web settings where neither the client nor the server have all the files available at once, but the client may be in a better position to see whether a given file exists. The most we do to leverage this is in @andrewbranch's pull request at #42539 which tries to construct paths from relative imports, but doesn't really work if your import needs to resolve to a .ts/.tsx file.

Idea

Coming back to the idea in #37713, we could produce a result that signals to the editor that TypeScript doesn't have enough files open to produce an answer - but if the editor opens some set of files on its behalf, then it might be able to.

In other words, some sort of response type for go-to-definition that has

  • A list of entities TypeScript would like to resolve, each containing
    • the specific file paths that TypeScript needs to try to resolve to load those paths

When this information is given to the editor, the editor can try to open the set of files and send fileOpen requests to TSServer with the appropriate file content. Once that's done, TypeScript can re-request go-to-definition to produce further results.

Beyond the file transfer, adding a single file to a small program is likely not to have considerable overhead for most users.

Risks/Drawbacks

  • If opening files actually means opening editor tabs. That might not be desirable, especially if these requests have a noticeable delay. If a user closes this tab (or the tab is "ambient" and navigating away ends up closing the tab), then a user might end up with a "thrashing" editor experience where TSServer keeps trying to reload content.
  • If a re-request results in another "open more files" response, then this can result in a cascading effect that might be very slow.

This mechanism

Related work

Relative References in Partial Mode

In #39476, @sheetalkamat had partial semantic mode try to construct a program out of only relative references. This was backed out in #40026.

unverified Go-to-Definition Results

In #42539, @andrewbranch had go-to-definition jump to files that might not actually exist. Editors had the option of handling these requests and occasionally suggesting creating a file if they didn't exist.

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus Domain: TSServer Issues related to the TSServer labels Aug 3, 2021
@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Aug 4, 2021

From today's editor sync:

  • Editors like VS Code are able to support fileOpen requests without actually opening the file. We don't have to always open tabs to open files in the editor.

  • An event might be a better strategy than signaling incomplete results. The event could signal to the editor that if capable, it should try to open certain files, optionally cancelling the results of the last request, or marking them as incomplete and then reconciling with the new results.

  • Be mindful of "barrel files" - branching might get very expensive.

    export * from "./a";
    export * from "./b";
    export * from "./c";
    export * from "./d";
    export * from "./e";
    export * from "./f";
    // ...
    export * from "./z";

    In theory, we could stop at them if things are too ambiguous. We could employ some heuristics based on the file path too of "most likely" place to look. But if we always end up at these hops, that is not ideal.

@andrewbranch
Copy link
Member

but the client may be in a better position to see whether a given file exists

How would this ever be the case? It feels like to do this correctly, either the server needs to fully know how to do module resolution, or TS needs to generate and send the full list of lookup locations for any reference it would like to be able to resolve, like you mention in #37713.

@DanielRosenwasser
Copy link
Member Author

One idea was that if clients have a directory tree available to them, that effectively provides existence checks.

@DanielRosenwasser
Copy link
Member Author

Spoke with @andrewbranch, we had discussions around

  • leveraging LSIF in some scenarios instead of this and deciding when certain details can be reused
  • to what extent we feel confident around what clients actually have (e.g. is any of this really useful for a web editor)

@DanielRosenwasser
Copy link
Member Author

  • We should consider whether or not there's some "eager" loading we want to do, and whether there's some sort of memory limit, and how we should bail out if we cross that.
  • We want to avoid any behavior that doesn't feel "predictable" in some way.
  • We think the assumption that the editor will always have a directory listing actually isn't true, so it might not be "cheap" to delegate that task to an editor. This is a pre-requisite for doing anything in this domain.
    • On top of that, it's unclear what sort of performance characteristics we can expect for loading up directory entries and file contents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: TSServer Issues related to the TSServer In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants