Documentation and docstrings

[zth]

A goal for Core is to provide a really solid developer experience. One important piece in doing that is providing good documentation, and good docstrings. For those who don't know, docstrings are the strings you prepend functions etc with to explain what they are. They show up via the editor tooling on hovering, auto completion, etc. Example from Promise.resi: https://github.com/rescript-association/rescript-core/blob/main/src/Core__Promise.resi#L11-L17

We will need help to do this. Crafting good docstrings takes time, and there's a fairly large API surface to cover.

Eventually, these docstrings will also be extracted and used for real documentation, via the doc extraction project that's in the works.

Current state

• Array
• AsyncIterator
• Console
• Date
• Dict
• Error
• Float
• Global
• Int
• Iterator
• JSON
• List
• Map
• Math
• Null
• Nullable
• Option
• Promise
• RegExp
• Result
• Set
• String
• Type
• Undefined
• Symbol
• BigInt
• Intl Collator
• Intl DateTimeFormat
• Intl Locale
• Intl NumberFormat
• Intl PluralRules
• Intl RelativeTimeFormat
• Object
• ArrayBuffer
• BigInt64Array
• BigUint64Array
• Float32Array
• Float64Array
• Int8Array
• Int16Array
• Int32Array
• TypedArray
• Uint8Array
• Uint8ClampedArray
• Uint16Array
• Uint32Array
• WeakMap
• WeakSet
• DataView

Suggested format

1. Describe the thing in a sentence or two. Keep this short. Good to be inspired by MDN for example.
2. Write a ReScript example in code, inside ```rescript notation. This is important and I think we should aim to have this for everything it makes sense for.

This example (and the rest in that file) is a good guide: https://github.com/rescript-association/rescript-core/blob/main/src/Core__Promise.resi#L11-L17

Workflow

I propose the following workflow:

1. Look up a module in the project that you want to start working on.
2. Generate a .resi file for the module, if it doesn't have one already. We aim for all modules having resi files, and to keep the documentation in there.
3. Write in this thread that you'll start working on said module.
4. Start working, and open a WIP PR. Commit and push often. This is so that others can pick up where you left off if needed.

I can recommend having the compiler running as you work on docstrings, and then use TempTest.res to get immediate feedback on your changes. Just use whatever you're working on and hover it in there and you'll see how the docstrings render immediately.

Editor integration

We want to make using Core an as pleasant experience as possible. Part of that is also that the editor tooling behaves the way we want it to with regards to docstrings etc. #3 this issue covers that, feel free to post things you discover in there.

Open questions

First off, please give feedback on the format, so we can ensure we cover everything we want to cover. Here are some additional open questions:

• Should we suggest alternatives in the docstrings? This is particularly important for things like mutable APIs, like Map which binds to the native JS Map, where Belt.Map is an immutable alternative.
• On the same theme, should we highlight clearly what things are mutable? More discussion in this thread: [Discussion] Immutable defaults for mutable JS API:s #23
• Should we add links to the website docs where applicable? Concepts involved in using the thing, etc.

[woeps]

Regarding the open questions:
I'd say it depends on the plans/ideas for an online api reference.

If the docstrings are the one and only truth and everything else is generates from them, I'd say yes to all of the three questions above.

If there is a plan for displaying docstrings directly on hover and offer additional resources & eplanations in some kind of "extended online api reference", my answers would change to the following:

1. Don't suggest alternatives. Assume the dev explicitly chose the function and only show information to support the task at hand: Use the function and it's return with valid arguments. Offer some reference / link to more information.
2. Text on hover should clearly state any side-effects: e.g. mutation - because this may be important to be remembered of, at the moment of using the function
3. The referenced / linked content (see 1.) should support devs, when researching the right function for the usecase: Provide more elaborate explanations, links to native js docs, links to alternatives. So they are able to browse through the api, learn about differences between similar functions and choose the right function for the task.

[zth]

Great feedback! I agree. I think mid to long term we definitively want to provide an "extended online api reference". It's however likely that it'll be a while before that's in place, given that it takes time to build and we're a limited set of contributors with limited time at hand right now.

@woeps would you be interested in helping out shaping the initial set of docstrings? They'll set the standard going forward.

[woeps]

I'm definetly willing to. I just need to see how much time I'll be able to spend. 😉

I'd like to propose to separate more extensive information with a line containing only --- ("hr" element in markdown) after the most necessary information.

This would enable us to have all information at one place until more tooling / infrastructure is being worked on. This might also be simple to parse in the vs code extension and potentially truncate the extended info if necessary?

[zth]

Don't we all 😉

That sounds good, all for that! That'll be easy to handle and change as needed. Doc extraction tooling is coming fairly soon, so we're not ways off at least.

[vasco3]

inspiration:

[aspeddro]

Should the content of the comments be indented?

https://github.com/rescript-association/rescript-core/blob/7947146895e112ad8f85427afdb7d702d04ce7ec/src/Core__Array.resi#L71-L80

Or aligned with /:

/**
`reduceReverse(xs, init, f)`

Works like `Array.reduce`; except that function `f` is applied to each item of `xs` from the last back to the first.

```rescript
Array.reduceReverse(["a", "b", "c", "d"], "", (a, b) => a ++ b) == "dcba"
```
*/
let reduceReverse: (array<'b>, 'a, ('a, 'b) => 'a) => 'a

Indenting can affect how the content is presented in the editor.

[aspeddro]

I'd like to propose to separate more extensive information with a line containing only --- ("hr" element in markdown) after the most necessary information.

I think we should keep it as simple as possible. These comments will be consumed by other tools that can apply this transformation.

For editor tools, comments are already rendered with good separation. In Neovim for example, it adds the --- separator before the code blocks, see rescript-lang/rescript-vscode#619.

Maybe add a # Examples header before code block?

[woeps]

I think we should keep it as simple as possible. These comments will be consumed by other tools that can apply this transformation.

@aspeddro Are you saying, you're against it?

I was thinking about more extensive information, and how to seperate it. I wanted to show a real world example on the first docstring I'll work on, but for the sake of the discussion here is a fictive example:

/**
Do something very important, based on data t.

`t` is the data being operated on and `arg1` is a random string being used `arg2`-times in this operation.

**This mutates the original `t`.**

\```rescript
let x = makeT("hellp")
let result = fnName(x, ~arg1="!", ~arg2=42)
\```

---

## Explanation

This is a more thorough explanation suited for devs,
who are not familiar with this type of function.
Probably involving further meaningful examples and
step by step walkthrough if necessary. 

## References

- [MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Functions) provides further usage information reagrding the bound js function
- there are some specificas to [usage in node](https://nodejs.org/api/n-api.html#napi_threadsafe_function)

## Alternatives

- `fnName` provides an immutable version of this function
- `OtherModule.otherFn` is similar, but provides more fine-grained control
- `AnotherModule.fnName` provides the same operation but on the data type `foo`

*/
external fnNameInPlace: (t, ~arg1: string, ~arg2: int) => t

Note: I needed to "escape" the example code block in the doc string to avoid weird rendering of this comment.

I was proposing, that the language server could ignore everything after (including) the line containing ---. This way we start collecting all the information now and have it available in the interface files for everyone to see. Future tooling could make use of the additional information provided.
This would also mitigate your concerns regarding the rendering in some code editors. Right?

If we find consesus on the usage of this separator I'll file an issue on the language server repo.

[zth]

I think that's a good format @woeps . I say we go for that. Editor tooling wise this will be simple to change/fix later on, so no need to worry about that now, I'll fix that when needed. As long as there's a coherent format.

Some thoughts from me:

• Keeping it fairly terse is nice. I like this last example from @woeps.
• We can probably aim to provide one very simple example of usage, and then extend with more advanced usage examples in the "add on" section, if needed.

[zth]

Should the content of the comments be indented?

https://github.com/rescript-association/rescript-core/blob/7947146895e112ad8f85427afdb7d702d04ce7ec/src/Core__Array.resi#L71-L80

Or aligned with /:

/**
`reduceReverse(xs, init, f)`

Works like `Array.reduce`; except that function `f` is applied to each item of `xs` from the last back to the first.

```rescript
Array.reduceReverse(["a", "b", "c", "d"], "", (a, b) => a ++ b) == "dcba"

*/
let reduceReverse: (array<'b>, 'a, ('a, 'b) => 'a) => 'a

Indenting can affect how the content is presented in the editor.

I say we don't indent, keep it aligned to the leading / in /**.

[aspeddro]

I think we should keep it as simple as possible. These comments will be consumed by other tools that can apply this transformation.

@aspeddro Are you saying, you're against it?

I was thinking about more extensive information, and how to seperate it. I wanted to show a real world example on the first docstring I'll work on, but for the sake of the discussion here is a fictive example:

/**
Do something very important, based on data t.

`t` is the data being operated on and `arg1` is a random string being used `arg2`-times in this operation.

**This mutates the original `t`.**

\```rescript
let x = makeT("hellp")
let result = fnName(x, ~arg1="!", ~arg2=42)
\```

---

## Explanation

This is a more thorough explanation suited for devs,
who are not familiar with this type of function.
Probably involving further meaningful examples and
step by step walkthrough if necessary. 

## References

- [MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Functions) provides further usage information reagrding the bound js function
- there are some specificas to [usage in node](https://nodejs.org/api/n-api.html#napi_threadsafe_function)

## Alternatives

- `fnName` provides an immutable version of this function
- `OtherModule.otherFn` is similar, but provides more fine-grained control
- `AnotherModule.fnName` provides the same operation but on the data type `foo`

*/
external fnNameInPlace: (t, ~arg1: string, ~arg2: int) => t

Note: I needed to "escape" the example code block in the doc string to avoid weird rendering of this comment.

I was proposing, that the language server could ignore everything after (including) the line containing ---. This way we start collecting all the information now and have it available in the interface files for everyone to see. Future tooling could make use of the additional information provided. This would also mitigate your concerns regarding the rendering in some code editors. Right?

If we find consesus on the usage of this separator I'll file an issue on the language server repo.

I had understood to add --- before the block of code (after most necessary information). It's a good format.

[aspeddro]

I would like to propose adding a ## Examples section after the most needed information, to improve the division.

The current state:

[zth]

I think that sounds good.

[aspeddro]

A proposal for testing code blocks in the future.

Most docstring have checks like:

String.startsWithFrom("BuckleScript", "kle", 3) == true

However, this code is not valid. Error: Toplevel expression is expected to have unit type.

We could use assert expressions to test blocks of code.

assert(String.startsWithFrom("BuckleScript", "kle", 3))

Is it a good format or should we keep the current one?