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

Documenting components with a docstring that appears on hover documentation #280

Closed
fnune opened this issue Jul 5, 2020 · 16 comments
Closed
Labels
feature request New feature or request Fixed Fixed in master branch. Pending production release.

Comments

@fnune
Copy link
Contributor

fnune commented Jul 5, 2020

Problem

With React's tooling, you can comment on a component like this:

/** Documentation that will appear on hover in other places where this is imported */
function MyComponent() { return null }

However, in Svelte, I haven't found a way to do this.

Solution

I would like to know where I can place a comment that will be taken up by the on-hover documentation of editors like VSCode and Vim/Neovim (with coc.nvim).

To illustrate, here I'm hovering MyComponent but I only see import MyComponent:

image

I would like my docstring Documentation that will appear on hover in other places where this is imported to be included in the tooltip.

I've googled around for this for a while and tried my local setup with TypeScript and I can't find a way to do it. I found an interesting related discussion on metadata for Svelte components but the discussion is more about prop types.

Edit: since we're discussing whether this is better added to the compiler, I created an analogous issue here sveltejs/svelte#5102

@dummdidumm dummdidumm added the feature request New feature or request label Jul 5, 2020
@dummdidumm
Copy link
Member

Since this could also be valuable for components without a script tag, I propose using the HTML comment for that. svelte2tsx would then look for HTML comments starting with a special tag (like @doc) and put the content as a jsdoc above the default export.

Opinions? Other ideas?

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

I would be really interested in having a go at implementing this. However I think it needs to be designed carefully because it's a feature some devs use very often every day.

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

Something that comes to mind is:

/**
 * @file Here is my documentation for this component
 */

At the start of the file. But I think only HTML comments work there, right?

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

Also, maybe to make it more similar to the other tags, I feel like a <svelte:documentation> tag akin to <svelte:head> could be nice, but that would need to be implemented in the compiler.

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

Perhaps even <svelte:options documentation="blabla" />?

@jasonlyu123
Copy link
Member

That will also needs some work in the compiler and the approval of the main repo. The options got verify by the compiler.

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

I created an analogous issue on the main repo, let's see if we can reach some conclusion 👍

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

Half of it is done once #282 gets merged. The LSP wasn't showing documentation strings on hover.

Now what I need to do is source a docstring from a @doc HTML comment and add it to addComponentExport in svelte2tsx.

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

@dummdidumm Currently it's not possible to add docstrings to the default exports, since they're not named:

export default class {
    $$prop_def = __sveltets_partial(render().props)
    $$slot_def = render().slots
}

I suppose, since the only things that can go on the root scope of each file are render and the default export, then it's harmless to give it a generic name like Component? Or maybe pick from the name of the file?

@dummdidumm
Copy link
Member

Yes that should be no problem.

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

Does any of this code ever end up being part of the runtime of an application, or is it there just for the tooling support?

@dummdidumm
Copy link
Member

Just for tooling.
About that default export: we need to check how that affects the import autocompletion.

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

Right hmm. I don't understand how it works currently. If I try to emulate it in a TS-only environment like this:

// Component.ts
export default class {}

I don't get autocompletions for Component on other files, but on Svelte it works somehow. I should probably read the related code in the LSP.

Anyway, if I do this instead:

// Component.ts
export default class Component {}

Then I do get autocompletions for the default export as Component on other files. So I support the safe bet is to name the default export the same as the file, for now, and then test it live.

@fnune
Copy link
Contributor Author

fnune commented Jul 5, 2020

Alright, so I've tried two options, but with our LSP, the only thing that works for autocompletion of component imports is:

export default class {
  // etc
}

Things I've tried:

  • Declaring a class and then exporting it as default in a different line.
  • Adding the name right in the export default class Name line.

Neither of them works. I'm now looking into the completion provider (https://github.com/sveltejs/language-tools/blob/master/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts) to see where the culprit is, since in TS-only environments I get completions for default exports of named classes, based on the name of the class.

Update: declaring a class and then export it as default in a different line works as long as the name of the export is the same as the file name. I'm going for this option.

@dummdidumm
Copy link
Member

Yeah the import completion for svelte components has some extra code there, essentially constructing the import suggestion. Maybe that code can be thrown away if svelte2tsx has a named default export which is the same as the file name.

@dummdidumm dummdidumm added the Fixed Fixed in master branch. Pending production release. label Jul 7, 2020
@fnune
Copy link
Contributor Author

fnune commented Jul 7, 2020

Fixed via #285

@fnune fnune closed this as completed Jul 7, 2020
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 Fixed Fixed in master branch. Pending production release.
Projects
None yet
Development

No branches or pull requests

3 participants