genType support

[mununki]

I'm using @genType in one of my projects. Rewatch doesn't seem to be able to generate *.gen.tsx yet, is there any plan to support it?

[jfrolich]

Any idea what needs to happen to support genType? Is it an extra PPX that implicitly needs to be called or something?

[Bushuo]

I poked around a bit and it seems that -bs-gentype needs to be passed to the compiler during compile step.
From my observations here the gentype implementation currently does not support absolute paths.

during compilation rescript invokes the compiler like this:

/rewatch/testrepo/node_modules/rescript/darwinarm64/bsc.exe -I src -bs-gentype -uncurried -bs-package-name testrepo -bs-package-output es6:src:.mjs src/Test.ast

whereas rewatch does it like this (I added the -bs-gentype flag)

"/rewatch/testrepo/node_modules/rescript/darwinarm64/bsc.exe" "-I" "." "-bs-gentype" "-bs-jsx" "3" "-uncurried" "-bs-package-name" "testrepo" "-bs-package-output" "es6:src:.mjs" "/rewatch/testrepo/lib/ocaml/Test.ast"

[adnelson]

Rewatch is awesome! But I definitely need gentype support in order to be able to use it at my company. I tried making this change and building my project, and it didn't produce gentype files:

diff --git a/src/build/compile.rs b/src/build/compile.rs
index ea52e6a..dd35931 100644
--- a/src/build/compile.rs
+++ b/src/build/compile.rs
@@ -477,6 +477,7 @@ pub fn compiler_args(
         debug!("Compiling file: {}", &module_name);
 
         vec![
+            "-bs-gentype".to_string(),
             "-bs-package-name".to_string(),
             config.name.to_owned(),
             "-bs-package-output".to_string(),

Of course we wouldn't want to always add the flag like that, it'd be based on the bsconfig.json, but regardless it still doesn't work, no .gen.ts files are produced (which is what rescript build produces).

Also I'm not sure how extra configs related to gentype in the bsconfig are supposed to get passed in, such as shims, output file extension etc.

If anyone with more knowledge of the compiler api knows what needs to happen in order to produce gentype files, it'd enable us to use rewatch 🔥 and I'd be more than happy to make a PR for it if someone points me in the right direction

[zth]

-bs-gentype should be all that's needed (cc @cristianoc ). Could it be that the files are produced and then cleaned up directly...?

[cristianoc]

-bs-gentype should be all that's needed (cc @cristianoc ). Could it be that the files are produced and then cleaned up directly...?

Probably.
Also, ideally -bs-gentype will become unnecessary in future, right @cometkim ? But for now, I might be needed.

[zth]

Also I'm not sure how extra configs related to gentype in the bsconfig are supposed to get passed in, such as shims, output file extension etc.

I think gentype reads the config file by itself.

@adnelson so the fix you did there should work. Not clear why it doesn't. And adding proper support to rewatch should be a matter of checking whether the config file has a prop gentypeconfig, and if it does, make sure bsc gets the -bs-gentype flag, just like you hard coded.

[cometkim]

Also, ideally -bs-gentype will become unnecessary in future, right @cometkim ? But for now, I might be needed.

Exactly. In the near future, gentype will emit for all or no files by config.

[Bushuo]

@cristianoc @zth The files are not generated because rewatch passes the path to the .ast file as an absolute path. Currently gentype does not handle absolute paths.
I have a fork of rewatch that handles this by calling bsc twice for each file. I would be glad to submit a PR.

[zth]

@cristianoc @zth The files are not generated because rewatch passes the path to the .ast file as an absolute path. Currently gentype does not handle absolute paths. I have a fork of rewatch that handles this by calling bsc twice for each file. I would be glad to submit a PR.

Sounds like something that should be fixed in the compiler so that gentype does handle absolute paths..?

[cristianoc]

To confirm, assuming this is true:
1 bsb passes relative paths
2 rewatch passes absolute paths
3 the compiler is generally robust w.r.t. absolute paths
4 gentype isn't robust wrt absolute paths

Then yes, gentype should behave like the rest of the compiler.

[adnelson]

@Bushuo a PR that fixes it would be amazing 💯 of course, having to call bsc twice for each file is definitely not ideal, but could be omitted once the gentype compiler is fixed to work with absolute paths (which is strange that it doesn't).

I wonder if alternatively rewatch could be configured to pass in relative paths instead of absolute paths, again until absolute paths work. I don't know what the advantages/disadvantages are for that.

[Bushuo]

@adnelson I tried using my implementation in a larger project that was already set up with Rewatch, but unfortunately, it didn’t work. Apologies for that.

It seems like this needs to be handled at the compiler level. The issue with the paths stems from how Rewatch organizes its files in a flat structure under lib/ocaml, whereas bsb structures the compilation artifacts under lib/bs to mirror the source folder's hierarchy. At least, that’s what I’ve gathered from my investigation so far.

[adnelson]

@Bushuo no worries. Presumably it should be possible to make gentype work with absolute paths, but unfortunately I've spent the last few hours trying to get the compiler built on my macbook to no avail :\ so I can't really hack on the compiler side of things.

[zth]

Around here might be a good place to start looking: https://github.com/rescript-lang/rescript-compiler/blob/master/compiler/gentype/Paths.ml#L41-L45

[Bushuo]

@adnelson @zth
I think I got something working on my fork and tested it in my project.
I have based it on v11.1.4. But it seems the file is formatted differently now.
I would rebase it on master then and file a PR.

[zth]

Ah right. This is definitely something we could backport to 11.1.x and release so you don't need to go to v12 for it.

[adnelson]

I'm still on v10 :\ there's more that I'll need to do in order to get v11 working with our app. But hopefully whatever you @Bushuo got working with v11, will also work with v10 (or close).