[docs] adds jsdoc for most functions in runtime API #7235

ehsan2003 · 2022-02-09T07:19:52Z

Before submitting the PR, please make sure you do the following

It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
Prefix your PR title with [feat], [fix], [chore], or [docs].
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.

Tests

Run the tests with npm test and lint the project with npm run lint

what happened ?

this PR adds jsdoc for most functions in runtime API
some parameters are renamed for better readability and more meaningful names
some interfaces are exported ( they were used in public api but didn't exported ) and they are mostly renamed ( it does not cause breaking change because its a new export )

additionally the fallback parameter off crossfade is merged into single interface ( it was separated )
#7230

dummdidumm

Thank you! Generally I think these are good additions. I'm not sure though if we need the docs to start with the function signature, that is something the user knows already from the other IDE visualizations.

src/runtime/transition/index.ts

src/runtime/internal/Component.ts

Co-authored-by: Simon H <[email protected]>

ehsan2003 · 2022-02-09T09:30:04Z

that is something the user knows already from the other IDE visualizations.

its kind of obscured in typescript and not reads well
Its not really annoying for the user
additionally I think there is still a way to reference to jsdoc from docs ( for example through a build script for docs so these docs will be attached here ) so its easier to maintain docs and keep it in sync with jsdoc and code

benmccann · 2022-02-11T19:48:24Z

I would agree we should not include the function signature. If there's some tool that doesn't display the function signature well we should fix that tool rather than updating all of our documentation

src/runtime/internal/Component.ts

benmccann · 2022-02-11T19:53:10Z

I'm also not sure we need examples of these functions being called

ehsan2003 · 2022-02-12T05:07:45Z

I'm also not sure we need examples of these functions be called

Its some thing helpful during development

why not ?

Co-authored-by: Ben McCann <[email protected]>

ehsan2003 · 2022-02-12T05:31:25Z

I would agree we should not include the function signature. If there's some tool that doesn't display the function signature well we should fix that tool rather than updating all of our documentation

what about signiture of transitions?
for example :

transition:fly={params}

benmccann · 2022-02-13T04:32:35Z

Quite a lot of them don't add anything. E.g. the first one I see in the PR is component.$destroy(). But that's already pretty obvious, so all it does is take up space and make it harder to keep the docs in sync with the code

bluwy · 2022-02-13T04:38:15Z

Looking at src/runtime/motion/spring.ts, src/runtime/motion/tweened.ts and src/runtime/store/index.ts, I think we should focus on updating the jsdoc only for now. Adding new type exports and renaming the variables would be better done in a separate PR so we can get the documentation updates in first.

Re transition:fly={params}, I think this could be an exception and is fine by me if it's in the jsdoc. Maybe with the @example tag. Not sure of other's opinion on this though.

ehsan2003 · 2022-02-13T06:09:54Z

Quite a lot of them don't add anything. E.g. the first one I see in the PR is component.$destroy(). But that's already pretty obvious, so all it does is take up space and make it harder to keep the docs in sync with the code

component.$destroy() is obvious even in the svelte.dev/docs but it doesn't mean that it shouldn't be there !

Jsdoc could reduce the need for referring to documentation for developers most of the time.

bluwy · 2022-02-13T07:05:27Z

component.$destroy() is obvious even in the svelte.dev/docs but it doesn't mean that it shouldn't be there !

If you're typing:

const app = new App({ target: document.body })

app.$d|

where | is the caret position with intellisense, the syntax alone already implies component.$destroy(). I think that's where it feels redundant as the IDE helped filled in the gap there.

Also side note, it looks like when I test the above example in VSCode. There're already JSDocs from Svelte2TsxComponent.

ehsan2003 · 2022-02-13T07:26:09Z

component.$destroy() is obvious even in the svelte.dev/docs but it doesn't mean that it shouldn't be there !

If you're typing:
const app = new App({ target: document.body })

app.$d|
where | is the caret position with intellisense, the syntax alone already implies component.$destroy(). I think that's where it feels redundant as the IDE helped filled in the gap there.

Also side note, it looks like when I test the above example in VSCode. They're already JSDocs from Svelte2TsxComponent.

so It might be a problem for component jsdoc. but my main Idea was about the runtime functions for example svelte/store and ...

specially I find it really annoying to refer to documentation every time I want just know what stiffness does in spring function for example !

ehsan2003 · 2022-02-13T20:35:15Z

Looking at src/runtime/motion/spring.ts, src/runtime/motion/tweened.ts and src/runtime/store/index.ts, I think we should focus on updating the jsdoc only for now. Adding new type exports and renaming the variables would be better done in a separate PR so we can get the documentation updates in first.

I generally agree.

bluwy · 2022-02-14T05:58:33Z

specially I find it really annoying to refer to documentation every time I want just know what stiffness does in spring function for example !

We could have just text to describe the properties of the option, which I think this PR already does, but I think the point was that we should avoid having example code in the jsdoc unless it provides more value that the text can cover. Now in most cases, it would be better with text only.

It'll also benefit if we can get shorter jsdoc since inline documentation should be more straightforward than a full documentation. Of course, writing/adding docs is subjective though so I can't blame you with the first pass of the PR 😬 For now, it would be great to keep it simple and enhance it later in the future.

ehsan2003 · 2022-02-14T10:45:02Z

We could have just text to describe the properties of the option, which I think this PR already does, but I think the point was that we should avoid having example code in the jsdoc unless it provides more value that the text can cover. Now in most cases, it would be better with text only.

you know I am really a fan of Rust and its ecosystem
there are many tutorials even inside the docs ( for example docs.rs/warp )
the docs examples are running just like normal tests in rust
even keywords in the language contain some thing like docs.

and almost all crates in rust ecosystem have a good documentation through inline docs which is really helpful and that was a great experience.

In most IDEs the full documentation is not showing up so the users will see what their expect at the first time. if the user wants more scroll is available ...

although at the end of the day I think svelte ( or even javascript ) community wouldn't accept it or consider it helpful ☹️

so what should I do now ?
should I remove the examples from the docs and just leave a brief overview of functions ?
or what ?

benmccann · 2022-02-14T16:16:49Z

src/runtime/internal/Component.ts

+	 * off();
+	 * ```
+	 * 
+	 * @param type 


I think if we're going to add @param tags then they should have descriptions of those parameters

benmccann · 2022-02-14T16:21:38Z

src/runtime/motion/spring.ts

-export function spring<T=any>(value?: T, opts: SpringOpts = {}): Spring<T> {
-	const store = writable(value);
+/**
+ * A `spring` store gradually changes to its target value based on its `stiffness` and `damping` parameters. Whereas `tweened` stores change their values over a fixed duration, `spring` stores change over a duration that is determined by their existing velocity, allowing for more natural-seeming motion in many situations. The following options are available:


This is copied from https://svelte.dev/docs#run-time-svelte-motion-spring

benmccann

Thanks for making the effort to improve the docs

I think that copying info from the docs into the JSDocs is not the right approach. Almost certainly we will end up with cases where one version of the docs is updated and not the other leading to confusion and difficulty maintaining the docs. We're planning to investigate a tighter integration between the code and docs. We're going to start with that in the SvelteKit docs by trying out https://shikijs.github.io/twoslash/. I understand the convenience of seeing more in the editor and not having to pull up the docs in a separate browser window. However, I think we need to figure out the tooling piece of this so that we can have just a single place to maintain the docs

I'm not sure it's worth continuing with this PR. The better approach might be to close it and instead work on improving the tooling of the SvelteKit docs and then leveraging those improvements in the Svelte core docs once we've decided on a good way of doing things

ehsan2003 · 2022-02-15T04:18:00Z

I think that copying info from the docs into the JSDocs is not the right approach. Almost certainly we will end up with cases where one version of the docs is updated and not the other leading to confusion and difficulty maintaining the docs. We're planning to investigate a tighter integration between the code and docs. We're going to start with that in the SvelteKit docs by trying out https://shikijs.github.io/twoslash/. I understand the convenience of seeing more in the editor and not having to pull up the docs in a separate browser window. However, I think we need to figure out the tooling piece of this so that we can have just a single place to maintain the docs

Yes of course it is not a good idea to just copy/paste the docs.

and this was the reason why I've added all code examples and even the function signature at first
I was just planning to use (Type doc)[https://typedoc.org/] api to extract necessary docs and put it into docs with a script ( It could be done through a template engine for example.

benmccann · 2022-02-15T18:47:12Z

It's an interesting idea. I think we need to agree on a plan and what tooling we want to use before we start changing stuff. I'd suggest the #contributing channel in Discord or an issue in SvelteKit might be a good place to start. We want to experiment with SvelteKit before making any changes here, so I'm going to go ahead and close this. Thanks for your efforts on this

[docs] adds jsdoc for most functions in runtime API

d0e716a

dummdidumm requested changes Feb 9, 2022

View reviewed changes

ehsan2003 and others added 3 commits February 9, 2022 12:43

Apply suggestions from code review

da912cd

Co-authored-by: Simon H <[email protected]>

[docs] separates fallback parameter from the rest in crossfade

5864570

[docs] fixes formatting in Component.ts

ebf4054

benmccann reviewed Feb 11, 2022

View reviewed changes

src/runtime/internal/Component.ts Outdated Show resolved Hide resolved

benmccann reviewed Feb 11, 2022

View reviewed changes

src/runtime/internal/Component.ts Outdated Show resolved Hide resolved

ehsan2003 and others added 2 commits February 12, 2022 08:38

Update src/runtime/internal/Component.ts

fd18fb3

Co-authored-by: Ben McCann <[email protected]>

Update src/runtime/internal/Component.ts

843c3d4

Co-authored-by: Ben McCann <[email protected]>

[docs] removes the signiture from docs for functions

c9c34b9

benmccann reviewed Feb 14, 2022

View reviewed changes

benmccann requested changes Feb 14, 2022

View reviewed changes

benmccann added the documentation label Feb 14, 2022

benmccann closed this Feb 15, 2022

Uh oh!

[docs] adds jsdoc for most functions in runtime API #7235

[docs] adds jsdoc for most functions in runtime API #7235

Uh oh!

Conversation

ehsan2003 commented Feb 9, 2022 • edited by benmccann Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before submitting the PR, please make sure you do the following

Tests

what happened ?

Uh oh!

dummdidumm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ehsan2003 commented Feb 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benmccann commented Feb 11, 2022

Uh oh!

Uh oh!

Uh oh!

benmccann commented Feb 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ehsan2003 commented Feb 12, 2022

Uh oh!

ehsan2003 commented Feb 12, 2022

Uh oh!

benmccann commented Feb 13, 2022

Uh oh!

bluwy commented Feb 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ehsan2003 commented Feb 13, 2022

Uh oh!

bluwy commented Feb 13, 2022 • edited by benmccann Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ehsan2003 commented Feb 13, 2022

Uh oh!

ehsan2003 commented Feb 13, 2022

Uh oh!

bluwy commented Feb 14, 2022

Uh oh!

ehsan2003 commented Feb 14, 2022

Uh oh!

benmccann Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

benmccann Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

benmccann left a comment

Choose a reason for hiding this comment

Uh oh!

ehsan2003 commented Feb 15, 2022

Uh oh!

benmccann commented Feb 15, 2022

Uh oh!

Uh oh!

ehsan2003 commented Feb 9, 2022 •

edited by benmccann

Loading

ehsan2003 commented Feb 9, 2022 •

edited

Loading

benmccann commented Feb 11, 2022 •

edited

Loading

bluwy commented Feb 13, 2022 •

edited

Loading

bluwy commented Feb 13, 2022 •

edited by benmccann

Loading