Skip to content

Can't use genType #128

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

Closed
jmagaram opened this issue Apr 2, 2023 · 36 comments
Closed

Can't use genType #128

jmagaram opened this issue Apr 2, 2023 · 36 comments
Labels
enhancement New feature or request

Comments

@jmagaram
Copy link
Contributor

jmagaram commented Apr 2, 2023

GenType generates import statements for Core modules that don't work. What do I need to do to make this work?

I see in https://rescript-lang.org/docs/gentype/latest/usage#dependent-projects--libraries it says "The library must have been published with the .gen.ts files created by genType."

I've got a file ReactFirebaseHooks.res with this in it AND my bsconfig.json has "bsc-flags": ["-open RescriptCore"]

@gentype.import("react-firebase-hooks/auth") @genType.as("useAuthState")
external useAuthState: Auth.t => (Null.t<Auth.user>, bool, option<Error.t>) =
  "useAuthState"

This leads to the following in my ReactFirebaseHooks.gen.tsx...

import type {t as Error_t} from './Error.gen'; // does not exist
import type {t as Null_t} from './Null.gen'; // does not exist

If I change the code to reference Core__Error.gen this is probably closer to the desired result, but still doesn't work.

import type {t as Core__Error_t} from '@rescript/core/src/Core__Error.gen'; // does not exist
import type {t as Core__Null_t} from '@rescript/core/src/Core__Null.gen'; // does not exist

I tried taking out the -open RescriptCore but it still doesn't work. I tried adding node_modules/~rescript/core/src to bsconfig.json but this shouldn't be necessary and still doesn't work.

@zth
Copy link
Collaborator

zth commented Apr 2, 2023

Hmm, IIRC this was recently fixed. What ReScript compiler version are you on?

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 2, 2023

In my package.json I see ^10.1.3. My script is rescript build -w.

@zth
Copy link
Collaborator

zth commented Apr 2, 2023

Could you try 10.1.4 explicitly?

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 2, 2023

Same problem

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 2, 2023

Did a clean, verified with npm list. The .gen.tsx file...

import type {t as Core__Error_t} from '@rescript/core/src/Core__Error.gen';
import type {t as Core__Null_t} from '@rescript/core/src/Core__Null.gen';

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 2, 2023

I have the same problem with a different package. Does the author of the package need to put @gentype everywhere or is the compiler supposed to handle this?

@cristianoc
Copy link
Contributor

Not sure what the issue is here.
#129

@cristianoc cristianoc added the invalid This doesn't seem right label Apr 3, 2023
@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 3, 2023

I'll try again with a repo scenario but your example doesn't look like what I'm talking about - just reading on my phone. Array isn't a problem because it is built in. But Error.t and Null.t are not so typescript doesn't know where to find them. . Could you clarify if that gentyoe doc comment is still true that the library author must generate and publish the .gen files with their package?

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 3, 2023

Just to be clear I'm trying to use Core in another project when it lives in the node_modules folder.

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 3, 2023

Repo scenarios here https://github.com/jmagaram/core-gentype
Am I supposed to make shims for everything in Core?

@cristianoc
Copy link
Contributor

There was an issue to explore that scenario: picking up annotations from dependencies. But there was no concrete work on that.
So it's about first finding out what's the current situation and take it from there.

@cristianoc cristianoc added enhancement New feature or request and removed invalid This doesn't seem right labels Apr 4, 2023
@cristianoc
Copy link
Contributor

@jmagaram I should look for the analogous discussion in the genType repo, where exactly these kinds of issues were discussed.
I think this might be a good time to look a bit more in depth into that. Because we might have the opportunity to, for example, make it work well for at least Core when used as a dependency. And for other projects after that.
Not sure if you're time/interest to help, but starting from figuring out current status and how one would expect this to work would be a first step.
Specifically, if a dependency has genType annotations then 2 things will happen:

  1. .gen.tsx files should be checked in or anyhow produces so they are available.
  2. one might need to access relevant type information from those files in the dependency. To see if this works one can try this: use some gentype'd value or type from another file in the same project. Then do it for another file in a dependency. Is the behaviour the same
  3. Further ahead, there's the opportunity now after the latest changes for genType to conform to the way that TypeScript expects the .d.ts to be in a project. Particularly a dependency. So one can see if that is possible to do in future. So one would be able ideally to consume a dependency transparently from TS and from ReScript

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 4, 2023

I don't know how helpful I can be with this. I'll read whatever you send my way.

I tried getting it to work with Core by removing Core from bs-dependencies and adding it to sources, as if I wrote it all myself. Then I added @gentype annotations to the Error.t and Null.t types in the resi files. This works when I access the Core modules from my project directly like Core__Error. The .gen.tsx file now references import type {t as Core__Error_t} from '../node_modules/@rescript/core/src/Core__Error.gen'; which exists.

But whenever I try to use them via RescriptCore... it doesn't work. The gen.tsx file is looking for the types in import type {Error_t as RescriptCore_Error_t} from '../node_modules/@rescript/core/src/RescriptCore.gen'; which does not exist. I think this is related to that thing I experienced before where those @gentype annotations seem to get lost whenever there is any kind of functor or module renaming. So then I tried "recreating" the modules from scratch inside the RescriptCore.res file by changing this...

module Error = Core__Error

to this. I think the _trigger line is essential because otherwise I don't think gentype is doing anything. And this seems to work.

module Error: {
  include module type of Core__Error
} = {
  include Core__Error
}

@genType
let _trigger = true

There is a problem with import type {Exn_t as Js_Exn_t} from './Js.gen'; - can't find this. So maybe a shim is needed?

@cristianoc
Copy link
Contributor

So to summarize: the path generated is correct but there is no file at that path?
If that is the case it means that core needs to ship with the .gen.tsx files.

Assuming this is all true (please confirm in case) it would be useful to just add the file by hand and check that everything works.

If everything works, then it's just a matter to make sure that the file is there.

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 4, 2023

All scenarios require @gentype annotations on the types in the resi files.
All scenarios require adding genType configuration to bsconfig.json.
Sometimes you need a @gentype let _trigger=true to get things going.

Now I have it set up with @rescript/core in bs-dependencies. The behavior is different - see below. I don't understand what is happening. Somebody else should look at this. I did not set any auto-open flags.

If you access the code with full path, like Core__Null, it works. The TypeScript tries to find files like '@rescript/core/src/Core__Error.gen';. We could remove the wrapper module RescirptCore and instead rename all the modules like Core__Array => Array. Then put stuff we want to be global like null, window, and other things in a CorePervasives module and have that auto-open. I think this would all work.

But if we want to access things through the root RescriptCore module...I updated RescriptCore.res like this because I think this might be necessary to make the proper gentype files...

module Error: {
  include module type of Core__Error
} = {
  include Core__Error
}

But if you access the library through the RescriptCore wrapper it tries to find things in './Error.gen' and './Null.gen - so the path is totally wrong.

@genType
let makeError1: unit => RescriptCore.Error.t = () => RescriptCore.Error.make("wow")

@genType
let makeError2: unit => Core__Error.t = () => Core__Error.make("wow")

This is some of the javascript.

// tslint:disable-next-line:no-var-requires
const OpenCoreBS = require('./OpenCore.bs');
import type {t as Core__Error_t} from '@rescript/core/src/Core__Error.gen';
import type {t as Error_t} from './Error.gen';
import type {t as Null_t} from './Null.gen';
export const makeError1: () => Error_t = OpenCoreBS.makeError1;
export const makeError2: () => Core__Error_t = OpenCoreBS.makeError2;

I'm running into this same problem on a package I built for my personal use. Maybe there should be some way to do something like @alias module Error = Core__Error that compiles each reference to RescriptCore.Error to Core__Error?

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

I don't understand the underlying problem. Something about the compiler not realizing that RescriptCore.Error.t is the same thing as Core__Error.t.

@cristianoc
Copy link
Contributor

cristianoc commented Apr 5, 2023

I think a couple of different aspects have been identified, and can be explored further separately.

  1. Sometimes the path is correct, but there's no file in there as Core is not configured to run genType

  2. Sometimes the path is nor correct, because of module aliases.

It's important to separate the two aspects and dig deeper to get clarity.

For 1, there's something new now: in v11, genType will not generate conversion functions (a.k.a. additional code) in the .gen.tsx files. That means, it becomes conceivable to add a global config flag equivalent to adding the @genType annotation to every single value and type declaration in every .res and .resi file. In addition, such a configuration could e set up so that it applies to dependent projects automatically. This would mean that: even if Core is not created with genType turned on, projects that use core can turn on this global config, and have it apply to all the dependencies automatically.

For 2, we need to cut down the problem. It should be possible to have a tiny example with 1 or 2 files with a couple of lines each to illustrate the issue. It might be that those files need to be in a dependency, or perhaps one can repro with just using a single project. One needs to try. So it's about: start with an example of broken behaviour and cut it down more and more until it becomes completely clear what the underlying issue is.

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

I created a simpler repro example inside the project in https://github.com/jmagaram/core-gentype
Special__ArrayIndex.res which is supposed to be like one of the Core__ modules.
Special.res is the wrapper module like RescriptCore
SpecialUsage.res is trying to use it with genType.

(1) If we use genType on the types in the Special__ modules - the full awkward module name - then that seems to work. I saw some post saying you might want to update the VS Code extension to HIDE these modules so the developer only seems the root module which would complicate things.

(2) If the developer attempts to put Special.ArrayIndex.t in the genType bindings - accessing the type through the wrapper module - it won't work. The code tries to import a type that isn't exported from Special.gen.

(3) If we duplicate the type inside the wrapper, like Special.arrayIndex just as Core did with some types like null, undefined, and put a @genType annotation on that, it works. Still won't work in case 2 which is kind of weird.

(4) I think there is an option to copy the entire implementation of every module into the wrapper module using that include syntax from above. I think that works but is quite ugly.

(5) Rename all the modules from Core__Error to Error, so using the full path is more natural. Don't have a wrapper module and instead have a Common or Pervasives that is auto-imported.

(6) Some new compiler feature or @alias module x = y

@cristianoc
Copy link
Contributor

cristianoc commented Apr 5, 2023

Let's cut this down just one more time.

  • Annotate all the relevant things with @genType, so we don't run into the issue of some type not being annotated (within a file, e.g. annotated function foo uses type t implies annotation propagates to t, but this does not happen across files)
  • No include tricks.
  • No attempts to fix/work around. Just observations. (Can't begin thinking of solutions before we fully understand the issue)

So I think what we're likely left with is a simple description: using the explicit module path works but using the alias does not.

Would you repeat the above to kill most of the 6 points so we're left with a simpler set of observations?

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

I think we have all the observations and think you're right - only using an explicit path to a type annotated with @genType works. So referencing Core.Error.t does not because it is an alias module Error = module Core__Error. Did you look at my code?

I experimented some more. I thought maybe taking out abstract types and interfaces would help but it didn't. I did find a hack! If you repeat the type in the wrapper module Special.res like this then you can put either Special.arrayIndex OR Special.ArrayIndex.t in bindings.

@gentype @genType.as("ArrayIndex_t")
type arrayIndex = ArrayIndex.t

@cristianoc
Copy link
Contributor

Next, we need to know if this is about dependencies. I suspect it is not.
Can you repro the same issue purely using files local to the project, and module aliases?

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

Do you mean like that situation with Core where there is another bsconfig.json?

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

Sorry I need to go to sleep. I'm not sure how to set it up. You could probably do it a lot faster than me. My repro code has the beginnings of it.

@cristianoc
Copy link
Contributor

There is no rush. The beauty of async communication.

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

I created another little package and tried to add it to bs-dependencies but it didn't work because the build system was looking inside node_modules. So I put it inside node_modules and was able to use it from my main project. Same issue. If you reference a type directly like Dependency__ShortString.t it works but if you go through the wrapping module it doesn't. When you access a type through the root wrapping module the typescript is looking for a file some dependency/src/Wrapped.gen and there isn't anything in it. Just doing module ShortString = Dependency__ShortString doesn't generate any .gen file. This little example I played with is not much different than the code I already have in my repro where I am trying to access Core.

I looked back at the code that is accessing Core. When I try to use a type through the root wrapping module the TypeScript is looking for ./Error.gen and ./Null.gen. Very weird. Why isn't it looking for RescriptCore.gen?

I don't want to spend more time on this. Please just take my repro repository and do whatever additional experiments you want on it. I'm very inefficient because I don't understand pinned dependencies and other things about the build system. I tried modifying Core inside node_modules to generate .gen files and it is very flaky. It was complaining about the annotations being ignored because there was a resi file, even though I used ignoreInterface in several places. I did a bunch of clean then rebuild etc. and finally it randomly started working. I have no idea why. Lots of wasted time. The existence of a RescriptCore.gen, Core__Error.gen etc. did NOT seem to change the fact that the generated TypeScript still looks for ./Error.gen and ./Null.gen.

@cristianoc
Copy link
Contributor

As per my question: is there a repro without using any dependencies?

@cristianoc
Copy link
Contributor

If it exists, it's going to be a simple combination of aliasing just like for the repo.

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

That repro I mentioned in this thread has trouble using gentyoe with a local file. AND there is some code that tries to use Core as a dependency. So you can see what happens in both scenarios. If you don't want the dependency just take out Core - should not have any impact on your the no-dependency case.

@cristianoc
Copy link
Contributor

Sorry if I misunderstood.
My understanding is that all your examples have a dependency (project in node modules).
My question was whether there was an example that does not use any dependencies.
The reason for the question is trying to understand if the issue has to do with dependencies.
An example of the issue without any dependencies would help clarify the scope of the issue.

It is common practice when trying to understand unexpected behaviour to cut down the possible causes. This helps coming up with solutions and debugging the issue more in detail.

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

Yes I understand the need to make examples that are as simple as they can be. I think my GitHub https://github.com/jmagaram/core-gentype is very simple. Yes it has a dependency but in 1 minute you can take it out of bsconfig.json and delete the res files that use it - UseRootModule, UseFullPath, and OpenCore. So you can see the case where (a) there is a dependency, and (b) there is no dependency. Couldn't be much simpler. Are we having a communication problem here? Just look at the code and it will be very obvious what is going on.

@cristianoc
Copy link
Contributor

Thank you. If you are confident that your example using core will have the same problem when core is removed, I will assume that dependencies can be excluded as a possible contributing factor to the issue.

@cristianoc
Copy link
Contributor

Because of this, we can close this issue, as this is the core repository.
And this issue can move to the compiler repository.

@jmagaram
Copy link
Contributor Author

jmagaram commented Apr 5, 2023

Ok please move the issue to the other repository and reference this one. The issue is definitely not specific to Core.

@cristianoc
Copy link
Contributor

I'll ask a "friend" to read this convo and provide a small example:

What do you think would have been a concrete example for cristianoc's questions?

...more exchanges...

I apologize for the oversight. Here's the revised example incorporating the @genType annotations:

MyModule.res:

@genType
type t = int

@genType
let add = (a: t, b: t): t => a + b

Wrapper.res:

module MyModuleAlias = MyModule

I think we need a file that actually uses MyModuleAlias to illustrate the issue

You are correct. We need an additional file that uses MyModuleAlias to fully illustrate the issue. Let's create another file called Usage.res:

open Wrapper

let a = MyModuleAlias.t(5)
let b = MyModuleAlias.add(a, 3)

@cristianoc
Copy link
Contributor

I made a small alteration to the last file

Usage.res:

open Wrapper

let a = 5
@genType
let b = MyModuleAlias.add(a, 3)

That provides the self-contained minimal repro that I will open up an issue about in the compiler repo.

@cristianoc
Copy link
Contributor

Done: rescript-lang/rescript#6112
Closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants