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

Better type documentation for components #58

Closed
mhkeller opened this issue Dec 21, 2021 · 5 comments
Closed

Better type documentation for components #58

mhkeller opened this issue Dec 21, 2021 · 5 comments
Labels
awaiting-merge Finished in a branch, will be included in next release

Comments

@mhkeller
Copy link
Owner

mhkeller commented Dec 21, 2021

Currently, all of the components have jsdoc comments but the vs code extension doesn't seem to recognize them well.

The component description only gets pulled in (when hovering over the component's import statement) if the format is such where the @type definitions are included in one big comment at the top immediately above the exported variables like this:

<script>
  /**
    Generates an SVG marker containing a marker for a triangle makes a nice arrowhead. Add it to the named slot called "defs" on the SVG layout component.
    @type {String} [fill='#000'] – The arrowhead's fill color.
    @type {String} [stroke='#000'] – The arrowhead's fill color.
  */
  export let fill = '#000';
  export let stroke = '#f0c';
</script>

<marker id="arrowhead" viewBox="-10 -10 20 20" markerWidth="17" markerHeight="17" orient="auto">
  <path d="M-6,-6 L 0,0 L -6,6" fill="{fill}" stroke="{stroke}"/>
</marker>

This yields a popup when imported like this, which is helpful because it has the comment description. It has undefined for the props though:

Screen Shot 2021-12-21 at 12 16 42 AM

In that format the popups over each individual property are also undefined and it pulls in that entire first comment, which isn't great:

Screen Shot 2021-12-21 at 12 17 17 AM

When the component is formatted like this:

<script>
  /**
    Generates an SVG marker containing a marker for a triangle makes a nice arrowhead. Add it to the named slot called "defs" on the SVG layout component.
  */
  /** @type {String} [fill='#000'] – The arrowhead's fill color. */
    export let fill = '#000';
  /** @type {String} [stroke='#000'] – The arrowhead's fill color. */
  export let stroke = '#f0c';
</script>

<marker id="arrowhead" viewBox="-10 -10 20 20" markerWidth="17" markerHeight="17" orient="auto">
  <path d="M-6,-6 L 0,0 L -6,6" fill="{fill}" stroke="{stroke}"/>
</marker>

The top-level hover now loses the main comment, but the individual props are nicely annotated:
Screen Shot 2021-12-21 at 12 18 25 AM
Screen Shot 2021-12-21 at 12 18 33 AM

But it still shows a lot of undefined fields.

Any thoughts?

@mhkeller mhkeller added the help wanted Extra attention is needed label Dec 21, 2021
@dummdidumm
Copy link

Do you have another extension installed which shows different hovers by any chance? For me, using VS Code with the official Svelte extension, it works as expected:
image

Documenting the default import of a Svelte file (the component): https://svelte.dev/faq#how-do-i-document-my-components

@mhkeller
Copy link
Owner Author

mhkeller commented Jan 10, 2022

Thanks @dummdidumm. That helped me figure out it was a Svelte intellisense plugin I had. I've documented the components now like this, following that link:

/** @type {Boolean} [gridlines=true] – Extend lines from the ticks into the chart space */
export let gridlines = true;
/** @type {Boolean} [tickMarks=false] – Show a vertical mark for each tick. */
export let tickMarks = false;

It doesn't say whether to document the props also in that html comment block. It works better if you add the comments inline for each prop. They don't show up when you hover over the component itself, but you get better hovers when you hover over each prop...

@dummdidumm
Copy link

Documenting the props like shown in your code snippet is correct. To also show them when hovering on the component itself - that's not supported yet, there's an issue for this where you can voice your opinion: sveltejs/language-tools#1308

@mhkeller
Copy link
Owner Author

Awesome, great to know!

@mhkeller mhkeller added awaiting-merge Finished in a branch, will be included in next release and removed help wanted Extra attention is needed labels Jan 11, 2022
@mhkeller
Copy link
Owner Author

This is now merged and released as 6.0.0 https://github.com/mhkeller/layercake/releases/tag/v6.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-merge Finished in a branch, will be included in next release
Projects
None yet
Development

No branches or pull requests

2 participants