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

TypeScript Plugin #580

Open
dummdidumm opened this issue Sep 27, 2020 · 10 comments
Open

TypeScript Plugin #580

dummdidumm opened this issue Sep 27, 2020 · 10 comments
Labels
feature request New feature or request

Comments

@dummdidumm
Copy link
Member

dummdidumm commented Sep 27, 2020

Now implemented, please provide feedback

See this comment for more info

Original Post

The language-server has quite a few features by now. What's missing for the complete developer experience and intellisense though is crossing the boundaries of Svelte and JS/TS files. This can be achieved through a TypeScript plugin. The following features would be cool to have:

Open Questions

  • How to connect the plugin to the running language server?
  • A TypeScript plugin has to be synchronous because all language service methods are synchronous. Our code is asynchronous because of the Source Mapping initialization. How to work around that?
@dummdidumm
Copy link
Member Author

dummdidumm commented Jan 15, 2021

Dumping some thoughts:

  • A TS plugin needs to be synchronous, meaning all methods like getDiagnostics need to be synchronous. This is not possible to do right now since we use source-map which asynchronously generates a SourceMapConsumer. So either we switch the library or we do some kind of "use latest processed if available"-logic. What we definitely cannot do is reuse the language server connection and do requestX, await response.
  • Because we cannot use the language server like the LSP protocol intends. Maybe it's possible to make the TypeScriptPlugin from the language server available for direct use while making sure it uses the same instance as the LS.

Idea for an incomplete solution

In the client inside extension.ts, add typescript/javascript as languages to deal with. Add logic to not active if it's not a Svelte project. Enhance the language server to deal with events from TypeScript and JavaScript files, too. This means things like "don't return usages from TS/JS files when find usages was triggered in a TS/JS file, as these will be shown twice then" or "route textdocument change events to DocumentManager only if it's a Svelte file". This will not solve the diagnostic errors problem because we cannot silence the TS server. It also doesn't work for rename because VS Code seems to go through the extensions one by one and stop at the first one providing rename functionality, which is the TS server - that's kind of a showstopper because this is likely the most valuable addition. Go to definition/find references/autocompletion/hover would work.

@pushkine
Copy link
Contributor

Hey I worked on a proof of concept today

2021-02-15.02-18-31_2.mp4

https://github.com/pushkine/ts-plugin-svelte/

Beside obvious bugs where everything gets stale this doesn't yet work with Components, might be a conflict with declare global module "*.svelte" or some tweaking to do with svelte2tsx

Looks good to me though, what do you think ?

@dummdidumm
Copy link
Member Author

dummdidumm commented Feb 15, 2021

This is a nice POC!

I though about this topic a little more and there are to me two ways to get to a solution:

Option 1: A "real" TS plugin (variant 1)

  1. Create performance-tests (Add performance tests #676)
  2. Replace async source-map with sync sourcemap-codec + own litte mapping function similar to how I did it in this ESLint plugin PR. Optionally reuse that logic in that PR to simplify+fix our mapping logic (could solve "Line numbers must be >= 1" error on module block when unused vars in regular script block #666)
  3. Test if the changes in 2 did not hurt the performance using the tests implemented in 1. If there are no big performance hits, continue.
  4. Make all TypeScriptPlugin function synchronous - should be easily doable then because the only reason it was asynchronous was because of source-map
  5. Refactor out the functionality of autocompletion/rename etc of the TypeScriptPlugin into a separate package (which we hopefully do not have to deploy, only pull it out for internal reuse)
  6. Create a svelte-typescript-plugin. Both this plugin and the language-server use the internal package to provide TS/JS intellisense
  7. Incorporate the plugin into the VS Code extension. Add an option to enable the plugin (maybe off by default for a short beta time).

Steps 1/2 can be done in parallel, the others are best done one after another. If someone wants to help in one of these steps, please ping here so we can coordinate. Step 5 is best done by me or @jasonlyu123 because we know the code base best and this is a critical part.

Advante: A "real solution"
Disadvantage: Especially "get generated position from original position" is likely much slower. Fortunately, we don't have to do this often.

Option 2: A "fake" TS plugin

  1. Create a minimal TS plugin:
    • convert Svelte files through svelte2tsx
    • patch all messages and filter out everything related to Svelte files from the results except for rename and diagnostics. Leave diagnostics untouched (disadvantage: "X is declared here" messages/references from TS will show transformed tsx code). For rename, filter out Svelte file related changes, package them up into a command and send it to the language server for further processing.
  2. Update the language-server as outlined in my "Idea for an incomplete solution" for the other intellisense features

Advantage: We keep the language server as-is mostly
Disadvantage: Incomplete "diagnostics"-solution? Will this work for rename?

Option 3: A "real" TS plugin (variant 2)

This is a combination of option 1 and 2: Create a TS plugin which converts Svelte files through svelte2tsx and uses sourcemap-codec + custom mapping logic to get original positions. It will reimplement some of the logic inside the language server, which on one hand is some duplicated work, but it will keep the plugin clear of all additional logic/workarounds that are only needed within Svelte files.

Advantage: Separate, clean implementation
Disadvantage: Duplicated work

=========

Right now I think I prefer option 3. What do others think?

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Feb 15, 2021
This replaces asynchronous `source-map` with a synchronous mapping implementation using `sourcemap-codec`. According to the performance test there is no noticeable difference at file lenghts like these - `source-map` is better at really large files (thousands of lines), which is not the case for 99,9% of all Svelte files the user will edit with this. A further (according to tests not noticeable) performance improvment would be to return the decoded form of the source map from svelte2tsx (it used sourcemap-coded, too).

The mapping implementation could also serve as a reference implementation for synchronous source mapping for TS plugin (sveltejs#580)
@jasonlyu123
Copy link
Member

I also prefer option 3. Maybe we can export some functions from the plugin package or create another utility package to reuse some logic. For example, svelte-sys, source mapping utility, and source mapping workarounds.

@sastan
Copy link

sastan commented Mar 22, 2021

First: Thanks for all the great work you are doing! You are creating a really good DX.

Now to the question:

I have created a typescript plugin that powers Twind IntelliSense for VS Code.

Is it possible or would it be possible to have the completions/diagnostics from that typescript plugin within svelte files using the same implementation?

I'm happy to contribute if there needs something to be implemented here. Or could give me some pointers/ideas on how that could be implemented in userland?

If this is the wrong issue I will move that into a separate issue.

Thanks for your time.

@dummdidumm
Copy link
Member Author

Thanks for the kind words! This question is unrelated to this issue. This issue is about implementing a TS plugin for Svelte. What you want is to hook into the Svelte language server. Please open a separate issue for that.

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue May 3, 2021
Initially support
- rename (doesn't work properly for all kinds of renames yet; need to filter out references inside generated code)
- diagnostics
- find references (need to filter out references inside generated code)

This makes all files TSX hardcoded for now, it seems the TS server is okay with importing tsx into js

sveltejs#580
sveltejs#550
sveltejs#342
sveltejs#110
dummdidumm added a commit that referenced this issue May 4, 2021
Initially support
- rename (doesn't work for prop renames yet)
- diagnostics
- find references
- go to definition
- go to implementation(s)

This makes all files TSX hardcoded for now, it seems the TS language server is okay with importing tsx into js

#580
#550
#342
#110
@dummdidumm
Copy link
Member Author

There exists a TypeScript plugin now which comes packaged with the VS Code extension and which you need to enable through the settings. It also is available standalone as a npm package if you need to use it outside of VS Code.
The one limitation is that you need to save changes of Svelte files to disk before they can be picked up inside TS/JS files.

Please test out the plugin and provide feedback. If you find bugs, please open separate issues for them.

@NickDarvey
Copy link

It is still possible to compile the code with Rollup despite the Typescript errors, however this breaks the build step when using Sapper and Webpack with Typescript.

sveltejs/svelte#5817 (comment)

@dummdidumm should using this plugin resolve the 'has no exported member' issue with exported members from Svelte modules when building with webpack? Or is it only intended to help IDEs?

@dummdidumm
Copy link
Member Author

Only IDEs can use plugins, they are not loaded by tsc

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Sep 2, 2021
There was no negative feedback so far, so let's make discoverability better by asking if it should be enabled.
sveltejs#580
dummdidumm added a commit that referenced this issue Sep 2, 2021
There was no negative feedback so far, so let's make discoverability better by asking if it should be enabled.
#580
@Jojoshua
Copy link
Contributor

Jojoshua commented Mar 15, 2022

Just following up on #1413 . I am using v105.13.0 of the extension and I do have svelte.enable-ts-plugin : true but the issue still persists on my work machine. Home machine is working fine. Anything I do/provide to help diagnose the issue?

Just FYI updating vscode from 1.64 to 1
65 fixed my concern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants