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

Adding JSDoc type support #9

Merged
merged 5 commits into from
Apr 7, 2020
Merged

Conversation

jasonlyu123
Copy link
Member

@jasonlyu123 jasonlyu123 commented Mar 19, 2020

Propose to add JSDoc type support to language-server

  • Add method to language server host to distinguish script kind of svelte script
  • Monitor script type attribute and notify typescript script kind changes Already done
  • The default script kind would be changed to js, check for if it make any changes.
  • Checking for // @ts-check Handle by ts language service
  • getSemanticDiagnostics for js
  • Add svelte/types to default tsconfig/jsconfig.json, mainly needed runtime/ambient.d.ts
    require Update service.ts to fix the errors with imports #8 to work

-- Shared With TS

More info will be added later

@orta
Copy link
Contributor

orta commented Mar 21, 2020

Cool idea!

@jasonlyu123 jasonlyu123 marked this pull request as ready for review April 2, 2020 03:23
@jasonlyu123 jasonlyu123 changed the title [WIP] Adding JSDoc type support Adding JSDoc type support Apr 2, 2020
@jasonlyu123
Copy link
Member Author

The default script kind would be changed to js, check for if it make any changes.

I checked my codebase and doesn't find any issue. But if someone do use typescript and doesn't "correctly" set script lang attribute, it may have some problem though.

Changes of this branch includes:

  1. Now ts won't complain about component have no default export. This also works when no workspace svelte dependency.
  2. ts-check now provide type check on script of js
  3. On script of js, language-service now recognize JSDoc type and won't suggest you to move it to ts type

About reactive statement and prefixed store variable, we could add JSDoc declaration in preprocessing. More work will needed to be done after typescript's work about this is done. For now, I think this branch is ready for review/merge.

@dummdidumm
Copy link
Member

Looks good to me!

Reactive statements / $-signs should be handled in a preprocessor later on. We also need to think about passing the whole svelte-file in to get rid of "unused method"-stuff and to be able to provide intellisense inside mustache-parts of the html (on:click={someFn}). But as you said, that should come later.

@jasonlyu123
Copy link
Member Author

@dummdidumm also <script context="module"> . Like you said in the overview thread, the fragments solution do needs some rework

@dummdidumm
Copy link
Member

@orta any chance you could get a look at this PR and my other two PRs this week and possibly merge them in if they are ok with you? (sry if I'm rushing you)

@orta
Copy link
Contributor

orta commented Apr 7, 2020

OK, cool, this looks good 👍

@orta orta merged commit e26f4e1 into sveltejs:master Apr 7, 2020
@jakobrosenberg
Copy link

Is this available now and if so how would I enable JSDoc support in a Svelte file?

@jasonlyu123
Copy link
Member Author

jasonlyu123 commented Apr 8, 2020

@jakobrosenberg not release yet. You could wait for beta version to release or build it from source.
---- updated
Release in 99.0.3 of svelte beta

@jakobrosenberg
Copy link

jakobrosenberg commented Apr 8, 2020

@jasonlyu123 thanks for the advice. I cloned it and ran yarn and yarn build, but now I'm not sure what to do next in order to add it to VSCode.

EDIT: Got it working by opening it in its own window instead of as part of a workspace and was then able to run the debug function to launch a new instance of VS Code.

However, I'm getting no error highlighting, even though Check JS is enabled in settings.

@dhonx
Copy link

dhonx commented Apr 8, 2020

@jakobrosenberg I will do the same thing I think

@jasonlyu123
Copy link
Member Author

@jakobrosenberg for now you can only use vscode debug (see README) to try it out or use some hack to replace svelte-language-server bin with the compiled one.
@orta Maybe it'll be good idea to have a option to config custom path to svelte-languge-sever and also publish beta/alpha version of svelte-language-server to npm.

@dummdidumm
Copy link
Member

dummdidumm commented Apr 8, 2020

Since #25 we have nightly deployments if commits happened on master, so theoretically - if the deployment succeeded - you should have this feature in the new svelte beta very soon.

@orta
Copy link
Contributor

orta commented Apr 9, 2020

It failed - but I fixed it d02a85d, and there's a new deploy 99.0.2!

@dhonx
Copy link

dhonx commented Apr 9, 2020

@orta Why the version is 99?

@jakobrosenberg
Copy link

@dummdidumm any chance you know how to:

A) enable highlighting of type errors in .svelte files (I've enabled Check JS in VSCode and it works in plain .js files)
B) enable types for modules imported from a package with a svelte key in the package.json?

@jasonlyu123
Copy link
Member Author

@jakobrosenberg I doesn't implement vscode's checkJS option, doesn't know there is an option for this. I'll make a new PR later. For now, you can set checkJS in tsconfig.json or jsconfig.json, also you could use // @ts-check directive at the top of script

@orta
Copy link
Contributor

orta commented Apr 9, 2020

it'll never follow semver, because the version is totally automatic - so just maxed it out to make that obvious

@jasonlyu123
Copy link
Member Author

@jakobrosenberg I found that svelte-language-server and the extension doesn't use vscode's typescript setting but has some similar settings under svelte.plugin. So even if I add a new setting to enable global checkJS, it won't be the one you mention.

About your second question,

package with a svelte key in the package.json

can you get me some example of what you mention?

@jakobrosenberg
Copy link

@jasonlyu123

{
  "name": "@sveltech/routify",
  "version": "0.0.0-development",
  "description": "Routes for Svelte, automated by your filestructure",
  "main": "lib/index.js",
  "svelte": "runtime/index.js",

The svelte key is what is being resolved within Svelte apps, but code completion doesn't work, since VSCode uses the main key.

Could not find a declaration file for module '@sveltech/routify'. '... routify/lib/index.js' implicitly has an 'any' type.

@jasonlyu123
Copy link
Member Author

@jakobrosenberg I know what you means know! Thanks for point that out, this is not implemented yet. I'll open a new issue for this, Thanks!

@jakobrosenberg
Copy link

jakobrosenberg commented Apr 23, 2020

JSDoc support has been working great for me, but all of a sudden inline imports in index.d.ts are now exported as any.

image

image

image

EDIT:
I might have solved this one.

Regardless. Is there a better approach to handle exporting types?

@jasonlyu123
Copy link
Member Author

@jakobrosenberg typescript 3.7 added this useful feature, I stumble upon this a few days ago. It can emit declarations for js written with jsdoc type. You can use tsc with --declaration, --allowJs and --emitDeclarationOnly flags to build declaration

@jakobrosenberg
Copy link

jakobrosenberg commented Apr 26, 2020

@jasonlyu123 thank you that was very helpful. After declaring types and replacing the imports from ./runtime/helpers to ./types/runtime/helpers it's now working.

@jakobrosenberg
Copy link

Sorry if this is not related to JSDoc or this pull. If I have a .d.ts file which uses types from another .d.ts file, types only works if both files are open in my editor at the same time.

image

Errors are gone when I open the definitions file.
image

@jasonlyu123
Copy link
Member Author

I don't know how to configure it to make ts always read those global type definitions. The only solution I come up with is to move it to .ts file and import it. @orta Do know another solution?

@ryanatkn
Copy link
Contributor

Check "include" in your tsconfig?

@jakobrosenberg
Copy link

Thanks so much guys. I had completely overlooked the facts that tsconfig handles more than just compilation. I had it it configured to just emit declarations from .js files.

@jasonlyu123 jasonlyu123 deleted the js-doc-support branch June 20, 2020 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants