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

Sveld auto doc of Props and Accessibility #364

Closed
wants to merge 1 commit into from
Closed

Sveld auto doc of Props and Accessibility #364

wants to merge 1 commit into from

Conversation

niktek
Copy link
Contributor

@niktek niktek commented Oct 13, 2022

DO NOT MERGE - this is for previewing

What does your PR address?

Fixes #351

Notes: This actually uses https://github.com/mattjennings/vite-plugin-sveld which in turn uses sveld.

To differentiate between props that are Props and props that are for Accessibility, it is using magic strings in the description, if a11y appears in the description, it will be moved into the Accessibility tab.

To enable this for a components doc page, add components={["Accordion/AccordionGroup","Accordion/AccordionItem"]} to the DocsShell's props. It handles a plain string for a single component and an array of strings components.

Although it handles migrating the Props tabs over incrementally (it replaces the props DocsShellTable, it can't be done for Accessibility due to extraneous tables being added like KeyBindings that sveld can't detect.

I've done the Accordion components as an example to show what's needed. They are not for publishing yet, but rather to show what's needed and how different scenarios work.

Where a type has a union of possible values, it must be declared using the @type directive, otherwise only the last type of the union is declared from the auto-inference.

Vite also complains mightily about doing dynamic imports like this, so some more testing needs to be done of a full published docs site. There is a fallback option if issues arise, but it's uglier to wire up.

@endigo9740 endigo9740 marked this pull request as draft October 13, 2022 21:06
@endigo9740 endigo9740 mentioned this pull request Oct 14, 2022
27 tasks
@ryceg
Copy link
Contributor

ryceg commented Oct 16, 2022

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
Copy link
Contributor

@ryceg see my response on Discord.

@niktek niktek marked this pull request as ready for review October 18, 2022 03:39
@endigo9740 endigo9740 marked this pull request as draft October 18, 2022 03:53
@endigo9740
Copy link
Contributor

This will still be available for reference in the closed PRs, but closing for now.

@endigo9740 endigo9740 closed this Oct 21, 2022
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.

Update to utilize JSDocs
3 participants