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

ATA for tsserver on web #46862

Closed
wants to merge 3 commits into from
Closed

ATA for tsserver on web #46862

wants to merge 3 commits into from

Conversation

mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Nov 18, 2021

For microsoft/vscode#172887

This prototypes ATA for the web version of TypeScript. The logic is copied from the @typescript/ata project

The current implementation is not complete, mainly because I don't understand how to get the d.ts files into the TS project correctly

/cc @orta @DanielRosenwasser

For #45314

This prototypes ATA for the web version of TypeScript. The logic is copied from the `@typescript/ata` project

The current implementation is not complete, mainly because I don't understand how to get the d.ts files into the TS project correctly
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Nov 18, 2021
@mjbvz
Copy link
Contributor Author

mjbvz commented Nov 18, 2021

To test this:

  1. Build the TS changes using gulp local

  2. In the VS Code codebase

  3. Remove the line where we disable ATA on the web: https://github.com/microsoft/vscode/blob/f47ae9e324c8abdb15fe2effef94c5c4cb02c5aa/extensions/typescript-language-features/src/tsServer/spawner.ts#L209

  4. Run yarn compile-web

  5. Replace the built tsserver.web file with the one from the TS project. I do this with a symlink:

    ln -s ~/projects/typescript/built/local/tsserver.js ~/projects/vscode/extensions/typescript-language-features/dist/browser/typescript/tsserver.web.js
    
  6. Run yarn web to launch VS Code in the web

  7. Close all TS files. Having a TS file in the implicit project caused ATA to become disabled (we need to investigate the UX here)

  8. Open a JS file

  9. Write import 'jquery'

  10. You should now see suggestions for the global $

Note that at the moment the import doesn't actually get resolved properly. You don't get any intellisense if you are typing in the {} for import {} from 'jquery'. I believe this is because I'm not registering the d.ts file with TS properly

@orta
Copy link
Contributor

orta commented Nov 29, 2021

Oh cool, I missed this. Yeah, where the source of truth for the files lives is a bit of a tricky one. There's a few potential options:

  1. Use the TSServer APIs to write the files in whatever is the 'source' for the other files. E.g. when the ATA comes back saying add node_modules/@types/abc/package.json then use whatever APIs we use in the emitter ( basically writeFile on a CompilerHost) to tell the other side to write that file to their disk/vfs

  2. Have the TSServer host the VFS files, where it understands that extends internal calls to readFile / readDirs and when it's node_modules it also looks inside the ATA acquired files to see if it should include them

I feel like the first one is probably more logical overall?

@mjbvz
Copy link
Contributor Author

mjbvz commented Dec 8, 2021

@orta Good point. My current approach would never be able to support features such as go to definition because VS Code would not have the acquired file content.

I wonder if we should have the web typing installer delegate the actually installation back to VS Code. That's the first approach I considered with this PR, but it seemed overly complicated when I thought about it. However that seems like it could cleaner than using the emit apis (which VS Code doesn't use at all currently)

To implement type installation on the VS Code side, I think we need:

  • A message / event where TypeScript can tell VS Code to install some typings
  • Some way for VS Code to tell TS that the Typings have been installed
  • On the VS Code side, a way to add a document to the workspace such that it is picked up by TypeScript but does not actually get committed

Is that similar to what you were thinking?

@Rheeseyb
Copy link

Rheeseyb commented Dec 9, 2021

I wonder if we should have the web typing installer delegate the actually installation back to VS Code. That's the first approach I considered with this PR, but it seemed overly complicated when I thought about it. However that seems like it could cleaner than using the emit apis (which VS Code doesn't use at all currently)

To implement type installation on the VS Code side, I think we need:

  • A message / event where TypeScript can tell VS Code to install some typings
  • Some way for VS Code to tell TS that the Typings have been installed
  • On the VS Code side, a way to add a document to the workspace such that it is picked up by TypeScript but does not actually get committed

Is that similar to what you were thinking?

I've been thinking about this a lot (and tried knocking something together from your PR to explore it), and there is still the issue that with this ATA solution TS would be using the latest versions of every package, meaning a high likelihood of providing incorrect typings. However, it seems like what you've suggested above actually goes part way to a solution for full type acquisition. In essence, it seems like the above is needed in both directions, since:

  • Where possible, VS Code needs to tell TypeScript which versions of the packages to use (based on the nearest package.json)
  • For imports from the local project, VS Code needs to provide that imported file to TS, and it to install that file's typings (which of course means traversing and providing that file's dependencies too, probably meaning some back and forth as TS would need a way to request the missing files, or VS Code would need to provide them all upfront, and then there's the issue of keeping those files in sync since VS Code becomes the source of truth)

For a bit of context, we're specifically interested in this for utopia, where previously we were running a tsserver worker in the browser to build the user's project (providing all files upfront, storing them in memory via BrowserFS, updating and emitting the incremental build results after each file change), so that we then could provide the full set of types to Monaco (as we were using that as the file editor at the time) - maybe that's an avenue that would be of interest?

@jakebailey
Copy link
Member

After microsoft/vscode#184438 I think this one's no longer up to date or where we're going.

@jakebailey jakebailey closed this Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants