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

genType cannot "unwrap" some types with -open ReactV3 #5990

Closed
2 of 5 tasks
illusionalsagacity opened this issue Feb 7, 2023 · 8 comments
Closed
2 of 5 tasks

genType cannot "unwrap" some types with -open ReactV3 #5990

illusionalsagacity opened this issue Feb 7, 2023 · 8 comments
Milestone

Comments

@illusionalsagacity
Copy link
Contributor

illusionalsagacity commented Feb 7, 2023

Thank you for filing! Check list:

  • Is it a bug? Usage questions should often be asked in the forum instead.
  • Concise, focused, friendly issue title & description.
  • A minimal, reproducible example.
  • OS and browser versions, if relevant.
  • Is it already fixed in master?

I'm not really sure where this issue belongs

Given this ReScript code:

@genType
type publicAction = Foo | Bar({hello: string})

@genType
let use = () => {
  React.useCallback0(
  action =>
  switch action {
  | Foo => "Foo"
  | Bar({hello}) => hello
  })
}

JSXv3 gentype Code reproduction repository

no longer "wraps" the callback to convert the type representation from the TypeScript one to ReScript's version.

/* TypeScript file generated from Example.res by genType. */
/* eslint-disable import/first */


// @ts-ignore: Implicit any on import
import * as ExampleBS__Es6Import from './Example.bs';
const ExampleBS: any = ExampleBS__Es6Import;

import type {React_callback as ReactV3_React_callback} from '@rescript/react/src/v3/ReactV3.gen';

// tslint:disable-next-line:interface-over-type-literal
export type publicAction = "Foo" | { readonly hello: string };

export const use: () => ReactV3_React_callback<publicAction,string> = ExampleBS.use;

@rescript/react@~0.10.0 and rescript@10.0.1 produce this:

/* TypeScript file generated from Example.res by genType. */
/* eslint-disable import/first */


const $$toRE3555235: { [key: string]: any } = {"Foo": 0};

// @ts-ignore: Implicit any on import
import * as ExampleBS__Es6Import from './Example.bs';
const ExampleBS: any = ExampleBS__Es6Import;

// tslint:disable-next-line:interface-over-type-literal
export type publicAction = "Foo" | { readonly hello: string };

export const use: () => (_1:publicAction) => string = function () {
  const result = ExampleBS.use();
  return function (Arg1: any) {
      const result1 = result(typeof(Arg1) === 'object'
        ? Object.assign({TAG: 0}, Arg1)
        : $$toRE3555235[Arg1]);
      return result1
    }
};

I think this related is a fairly long-standing issue with gentype where the types don't "recurse" into third-party modules to resolve / unwrap. i.e. React_callback is just a type, it can't be resolved to a function that needs to have it's arguments wrapped.

rescript-association/genType#458

I can try to see if adding gentype annotations to a locally linked @rescript/react will help, but do not have time to do this immediately--Having trouble getting the compiler going on the m1 at the moment, I can try to reproduce on an intel mac later.

I fully realize this could (and probably should) be considered better output code, but it breaks gentype usage from typescript, which is IMO fairly disruptive

@cristianoc
Copy link
Collaborator

Would you qualify under what conditions this happens?
E.g. does it depend on the version of rescript-compiler, or just on the version of rescript-react (the latest rescript-react should work, once one adds -open ReactV3", on older compiler versions too).
So it would be useful to know more what the cause is, if it's the different way of organising code in the latest rescript-react (i.e. a mixture of -open and types defined in other modules), or something else.

@cristianoc
Copy link
Collaborator

OK looks like it's the path to type React.callback.
If one uses -open ReactV3, the type becomes ReactV3.React.callback, and genType does not recognise the type anymore.

A simpler test is:

open ReactV3

@genType
type ttt = React.callback<int, string>

This is pretty easy to patch, but it would be good to know what are all the types one needs to worry about.
From a look at the types treated specially, here's a possible list:

React.callback
React.componentLike
React.component
React.Context.t
React.Ref.t
React.ref
React.element

@illusionalsagacity
Copy link
Contributor Author

Yeah, it appears to be the @rescript/react@0.11.0 and with bsc-flags: ["-open ReactV3"] only. i.e. it behaves the same with rescript-compiler@10.0.1

@illusionalsagacity illusionalsagacity changed the title JSXv3 + GenType no longer generates wrapper code for useCallback genType cannot "unwrap" some types with -open ReactV3 Feb 8, 2023
cristianoc added a commit that referenced this issue Feb 8, 2023
@cristianoc
Copy link
Collaborator

@illusionalsagacity can you try with #5991, once CI is finished producing the package.
The PR can then be back ported to compiler 10.1. A bug fix release is coming.

@illusionalsagacity
Copy link
Contributor Author

I am not sure that I did this right, but it didn't appear to change the output:

  • downloaded the binaries-darwinarm64 from here
  • copied them into node_modules/rescript (over-writing what was there)
$ ./node_modules/rescript/rescript build -with-deps -verbose
BSB check build spec : ..rescript-10.1-gentype-bug/rescript-master/src
Package @rescript/react -> ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react
Package stack: rescript-master
Package stack: rescript-master @rescript/react
Dependency on @rescript/react
BSB check build spec : ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react/src
Entering ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react/lib/bs
Cmd: ..rescript-10.1-gentype-bug/rescript-master/node_modules/rescript/darwin/ninja.exe
Installation started
Entering ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react/lib/ocaml
Cmd: ..rescript-10.1-gentype-bug/rescript-master/node_modules/rescript/darwin/ninja.exe -f ../bs/install.ninja
Installation finished
Dependency Finished
ninja.exe -C lib/bs

@cristianoc
Copy link
Collaborator

I am not sure that I did this right, but it didn't appear to change the output:

  • downloaded the binaries-darwinarm64 from here
  • copied them into node_modules/rescript (over-writing what was there)
$ ./node_modules/rescript/rescript build -with-deps -verbose
BSB check build spec : ..rescript-10.1-gentype-bug/rescript-master/src
Package @rescript/react -> ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react
Package stack: rescript-master
Package stack: rescript-master @rescript/react
Dependency on @rescript/react
BSB check build spec : ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react/src
Entering ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react/lib/bs
Cmd: ..rescript-10.1-gentype-bug/rescript-master/node_modules/rescript/darwin/ninja.exe
Installation started
Entering ..rescript-10.1-gentype-bug/rescript-master/node_modules/@rescript/react/lib/ocaml
Cmd: ..rescript-10.1-gentype-bug/rescript-master/node_modules/rescript/darwin/ninja.exe -f ../bs/install.ninja
Installation finished
Dependency Finished
ninja.exe -C lib/bs

You need to download npm-packages and install the compiler.

@cristianoc
Copy link
Collaborator

Btw I tested your repo https://github.com/illusionalsagacity/rescript-10.1-gentype-bug and it's working.

cristianoc added a commit that referenced this issue Feb 8, 2023
@illusionalsagacity
Copy link
Contributor Author

Btw I tested your repo https://github.com/illusionalsagacity/rescript-10.1-gentype-bug and it's working.

Working for me too, thanks!

@cristianoc cristianoc added this to the v10.1 milestone Feb 9, 2023
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

2 participants