Update to utilize JSDocs

[endigo9740]

Describe what feature you'd like. Pseudo-code, mockups, or screenshots of similar solutions are encouraged!

@niktek confirmed we can use JSDocs comments to add additional intellense information for component props. We should aim to implement these asap

What type of pull request would this be?

Enhancement

Any links to similar examples or other references we should review?

If possible we should look into a way to share the same data between the JSDocs comment AND the docs page UI description, so we're not repeating ourselves. Assuming this is possible.

[niktek]

https://github.com/carbon-design-system/sveld is the most promising looking option so far and might give us more than just what jsdoc can provide.

[endigo9740]

Suggested by Rhys on Discord:

What we could do is use jsdoc2md to generate .md files, and then consume them via mdsvex

[ryceg]

Ideally, we want to avoid duplication; if we are documenting the usage of a component within the component, ideally, we would be able to generate the https://skeleton.brainandbonesllc.com/components/* docs from them, obviating the need for manual doc generation.

[endigo9740]

From Nik in Discord

---

Throwing this out for general input: Docs site - pretty much everyone else is autogenerating from single source of truth i.e. components.

Goal is to have single source of truth for things like props/methods of a component, which can be derived from the code using JSdoc so that it helps intellisense as well as the docs site.

Post process insertion of auto generated content of pnpm build is not an option due to chunking Pre process of pnpm build is an option - but feels super hacky chunking content in and out based on a placeholder

Most other doc sites use Markdown in one way or another for the human generated portions (Usage, examples etc) - which in theory should make it more accessible for contributors to help with.

Then there is all the full on solutions like storybook and histoire for auto genning doc sites at the component level and things like JSDoc at the code level. Histoire looks the best out of these as it is Vite based and integrates well with Svelte and also handles things like CSS displays - https://svelte3.examples.histoire.dev/story/src-introduction-story-js

All that said, pretty much every solution looks like a big revamp of what we have (unless we go pre-process) - so i would love any other solutions thrown in that other people have used.

[endigo9740]

https://github.com/leveluptuts/bookit

[endigo9740]

@ryceg let me know if you wish to take this on as discussed on Discord. I'll assign to you.

I'd advise you create a draft PR asap so Nik and I can monitor progress and provided feedback. @niktek's PR is available for here if you two want to work together on the Svelte integration, perhaps in parallel: https://github.com/Brain-Bones/skeleton/pull/364

I'm sure he wouldn't mind answering questions about what he setup.

Otherwise I'll return to this mostly likely next release cycle, as my docket is going to be fairly full between now and the middle of next week (next release).

[ryceg]

I'll take it! I love docs ☺️

[endigo9740]

Assigned!

[ryceg]

So Sveld is mostly fit for purpose, but there's a couple deficits that get in the way of us using it fully for documentation, which I think is a very good long-term goal to aim for.

The Sveld interpreter is notably missing many of the JSDoc tags, which is working against us, but isn't actually a dealbreaker- it just interprets everything as an extension of the description, and includes new lines. If we use JSDoc tags, then we can use @ as the opening tag, and /n as a closing tag. For a robust documentation system, I think that we would want to make use of some custom tags, easy enough to set up. For writing the logic for dealing with tags, it might be good to keep in mind the different tag kinds (https://tsdoc.org/pages/spec/tag_kinds/)

Sveld has a rudimentary version of this to handle slots; @slots is kinda janky as it's closer to an inline tag, rather than a block tag, and only lets you define a couple different parameters. Notably, we do not expose any information about what props are passed through to slots at the moment in our current documentation, which is definitely sub-optimal. We also break from Svelte architecture a little bit by defining whether a slot is "required" to be filled; typically Svelte slots are entirely optional, whereas we have elements that (rightly) have a need to be filled, like the details component. Adding in some extra logic into the Sveld interpreter to allow us to tag it with a JSDoc custom modifier tag @required would be needed to make the slots stuff work for our needs, but we could always fake it with a passed prop (but I'm really loathe to do that).

[endigo9740]

@ryceg see my response on Discord.

[endigo9740]

@niktek @ryceg I've completed my review of Sveld and kicked the tires on it in test project. Here's some of my notes and findings.

@extend for Parent/Child relationships

https://github.com/carbon-design-system/sveld#extends

In some cases, a component may be based on another component. The @extends tag can be used to extend generated component props.

If I'm reading this correctly, is this not the intended way to denote that this is a child component?

Regardless I'm feeling less confident about the need for @childOf and @parentOf. These seems unnecessary when the documentation page spells this out. If we really need this info I'd recommend using the @component, which provides a great catch all for descriptions not built into Sveld by default.

Use of vite-plugin-sveld versus the standard Sveld plugin

Seems I went down the same path you guys did in regards to this. I was having trouble getting the Svelte to work as a rollup plugin. Though I feel I was doing something wrong. Per these instructions it sounds like this is possible: https://github.com/carbon-design-system/sveld/issues/57#issuecomment-1118101139

Funny enough, I actually know Tropix, the user posting the thread. I interacted with him a bit regarding a11y early into the open source launch of Skeleton. However that brings me to my next topic...

Fluent Svelte

https://github.com/Tropix126/fluent-svelte https://fluent-svelte.vercel.app/

This is a UI component library built around Microsoft's Fluent design system. Surprisingly this is my first time seeing it. And look who maintains it - Tropix! Even better, if you review the codebase they are using Sveld. So this should be an excellent project for reference, plus we can reach out to Tropix with questions if we run into a wall.

Sveld Prototype

Here's my simple prototype:

Here's the documentation page with tables and raw data logged:

Here's the component definition:

This provides a wealth of great info, all using the stock settings described on the Sveld README documentation. There's actually very little I'd want to extend or change to be honest. I REALLY like that it auto-documents the default slot and any forwarded events. No JSDoc markup required. That's awesome!

Per code style guidelines - Sveld infers the type info pretty well, at least for basic types which the majority of our props use (string, number, etc). I wouldn't mind us defaulting to the single line method for JSDocs format, but expanding to multi-line only when needed. This will help keep the code a bit more compact and easier to scan for readability.

However, I did notice we're not getting HMR. It requires a manual refresh to see the data change. Do we think that's a side effect of how we're injecting the plugin? Perhaps another reason to look into using the vanilla Sveld plugin?

As for other missing custom tags:

• @required or @optional - perhaps we just use the description? Ex: "This field is required. Blah blah blah"
• @a11y - I think our current ARIA APG link in the doc shell will suffice. There's a lot of nuance that's going to be hard to convey in a small popup for this. I'd say we provide good semantic descriptions and call it a day

That said, if the Sveld author will accept a PR to extend with these features, then I'm all about it. But I don't feel comfortable having to maintain a fork. Then maintaining one project becomes two!

Alternatives

I'm not saying we need to give these any sort of attention, but for completeness these are the alternatives to Sveld that I've seen recommended in other places:

• https://github.com/alexprey/sveltedoc-parser
• https://github.com/acornjs/acorn

They go in a MUCH different direction, and unfortunately sveltedocs-parser seems to be abandoned.

[endigo9740]

@ryceg One quick follow up question - the only thing I did use was @typedef. How would we utilize this alongside Typescript interface definitions? The Conic Gradient and ConicStops type comes to mind here. Do we setup both a @typedef AND interface? The former would handle the page documentation, and the latter would be for type safety when setting up the component, correct?

[endigo9740]

@niktek @ryceg writing up my notes based on the current changes to Nik's PR here: https://github.com/Brain-Bones/skeleton/pull/412/commits

NOTE: see the accordion component, which contains all the latest changes and standards

Doc Shell

• I've reworked the layout shell with several notable changes, the biggest one being that we can now parse Sveld docs
• I've moved the ARIA APG link to the heading area - no more hiding that away in the a11y tab
• The a11y tab has become the "keyboard" tab, since the props will now exist in the Props tab
• I've nuked DocShells properties for props, slots, events, classes, etc. These are now part of settings
• I've created a small utility that runs on init that counts the combined props, slots, events, to dynamically enable tabs (see sveldCounts)
• I've updated the Tab if statements to use these count values
• Props/slots/events tab content are fed by pageSettings.sveld
• Classes are fed by pageSettings.classes
• Keyboard Interactions by pageSettings.keyboard
• I've updated the props, slots, and events tab to map the data and display UI per component instance
• Note that classes/keyboard tab panels have hardcoded table headings and retrieve data from pageSettings

sveldMapper.ts

• Renamed from "parser" to be more semantic
• Spit the single parser function into a set of smaller mapper functions per type (props, slots, events)
• These take in each component's Sveld docs data
• The return data table data for headings/source
• The headings and order fixed and hardcoded, for standardization
• The values are fixed, with minor HTML markup introduced

Settings Updates (DocsShellSettings)

• I've renamed a few values to be more semantic, but that should all be obvious
• The docs prop created by Nik is now settings.sveld, this contains props, slots, events, etc per component
• The sveld key accepts a label/description/source (source being the raw component docs)
• I've introduced settings.ariaApgLink to handle the ARIA APG link in the header
• I've introduced settings.classes, which accepts labels/table source (headings are hardcoded in the shell)
• I've introduced settings.keyboard, which accepts the table source (headings are hardcoded in the shell)
• These updates DRAMATICALLY reduce the amount of code need per individual docs page.

types.ts

• Several modifications to support the new settings and included JSDoc descriptions
• Dropped a few of the individual types (but I'll bring them back asap!)
• Moved the DocsShellSettings.parameters to the bottom, see my notes on this below.

What's Not Working

Most is working well, but here's a few things of note:

• I really do not like the splitting the description string. String parsing is about as fragile as things come. Let's investigate another way to handle this on Discord later please.
• There's a few data points that aren't mapping as I'd hoped with JSDoc. Let me know if you have ideas:
  • Props: already have isRequired but it's based on whether a default value is provided.
  • Context Props: have undefined type/value by default, so I'm excluding them in the mapper
  • Slots: no way to add a description that I can tell
  • Events: I've only tested with forwarded events, but dispatched events need the "mapper" adjusted slightly
• Per the context props mentioned above, see my proposed solution on Accordion item. Where we specify overrides in the sveld description.
• This is a big one - we haven't yet discussed how to handle parameter values for Actions! These are a lot like props, but actions are .ts files, not Sveld components. So I don't know if we can parse that info with Sveld. There's only a few, but we should review how we can handle these!

[endigo9740]

@niktek @ryceg FYI I've completed another round of updates and update Elements, Components, Actions, and Utility documentation pages to the new settings format. I've not yet began updating the JSDocs comments per each component yet, but that's what we can divvy up soon. For now I've just ensured everything is "wired up".

A few notable updates:

• Implemented a suggestion for handling description parsing PER data type (props, events, slots, etc). See the inline comments on the Mapper script
• I've made some minor adjustments to the Mapper script to accommodate some autodoc tables that weren't working. And fixed a couple edge case bugs
• I've DROPPED classes from all but Tailwind Elements. The value-add just wasn't there to keep those around
• I've folded Action param data into settings.parameters, which works similar to settings.keyboard. Still manually maintained, but we only have 4 instances (filters, clipboard, menu, tooltips)
• Renamed ariaApgLink to just be aria
• I've commented ALL manual docs for props, slots, events, and a11y. These can act as a reference when documenting each value inline with JSDocs. Please remove the comment block as you document inline please!

Still on my list to do:

• I've currently disabled setting object's description keys temporarily. These do not work with the current implementation. I have some ideas for fixing this though
• We'll need to evaluate my proposal for the description @tag parsing

Check the PR for the latest!

[endigo9740]

@niktek @ryceg FYI I've pinged the vite-sveld-plugin author on Nik's PR to see if we can get some movement on an update: https://github.com/mattjennings/vite-plugin-sveld/pull/2

Likewise I've pointed out that Sveld has updated to include two major features we need:

• https://github.com/carbon-design-system/sveld/pull/99
• https://github.com/carbon-design-system/sveld/pull/100

It's critical we have these. If we do not see movement on the plugin, we may need to fork and handle this ourselves. Additionally, we should revisit using Sveld directly as a Rollup plugin, since it seems to be technically possible. I believe this would cut out the plugin as the middleman here.

UPDATE The plugin maintainer has responded and will look to update to the latest version. We should still aim to use Sveld as a Rollup plugin though, as it's the path of least resistance.

[endigo9740]

I've pushed a few more adjustments today, including:

types.ts

• Improved the types, structure, and JSDoc descriptions for DocsShellSettings
• settings.classes is now a singular table source array, params/keyboard
• Renamed svelte -> components and sveld.source to settings.sveld to be more semantic
• Renamed Docs interface type to Component to be more semantic
• Split Component singular description to descProps | descSlots | descEvents

sveldMapper.ts

• I've changed Slot > fallback to just be a check for now. Components like the dropzone have way too much content in the fallback to show this in a table.
• I've updated Events > element to look more like a tag, like <div>
• Ive accomplished this using a new escapeHtml() function, which we might can reuse for Slot fallbacks in the future.

I think aside from getting Sveld updated to support Event/Slot descriptions, we've got everything we need to begin documenting each Skeleton feature using the auto-doc system!

[endigo9740]

@niktek @ryceg Unfortunately it doesn't appear we're going to be able to use Sveld's restProps feature. We always divide the $$props.class from the rest of the restProp attribute values. The class is applied to the parent, while the restProp values are applied a child within the temple, but with class pruned. Sveld can only handle this if restProps is applied directly to an element.

Here's an example of this scenario in the File Button component:

However, this seems like something we could easily build into the existing DocShell settings. With either a boolean for mentioning it's used, or follow Sveld's lead and specify the element that restProps is applied to. I'm keen to use the latter.

EDIT

I'll push this shortly, but here's how it'll operate for the four or so components that need this:

const settings: DocsShellSettings = {
	// ...
	restProps: 'input'
};

The UI, which links to here: https://svelte.dev/docs#template-syntax-attributes-and-props

[endigo9740]

@niktek @ryceg I've pushed one more set of updates which, unless we get some feedback from the Sveld author, will be what we merge later today. This includes:

• I've tested a few combination of comment formats for @slots. I've updated AccordionItem to use my recommendation. This uses a JS comment as the first line, which looks fine, but is an odd requirement for sure!
• For forwarded @event, we cannot add descriptions to these without breaking the type. Leave these default.
• For dispatched @event, I've updated the Paginator component as a reference for this. It's a lot like props.
• I've made a couple small changes to the Mapper:
  • prop > slot_props is currently disabled, we don't really use these AFAIK, but maybe we can in the future?
  • events > detail this has been added to show dispatched event "Response" column information

The Sveld author has responded, but nothing we can act on yet. Long story short, when using Sveld in our project the capabilities are more limited and don't match the Sveld Playground.

EDIT

Quick update, we were able to get a version of the multi-line comments working that works for @slot descriptions, but it still requires the proceeding // comment. I've committed a small update to this format as it reads a bit better.

Make sure you catch up on the conversation as it's progressed today, starting from here: https://github.com/carbon-design-system/sveld/issues/102#issuecomment-1290955841

EDIT 2

I've pushed an update to use the production release of vite-plugin-sveld v1.1.0

[endigo9740]

@niktek @ryceg Unless you guys see any other changes needed I will aim to merge the PR today. Then note I've included a checklist in the OP, which divvies up who will be documenting what. As mentioned on Discord let me know if this split works for you guys.

[endigo9740]

Some recommendations based on my usage of the current implementation:

• I've left the original descriptions in commented out blocks in the doc +page.svelte, feel free to copy those over. But make sure to nuke these when you're done so the file is clean!
• In a majority of cases we should only need to provide a description for props, basic types are picked up by default
• That said, CONFIRM each as you go, because custom types and context elements don't always set properly
• I am excluding props with undefined types (getContext values). Follow my lead for defining "redundant" getContext descriptions on the AccordionItem. These will be used purely for intellesense.
• Let's go ahead and convert any prop using string | undefined to string. ex: export let foo: string = ''
• Note the recommended @slot format, which requires the // leading comment. I've used this as a label: // Slots:
• DO NOT manually document forwarded events - this then makes them "dispatched" events, which is incorrect. Just let Sveld handle these automatically
• DO document dispatched events - just like props, handle this on the line above the usage. You don't have to do this more than once if the event is dispatched multiple times
• The Paginator component is setup as an example for dispatched events if you need a reference
• The @component comment in the HTML template can just utilize the same component description set in the documentation header for all parent components. Children should follow the pattern set within AccordionItem.

[endigo9740]

@niktek @ryceg Here's my updates from my audit of Nik's PR. These changes have now been been committed and pushed.

FYI, here's what happens when we use <code> in the JSDoc comments. It drops the word! Should be:

Set 0 to disable.

Svelte Components

• General:
  • Style: Let's keep loose @slot/@event declarations at the top of the component file
  • Style: Make sure to add the space at the end of JSDocs comments, ex: _*/
  • Style: Comments slashes should have a trailing space, ex: //_
  • I was in the midst of migrating from prop slotX naming to regionX, which is more apt
  • Use backticks instead of <code>, else the Intellesense popup hides the contents! (screenshot above)
  • Reminder that non-primitive types are set as 'undefined' and pruned by default! Specify a type for these!
  • BUG: Single line comments won't let me define type + desc, ex: /** @type {foo} desc /
  • @ryceg: would love for you to review our event typing throughout the project, needs some love
  • Implemented some additional key events to satisfy a11y warnings
• Alert:
  • (audit) no need for slotDefault prop class as content wraps it and title
• Avatar:
  • I've implemented a custom src prop so we can document it. Hate that this is required though!
  • For action I set a type of of function and changed to a block comment
  • For actionParams I set a type of string
• Breadcrumb:
  • (audit) we'll need some form of escaping to allow unicode separator to show, fine as is for now
• ConicGradient:
  • Set stops to type ConicStop[]
  • (audit) I agree with the default comment description being useful, though the first Usage example shows the use case
• File Button:
  • Seem this got skipped? I've added docs for this.
  • files was missing completely. See my comment about non-primitive types above.
• FileDropzone
  • I added missing dispatch event docs.
• ListBox/Item
  • Default values that are Svelte Stores do not show? Shows - instead. (ex: selected)
  • I added missing dispatch event docs.
• Paginator
  • amounts prop requires @type {number}, otherwise it is hidden in the table
  • Adjusted button description, no link required
• Progress Bar
  • value requires @type {number}, otherwise the prop was hidden in the table.
• RadioGroup/Item
  • Fixed a typo for display type of inoine-flex -> inline-flex
  • On second though, made type string, since any display class is valid. I would love to include recommendations though!
  • (audit) per item, we can't add stronger typing for value because it can contain literally any value (string|number|object|etc)
  • I added missing dispatch event docs.
• SlideToggle
  • I added missing dispatch event docs.
• Stepper/Step
  • (audit) since writable generics aren't available, using writable(number). Same convention ListBox.
  • JSDoc tooltips support markdown. We should use that instead of anchors. They work in the Intellesense popup!
  • Good call on absolute instead of relative URL too.
  • (audit) improved the step getContext types
  • Added missing @slot description
• SVGIcon
  • Thanks for including this!
• TabGroup/Tabs
  • selected wasn't displaying until I added a type. Used writable(any)
  • Added missing @slot description
• DataTable
  • Including current heading/source typing, Rhys may want to revisit later
  • (audit) Change search to type of string
  • Added missing @slot descriptions
  • I added missing dispatch event docs.

Utilities

• Drawer
  • Updated open to type Writable(boolean)
  • Adjusted the Drawer props desc slightly to indicate Drawer/Backdrop
• Toasts
  • (audit) position is based on the Tailwind convention, but we should probably sync with Drawer for this.

[endigo9740]

Pushed out a couple more quick changes:

• Delete some left over comment blocks in the doc pages
• Implemented a simple mechanic for displaying child override properties

This has now been merged into the dev branch. Remaining issues should be handle in their own separate tickets going forward!