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

ReScriptify user-facing Js / Belt modules and API doc generation #5459

Open
ryyppy opened this issue Jun 22, 2022 · 13 comments
Open

ReScriptify user-facing Js / Belt modules and API doc generation #5459

ryyppy opened this issue Jun 22, 2022 · 13 comments
Assignees

Comments

@ryyppy
Copy link
Member

ryyppy commented Jun 22, 2022

As a follow up to #5361, we want to convert all ml[i] files to `res[i].

Master already comes with the new /** */ sugar, so we won't make any trade-offs in readability.

There are some files that can't be converted right away (see #5458), but we'll just tackle this in a separate task.

@ryyppy ryyppy self-assigned this Jun 22, 2022
@Minnozz
Copy link
Contributor

Minnozz commented Jul 26, 2022

This is mentioned in the changelog for 10.0.0-beta.3; has this already been completed?

@ryyppy
Copy link
Member Author

ryyppy commented Jul 27, 2022

No, the doc headers have been synced up, but there's still no rescriptified files yet. I got this as a pending local change, but still need to finalize it.

@ryyppy ryyppy changed the title ReScriptify user-facing Js / Belt modules ReScriptify user-facing Js / Belt modules and API doc generation Sep 15, 2022
@ryyppy
Copy link
Member Author

ryyppy commented Sep 15, 2022

Alright, due to time constraints I am not able to follow up on this in a timely manner, so I talked to @cristianoc to write down what's needed to make this work.

Step 1 (ReScriptify JS/Belt with our new doc header syntax):

First we need to actually convert the relevant belt / js files.
I made a temporary script that is actually capable of converting all those files (except the ones that are meta-generated with cppo).

You can find the script here:
https://gist.github.com/ryyppy/a0094cbda52ff57b86fdb0bdcba1edb6

Generally the script actually works. One needs to make sure to build the latest rescript binary and globally install rescript, so the script picks up the proper version.

Main work here is to run the script (maybe start with a single file and comment out everything else), and see if the conversion looks good (the code should look fine, and especially the new ReScript doc headers should be correctly placed with /** */ and /*** */).

Step 2 - The Doc Extraction Script:

I've got a working branch on the syntax repo called experimental-docgen, that contains a simple OCaml program to extract ocaml.doc headers from an mli / resi file. This needs to be adapted to extract the new doc header representation (ask @cristianoc for details).

The script should generate a pretty complete JSON file that contains all the doc header / function and value signatures needed to generate proper markdown files for our website.

Step 3 - Generate the API docs for rescript-lang.org

Now that we are able to generate JSON files for each relevant JS / Belt interface file, we should now have a look at the rescript-lang api docs structure (as listed here https://github.com/rescript-association/rescript-lang.org/tree/master/pages/docs/manual/latest/api) and try to replicate the markdown output with our new JSON input.

Depending on how well this works, we may not even need to generate sidebar tocs, since we currently generate the TOCs from our markdown structures.

Step 4 - Automate

Everything put together, we should create a script that will automatically generate our api markdown files by pointing it to a specific rescript installation. That way it should be a matter of pressing a button to release new API docs.

@aspeddro
Copy link
Contributor

aspeddro commented Sep 17, 2022

I did the conversion of belt_Array.mli, is adding escape \ before string inside markdown code blocks

external length: 'a t -> int = "%array_length"
(** return the size of the array

```res example
// Returns 1
Belt.Array.length(["test"])
```
*)
/** return the size of the array

```res example
// Returns 1
Belt.Array.length([\"test\"])
```
*/
external length: t<'a> => int = "%array_length"

@cristianoc
Copy link
Collaborator

To test this. First make sure the libs are up to date in the local compiler build:

node scripts/install -force-lib-rebuild

Then install the locally built compiler into a project:

cd project
npm install ~/GitHub/rescript-compiler

Then open up the editor and see how the hover looks like.

@cristianoc
Copy link
Collaborator

cristianoc commented Sep 17, 2022

For generated files, will probably need to change the generator so it works with .res files.
E.g. this is limited to .ml and .mli:
https://github.com/rescript-lang/rescript-compiler/blob/master/scripts/ninja.js#L1017

@aspeddro
Copy link
Contributor

I'm using the master branch build.

./node_modules/.bin/rescript -version                                                                                                             
10.1.0-alpha.2

@cristianoc
Copy link
Collaborator

I'm using the master branch build.

./node_modules/.bin/rescript -version                                                                                                             
10.1.0-alpha.2

Need to replace belt_Array.mli with belt_Array.resi, build from source, in order to test.

@aspeddro
Copy link
Contributor

Built from source:

image

A problem I encountered, perhaps related to LSP: Going to definition leads me to wrong definition.

@cristianoc
Copy link
Collaborator

Try:
node scripts/install -force-lib-rebuild to copy library files into lib/ocaml
in case it's simply stale lib in the local build

@aspeddro
Copy link
Contributor

Try: node scripts/install -force-lib-rebuild to copy library files into lib/ocaml in case it's simply stale lib in the local build

I dit it.

@cknitt
Copy link
Member

cknitt commented Aug 18, 2023

Almost done, only js.ml and js_math.ml remaining in .ml syntax now because of some unexpected changes to release.ninja when converting those.

@cknitt
Copy link
Member

cknitt commented Aug 25, 2024

@ryyppy @cristianoc Anything still open here?

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

No branches or pull requests

5 participants