Handle absolute paths in typegen

[Bushuo]

Like discussed in rescript-lang/rewatch#102 this PR enables gentype to handle passing of absolute file paths. Instead of manipulating the path to the .cmt file it gets the source file path from the .cmt file first. Then constructs the output file paths from there.

[cristianoc]

Would you do make format and then run tests locally:

make test
make test-gentype

The last command should produce no difference in the output.

[Bushuo]

@cristianoc Yes, will do. Tests are failing locally. I will keep working on it.

[Bushuo]

@cristianoc I added the resolved project_root and bsb_project_root to the default config. They where empty strings before.

[mununki]

This may not be related to this PR, but the genType output for v12 still differs from v11 in this way.

// Title.res
@genType @react.component
let make = (~text) => {
  <div className="title"> {text->React.string} </div>
}

v11

/* TypeScript file generated from Title.res by genType. */

/* eslint-disable */
/* tslint:disable */

import * as TitleJS from './Title.mjs';

export type props<text> = { readonly text: text };

export const make: React.ComponentType<{ readonly text: string }> = TitleJS.make as any;

v12

/* TypeScript file generated from Title.res by genType. */

/* eslint-disable */
/* tslint:disable */

import * as TitleJS from './Title.mjs';

import type {element as Jsx_element} from './Jsx.gen.tsx'; // Error: module not found

export type props<text> = { readonly text: text };

export const make: (_1:props<string>) => Jsx_element = TitleJS.make as any;

[cristianoc]

This may not be related to this PR, but the genType output for v12 still differs from v11 in this way.

// Title.res
@genType @react.component
let make = (~text) => {
  <div className="title"> {text->React.string} </div>
}

v11

/* TypeScript file generated from Title.res by genType. */

/* eslint-disable */
/* tslint:disable */

import * as TitleJS from './Title.mjs';

export type props<text> = { readonly text: text };

export const make: React.ComponentType<{ readonly text: string }> = TitleJS.make as any;

v12

/* TypeScript file generated from Title.res by genType. */

/* eslint-disable */
/* tslint:disable */

import * as TitleJS from './Title.mjs';

import type {element as Jsx_element} from './Jsx.gen.tsx'; // Error: module not found

export type props<text> = { readonly text: text };

export const make: (_1:props<string>) => Jsx_element = TitleJS.make as any;

would you open an issue for this looks like in v12 we expose Jsx types differently, and need to be handled specially by gentype as built in types. Otherwise, as the code generated show, it's just a mysterious unknown type.

[mununki]

https://github.com/rescript-lang/rescript-compiler/issues/7106

[zth]

CI seems to fail still.

[cristianoc]

CI seems to fail still.

Yes looks like there's a little annotation missing after refactoring to please the static exception checker.

[Bushuo]

Nice 🥳 I could swear that the static checker previously ran without having to explicitly make reanalyze.

[Bushuo]

Also, has it been tested on some rewatch project?

I only tested the initial version of this PR while I had it based on the v11 branch. Currently my project fails to build with v12.

[cristianoc]

@mununki @zth who has a rewatch project at hand to try this?

[zth]

@Bushuo this will need backporting to v11 anyway so if you want you could do a PR on top of this that just backports this to v11 and try it that way.

[Bushuo]

@Bushuo this will need backporting to v11 anyway so if you want you could do a PR on top of this that just backports this to v11 and try it that way.

@zth Sure, I can do it. I haven't backported before, though, so if you could give me a quick rundown of the ideal process, that'd be great. I might not get to it today though.

[zth]

@Bushuo this will need backporting to v11 anyway so if you want you could do a PR on top of this that just backports this to v11 and try it that way.

@zth Sure, I can do it. I haven't backported before, though, so if you could give me a quick rundown of the ideal process, that'd be great. I might not get to it today though.

Sure! It's really simple, and basically what you did before - just base a separate PR of this work off of 11.0_release instead of master. I think there are some formatting differences to watch out for, but it should be fairly straight forward.

So, after that, you'll have 2 PRs:

1. This, based on master, that'll go into v12
2. A separate PR based on 11.0_release, that'll go into the next v11 patch release

Clear?

[Bushuo]

@cristianoc @zth I assumed FindSourceFile.ml cmt cmt_annots would always return an absolute path, allowing it to be used for both output_file and output_file_relative. However, this only seems true for v12, not v11.

On the v11 branch, the function returns test/variantsMatching.res for the file jscomp/test/variantsMatching.res. I suspect this is related to how sources is configured in bsconfig.json.

The config file is at the root level.

"sources": {
    "dir": "jscomp/test/"
}

In contrast, configuring a similar test in v12 still returns an absolute path. Do you know why this behavior differs and could you provide some guidance on how to proceed?

[cristianoc]

@zth why do we need v11 support?

[zth]

@zth why do we need v11 support?

Because it's likely a large portion of users will remain on v11 for a good while. Backporting this will mean that those can move to rewatch if they're using gentype as well before it ships in the compiler itself only.

[Bushuo]

I made some changes and now it also seems to work on v11. Could you please take a look again?

[zth]

@Bushuo great work! ⭐