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 Roadmap via LSP #4518

Closed
orta opened this issue Mar 6, 2020 · 74 comments
Closed

TypeScript Roadmap via LSP #4518

orta opened this issue Mar 6, 2020 · 74 comments
Labels

Comments

@orta
Copy link
Contributor

orta commented Mar 6, 2020

Goal: TypeScript-style tooling in Svelte

Getting TS in Svelte might be a blocker for some, but I think the biggest underlaying advantage comes in that the "TS support" is just a stricter version of the JS tooling. Everyone using Svelte gets improvements in tooling from this, not just folks with TS.

This proposal makes a strong separation between the tooling and the svelte compiler. It explicitly minimizes the impact on core in favor of moving live-DX complexity to a svelte language server.

The language server provides editor support and CLI validation for working in svelte, not the core library svelte. This isn't a unique concept, it's based on the vue eco-system which has similar "extend HTML" constraints.

This pattern also allows different maintainers to work on JS/TS tooling independent of the svelte core team (which is how we do it in GraphQL), making it a good place for people who want to contribute to step up and own some of the svelte core ecosystem.

Three main steps to get there:

  1. Add support in core for transpiling TypeScript in tags script, but ignore type-checking. TS will always synchronously emit JS if it's reasonably legal JS. This is the only change required in core. When you encounter a TS script tag, you can look up whether they have TS/Babel in deps and throw if not. No need to force the dep.

  2. Create "svelte/svelte-language-tools" repo in main org:

    1. Add svelte-language-server
    2. Add svelte-vscode
    3. Create a new CLI tool which uses the LSP to validate - this makes LSP calls against all .svelte files to run TS/JS tooling for errors.
  3. Svelte Language Server improvements

Rationale

  1. This is to allow the editor support to make that assumption out of the box, and for a user to be able to safely write <script lang="typescript"> knowing that it will work with the core.

  2. Adding TypeScript support to Svelte can take a lot of tips from prior art done on Vue.
    Vetur acts as the higher level tooling framework for both providing the JS and TS tooling support in scripts.

    This pattern keeps most of the work in the LSP level, which means all editors can use it, the vscode extension would just be one client of many.

    The language server could turn into @svelte/language-server and the vscode-svelte can get some more maintainers added so that deploys can be done by others. If folks have other popular IDE integrations, they could live in this repo too.

    svelte check adds a way to validate on CI, which can handle folks without the extension not accidentally breaking things. Because TS will always emit JS even if it has type errors, on purpose.

  3. This is basically a "draw the rest of the owl"

Work on Svelte Lanaguge Server

Luckily, both vscode-svelte and svelte-langauge-server have already been built and worked on by @UnwrittenFun and he's interested in moving those to the org. They both are solid foundations to build upon, from looking through the code.

They're definitely good enough for JS today, and considerably better than nothing!

I think one of the first blockers on making it something recommended by default, is finding a way to make sure that money-labels (reactive designators) are handled in both JS and TS.

Because the $: syntax does more work than TypeScript can know about $: x = count + 1 would probably need transforming into something like const x = () => { count + 1 } as an in-memory transform, so that the variables exist in the "TS representation" of the LSP (but not on disk)

HTMLx

The quick route:

I think you can get autocompletion, but I'm not sure how feasible typechecking would be here in a cheap way.

For inside {} the LSP could make API requests to the known script tags above and request all the variables at the top level scope (basically the equivilient of adding a newline in that script and then checking what's available.)

In theory you could type check each one by making a unique TS document for all of them, with exposed globals from the main script - but that could be quite slow, and it's hard to say how well that could scale.

Longest term:

How TypeScript handles the LSP-ish server is by embedding it into the core codebase, perhaps over time it makes sense to give the LSP deeper access to the svelte compiler, so that the LSP can ask it questions about the document "what variables are available in this eval context?" as (I guess) you must have some of that information to do the compilation.

I'd see this as quite a long time away, and you can get a lot of value right now in the LSP without access to that.

The svelte language server could eventually split into a htmlx-language-server and the svelte version would wrap with svelte specific cases.

Things to think about

This opens up a lot of legitimate questions "like how does the tooling support x and y" - the answer to most of them is "that is something the language-server would have to handle" - the language server would need to keep track of files, their exports and more - it'll be a cool an interesting project for folks who care about the DX at editor level.

Anything in vetur today should be feasible for svelte.

/cc @octref who works on Vetur, and might have some insight.

@dimfeld
Copy link
Contributor

dimfeld commented Mar 6, 2020

My experience with the Typescript compiler API is limited to a few hours of poking around, but I would love to help out with this once the repositories are all set up.

@dragomirtitian
Copy link

I would love to help out with this. How can I get involved?

@orta
Copy link
Contributor Author

orta commented Mar 6, 2020

I'm going to give it at least the week to sit on the chance someone has a good blocker. I'd like to get an explicit "yes, this is OK" from @UnwrittenFun - because while it's legal to fork, and he's said it is a good idea before. I'd rather be sure.

In the meantime, I'll make a yarn workspace repo on my own user account which retains all the history of svelte-vscode and svelte-language-server and look into how their testing setup works. This can get transferred over as the base to work from.

As homework, I'd recommend reading the code for:

For folks who might be thinking this feels super complex types of code, don't worry, it really tends to be quite shallow code built on top of vscode-languageserver (because creating a LS is a reasonably common thing nowadays) - and a lot of the foundations are already set up.

@sjafferi
Copy link

sjafferi commented Mar 6, 2020

I'd also love to lend a hand!

@halfnelson
Copy link
Contributor

Sounds great! There is a project that has gone down this path that might be a great starting point:
https://github.com/simlrh/svelte-language-server/blob/feature/extend-ts-support/src/plugins/ts-svelte/service.ts as for handling the htmlx, https://github.com/halfnelson/svelte2tsx converts a svelte file to tsx for type checking. There is also a commandline tool that uses this infra

@orta
Copy link
Contributor Author

orta commented Mar 7, 2020

Thanks @halfnelson - I bet both of those tools have a lot of underlaying ideas we can get merged into the official LS tooling level!

/cc @simlrh

@jamesbirtles
Copy link
Contributor

Exciting stuff, absolutely a "yes, this is OK" from me 👍

@octref
Copy link

octref commented Mar 9, 2020

Happy to share lessons learned or answer questions, but I wouldn't have time to commit any code as I don't even find enough time to develop Vetur/VLS.

Just a notice that transpiling gets expensive and complex soon. Consider auto completion. Transpilation need to happen in < 50ms in the worst case for completion to show up fast enough for each keystroke. Besides, your compiler needs to reasonably transpile all invalid states while user inputs svelte-specific JS such as $: x = count + 1 one symbol by another. I wish you success, but Vetur doesn't transpile <script> regions in Vue. Only interpolations are transpiled.

@simlrh
Copy link

simlrh commented Mar 9, 2020

@orta I have pretty good svelte/typescript language server running locally using svelte2tsx, it does checking of both the scripts and the htmlx props, autocompletion, maybe some other stuff. I'll collect all the branches together and put them online somewhere so it's easy for others to try.

I think it's definitely the place to start, but does need some work - it's pretty incredible what @halfnelson achieved using string manipulation with https://github.com/rich-harris/magic-string, but I have some worries about correctness in edge cases, so I began a full rewrite of svelte2tsx using AST transformations (turning svelte htmlx ast into plausible/typecheckable babel jsx ast). I got about halfway through the necessary features, without source map support, before burning out on it (I don't use svelte for work so this is all hobby stuff). I'll put that online for reference.

@orta
Copy link
Contributor Author

orta commented Mar 10, 2020

Cool, so an update - doesn't feel like any blockers to the idea of merging and centralizing so far (other than, "there is more cool stuff we should integrate" - which is cool, we should try get that in)

I've started merging some of the tooling repos together: https://github.com/orta/language-tools and updating all the dependencies. If there's still no conceptual blockers I'll start poking folks to get this pushed into a new remote of sveltejs/langauge-tools on Friday.

@octref's point about $: x = count + 1 is legit, this might be a hard problem to solve performantly - however TypeScript can't infer that this creates a variable without some kind of transformation

@simlrh
Copy link

simlrh commented Mar 11, 2020

@octref's point about $: x = count + 1 is legit, this might be a hard problem to solve performantly - however TypeScript can't infer that this creates a variable without some kind of transformation

I think there are 3 related issues here:

First just to clarify, this is valid JS and TS, the $: is a JS label and the only problem this causes for type checking is the 'unused label' warning.

Second is how to type check the x variable, as the typical way to write these statements omits the variable declaration. I've been handling this just by expecting the user to do:

let x: number;
$: x = count + 1;

Third is the issue of parsing the invalid file while the user is typing. acorn-loose does this, and babel has recently added error recovery which I believe does the same thing - worth looking in to.

@porfirioribeiro
Copy link

porfirioribeiro commented Mar 11, 2020

For the $: blocks, why not allow some more concise interface like

let c = $(()=> count + 1);

Would be easier for TypeScript, as there's no magic syntax.
The generated code would be exactly the same, Svelte would see the code inside the lambda and do its transformations.
I would not mind loosing a bit of syntax sugar for a good tooling support

@PatrickG
Copy link
Member

Second is how to type check the x variable, as the typical way to write these statements omits the variable declaration. I've been handling this just by expecting the user to do:

Shouldn't this work?

$: x: number = count + 1;

This could simply be translated to:

let x: number;
const updateXorWhatever = () => {
  x = count + 1;
}

@simlrh
Copy link

simlrh commented Mar 11, 2020

Shouldn't this work?

$: x: number = count + 1;

Trying to parse this as TS gives an error, it seem to treat both $ and x as labels and number as a value. $: let x: number = count + 1 works ok for typescript but the svelte compiler throws an error "let is a reserved keyword".

In my humble opinion I'd be in favour of a not adding any new syntax, and doing the minimum to make existing svelte type checkable, hence simply adding let variable: type; before any reactive declarations. And that way you don't need transpilation at all.

@PatrickG
Copy link
Member

PatrickG commented Mar 11, 2020

@simlrh I thought the script content will be transformed before it will be checked by typescript. That's why I've written:

This could simply be translated to:

let x: number;
const updateXorWhatever = () => {
  x = count + 1;
}

@orta
Copy link
Contributor Author

orta commented Mar 13, 2020

Alright, doesn't sound like any blockers. Can an org admin please:

  • Create a new language services repo, and pull in orta/language-tools and I'll put some more polish in via PRs over the weekend

  • Make a #tooling (or #language-tools?) chat room in the Discord

I'm happy to handle incoming PRs for a while to help sveltejs/langauge-tools get bootstrapped (you can add me to the org, or make me a contributor to that repo), but I don't use svelte nor have constructive production experience with it. So ideally, like to hope that some of the folks in this issue end up taking over those responsibilities with some time - I'll be here to provide moral support :D

@antony
Copy link
Member

antony commented Mar 15, 2020

@orta you should have an org invite now giving you the access you need to transfer the repo(s) into the svelte org. 🚀

@orta
Copy link
Contributor Author

orta commented Mar 17, 2020

Thanks @antony!

Over the weekend I've built out https://github.com/sveltejs/language-tools and got it working as a solid set of foundations. Tests/CI/Debugging/etc.

Next up, this week, I'll try test a bunch of the tools with the community website and some open source examples and try give a sense of what a roadmap could look like for the tools in issues sveltejs/language-tools#6

You don't need to wait on me if you already have ideas - create issues! I'll be learning from scratch, so that comes with fresh perspective but does mean that I don't know what it's like to be doing something like this for a long time or for working on larger projects.

@parker-codes
Copy link

Just plugging this here in case it (or its concept) helps with the $: issue discussed above.

https://github.com/tree-sitter/tree-sitter

@Sholanki
Copy link

Sholanki commented Apr 8, 2020

I would love to help out in this. How can I help?

@GrygrFlzr
Copy link
Member

This would be a breaking change given that variables prefixed with $ indicates syntax for store variables.

@daarxwalker
Copy link

@GrygrFlzr So, unite them -> reactive prefix.

@malj
Copy link

malj commented Jun 8, 2020

@daarxwalker $ is a valid symbol for starting identifier names in JavaScript: https://mathiasbynens.be/notes/javascript-identifiers#valid-identifier-names. Adding special semantics would clash with user-defined variable names.

@daarxwalker
Copy link

daarxwalker commented Jun 8, 2020

@malj I know, what you mean, developers naming preferences, migrations to new syntax, etc... But Svelte isn't so standard thing, it's literally revolutionary, so if you add one simple special rule...don't see any problem. Honestly, who uses $ as starting symbol, maybe long time ago, with jQuery. Just trying find simple way :)

@daarxwalker
Copy link

Plus, Svelte as compiler (framework) doing things with own way, has some strict rules and developer have to follow these rules.

@marcusnewton
Copy link

@daarxwalker The suggestion is indeed valid, and could potentially resolve some issues with TypeScript, but the community sentiment is likely to keep the label syntax, even if an alternative was also added.

@daarxwalker
Copy link

@marcusnewton
I understand, what you mean. As you say, I'm looking for a way to elegant solve TypeScript syntax.

@Ciantic
Copy link

Ciantic commented Jun 8, 2020

It's pretty hard to gauge community sentiment if all the thumbs up here points towards replacing the $: syntax, maybe people here thumbing up are not representative.

However, I'd like to say in my opinion explicit syntax is better than implicit. After all you have to import all the other stuff from svelte too e.g. import { onMount } from "svelte"; adding a reactive there wouldn't make a much difference, it would just make it explicit.

I actually don't care too much how it's solved, it's just a bit verbose for a typescript right now, e.g.

  let urlUE: string;
  let titleUE: string;
  let facebookSharingUrl: string;
  let twitterSharingUrl: string;
  let linkedInSharingUrl: string;
  let whatsAppSharingUrl: string;
  let mailSharingUrl: string;
  let style: string;
  $: urlUE = ...;
  $: titleUE = ...;
  $: facebookSharingUrl = ...;
  $: twitterSharingUrl = ...;
  $: linkedInSharingUrl = ...;
  $: whatsAppSharingUrl = ...;
  $: mailSharingUrl = ...;

Looks a lot of syntax to me.

@daarxwalker
Copy link

daarxwalker commented Jun 8, 2020

@Ciantic
It's and option, but it's starting looks like Vue3 ;)
My solution:

  let $urlUE: string = '...'

@mbrowne
Copy link

mbrowne commented Jun 8, 2020

What's the problem to be solved regarding TypeScript again? Are we talking about this example from earlier in this thread:

$: x: number = count + 1;

These type annotations on the labels could be recognized and stripped out by the Babel parser. (See https://babeljs.io/docs/en/babel-parser#will-the-babel-parser-support-a-plugin-system for one possibility; another would be if it could become officially supported syntax enabled via an official plugin, like how it works with JSX).

The trickier question is how to actually do type checking on these. I think the labels would have to be transformed somehow to code that the TypeScript type-checker recognizes. This would complicate IDE integration as well.

Just some things to keep in mind while exploring options...

@benmccann
Copy link
Member

The way TypeScript support works is that svelte-preprocess processes the file and passes it to the TypeScript compiler and then to the Svelte compiler. We can probably support any syntax as long as svelte-preprocess handles it correctly. svelte-preprocess needs to be able to operate very quickly (for intellitype, autocomplete, etc. in the IDE) and today is just a regex, so it's best to keep that in mind while designing a syntax.

@Ciantic
Copy link

Ciantic commented Jun 8, 2020

@benmccann how about we do just this then:

$: foo = count + 1

before passing to typescript turn it to this:

/* $: */ let foo = count + 1;

TypeScript should be able to even infer the type.

Then when it comes from Typescript, transpile back to $:?

Right now the biggest issue with $: syntax is that it can't infer the type for some reason.

@orta
Copy link
Contributor Author

orta commented Jun 8, 2020

OK, this thread has gone pretty far off topic and I'm going to lock it. You're welcome to create individual threads about their own topics.

@sveltejs sveltejs locked as off-topic and limited conversation to collaborators Jun 8, 2020
@orta
Copy link
Contributor Author

orta commented Jun 27, 2020

Alright, popping back to give basically a final update. Today we declared the VS Code extension production worthy, and recently svelte-preprocessor became an official svelte project.

In my opinion, this is enough to call TypeScript support 1st class for Svelte.

  • The JavaScript and TypeScript tooling is solid, and does what you expect.
  • Use TypeScript pretty much everywhere in .svelte files
  • Use svelte-check to validate your builds

There is both official support for making TS files work in the compiler, and work in your editor.

There's a few docs + template issues to sort out but we're there and it took a bunch of work from a lot of folks to get there.

Specifically @dummdidumm, @jasonlyu123, @UnwrittenFun, @halfnelson and @octref who provided code, foundations and valuable advice. Y'all did great work. Thank you.

@orta orta reopened this Jun 27, 2020
@orta orta closed this as completed Jun 27, 2020
@sveltejs sveltejs unlocked this conversation Jun 27, 2020
@orta
Copy link
Contributor Author

orta commented Jun 27, 2020

There we go, that's the right button. Issues with how bits work can go on either the IDE level in the language-tools repo, or at the svelte compiler level in svelte-preprocess.

Docs issues can be new issues on this repo like normal 👍

@dimfeld

This comment has been minimized.

@jonolo6
Copy link

jonolo6 commented Jul 9, 2020

Hi all. Great work on this! Just getting into Svelte and TS is a huge bonus for me. Are there any issues that can be watched for when documentation will be added?

@benmccann
Copy link
Member

benmccann commented Jul 9, 2020

@opensas
Copy link
Contributor

opensas commented Jul 15, 2020

Hi everyone, contratulations con the great work

I followed this document and also tried starting directly with this template and I'm getting the following error when trying to import a .ts file from App.svelte:

App.svelte

<script lang="ts">
  import { todos } from './stores'
  export let name: string;

  console.log('todos', todos)
</script>

Error:

bundles src/main.ts → public/build/bundle.js...
[!] (plugin typescript) Error: Could not load [...]/svelte-typescript-app/src/stores.ts (imported by src/App.svelte): Debug Failure. False expression: Expected fileName to be present in command line
Error: Could not load [...]/svelte-typescript-app/src/stores.ts (imported by src/App.svelte): Debug Failure. False expression: Expected fileName to be present in command line

And this is my stores.ts

export const todos: number[] = [ 1, 2, 3]

Am I missing anything?

@Klustre
Copy link

Klustre commented Jul 15, 2020

@opensas You should open a ticket in the language-tools repo

@orta
Copy link
Contributor Author

orta commented Jul 15, 2020

Time to re-lock this thread I guess 👋 - this is all prod ready, we've just not had a big announcement for it all, you can read the announcement bits here: #5101

@sveltejs sveltejs locked as resolved and limited conversation to collaborators Jul 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests