Skip to content
This repository was archived by the owner on Nov 13, 2023. It is now read-only.

Variant incorrect generated type #585

Closed
tom-sherman opened this issue Dec 19, 2021 · 5 comments
Closed

Variant incorrect generated type #585

tom-sherman opened this issue Dec 19, 2021 · 5 comments

Comments

@tom-sherman
Copy link

First raised on the forum here: https://forum.rescript-lang.org/t/gentype-different-representation-in-rescript-compared-to-generated-typescript/2750

Repro:

Given the following module https://github.com/tom-sherman/rescript-xstate/blob/0e97ec7ef321ceb6408ad52cf7b245f7cfdc6f13/src/XStateFunctor.res

@genType
type rec stateNode<'state, 'event> =
  | State({name: 'state, on: array<('event, 'state)>})
  | Compound({name: 'state, children: array<stateNode<'state, 'event>>, initial: 'state})
  | Final({name: 'state})

@genType
type stateList<'state, 'event> = array<stateNode<'state, 'event>>

It outputs this typescript

export type stateNode<state,event> = 
    { tag: "State"; value: { readonly name: state; readonly on: Array<[event, state]> } }
  | { tag: "Compound"; value: {
  readonly name: state; 
  readonly children: stateNode<state,event>[]; 
  readonly initial: state
} }
  | { tag: "Final"; value: { readonly name: state } };

// tslint:disable-next-line:interface-over-type-literal
export type stateList<state,event> = stateNode<state,event>[];

However this type is incorrect, as the values of the stateNode variant is something like:

{ TAG: 0, name: 'idle', on: [ [ 'FETCH', 'loading' ] ] }

Differences:

  • TAG vs tag
  • Tag values are integers vs the variant constructor names
  • Variant payload is nested in a value field vs existing as properties at the top level of the object
@a-c-sreedhar-reddy
Copy link

a-c-sreedhar-reddy commented Dec 24, 2021

Hi,

Yeah the representation of variants in the generated javascript and the typescript are not same. But this would not break the app and is type safe.

So genType generated bindings are in such a way that it accepts the typescript type representation and then converts them to ReScript representation before passing to rescript functions.

For the following ReScript code

@genType
type rec stateNode<'state, 'event> =
  | State({name: 'state, on: array<('event, 'state)>})
  | Compound({name: 'state, children: array<stateNode<'state, 'event>>, initial: 'state})
  | Final({name: 'state})

@genType
let logStateNode = (a: stateNode<string, string>) => Js.log(a)

The generated typescript bindings are

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


// @ts-ignore: Implicit any on import
import * as Test2BS__Es6Import from './Test2.bs';
const Test2BS: any = Test2BS__Es6Import;

// tslint:disable-next-line:interface-over-type-literal
export type stateNode<state,event> = 
    { tag: "State"; value: { readonly name: state; readonly on: Array<[event, state]> } }
  | { tag: "Compound"; value: {
  readonly name: state; 
  readonly children: stateNode<state,event>[]; 
  readonly initial: state
} }
  | { tag: "Final"; value: { readonly name: state } };

export const logStateNode: (a:stateNode<string,string>) => void = function (Arg1: any) {
  const result = 
/* WARNING: circular type stateNode. Only shallow converter applied. */
  Test2BS.logStateNode(Arg1.tag==="State"
    ? Object.assign({TAG: 0}, Arg1.value)
    : Arg1.tag==="Compound"
    ? Object.assign({TAG: 1}, Arg1.value)
    : Object.assign({TAG: 2}, Arg1.value));
  return result
};

@cristianoc
Copy link
Collaborator

Note There's a warning:
/* WARNING: circular type stateNode. Only shallow converter applied. */

The conversion is not supported for recursive types. Because of complexity and unclear semantics. It would at best make a deep copy, lose sharing, etc.

@err0r500
Copy link

err0r500 commented May 5, 2022

I think I've the same problem. I use ts-pattern in order to handle output of a rescript function from typescript (but exact same thing with a switch statement)

In my case it's based on a Belt.Result.t<'a, 'e>.

the generated typescript gives me a

type MyResult<a,e> = 
    { tag: "Ok"; value: a }
  | { tag: "Error"; value: e };

so I can pattern match on it (typescript side) with

match(result)
     .with({ tag: "Ok" }, () => ...)
     .with({ tag: "Error"}, () => ...)

but it breaks at runtime because it receives something like {"TAG":1,"_0":0}

do you know how I can overcome this (without handling the result in rescript)

EDIT : here's a minimal example : https://github.com/err0r500/rescript-ts-issue-minimal

@BlueHotDog
Copy link

Hi,

Yeah the representation of variants in the generated javascript and the typescript are not same. But this would not break the app and is type safe.

So genType generated bindings are in such a way that it accepts the typescript type representation and then converts them to ReScript representation before passing to rescript functions.

For the following ReScript code

@genType
type rec stateNode<'state, 'event> =
  | State({name: 'state, on: array<('event, 'state)>})
  | Compound({name: 'state, children: array<stateNode<'state, 'event>>, initial: 'state})
  | Final({name: 'state})

@genType
let logStateNode = (a: stateNode<string, string>) => Js.log(a)

The generated typescript bindings are

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


// @ts-ignore: Implicit any on import
import * as Test2BS__Es6Import from './Test2.bs';
const Test2BS: any = Test2BS__Es6Import;

// tslint:disable-next-line:interface-over-type-literal
export type stateNode<state,event> = 
    { tag: "State"; value: { readonly name: state; readonly on: Array<[event, state]> } }
  | { tag: "Compound"; value: {
  readonly name: state; 
  readonly children: stateNode<state,event>[]; 
  readonly initial: state
} }
  | { tag: "Final"; value: { readonly name: state } };

export const logStateNode: (a:stateNode<string,string>) => void = function (Arg1: any) {
  const result = 
/* WARNING: circular type stateNode. Only shallow converter applied. */
  Test2BS.logStateNode(Arg1.tag==="State"
    ? Object.assign({TAG: 0}, Arg1.value)
    : Arg1.tag==="Compound"
    ? Object.assign({TAG: 1}, Arg1.value)
    : Object.assign({TAG: 2}, Arg1.value));
  return result
};

This seems to not work when dealing with interfaces.

cristianoc added a commit to rescript-lang/rescript that referenced this issue Mar 28, 2023
@cristianoc
Copy link
Collaborator

Fixed in compiler v11

cristianoc added a commit to rescript-lang/rescript that referenced this issue Mar 28, 2023
… has become unnecessary. (#6099)

* Emit tags as strings.

* Allow more general type for tags.

Compile is_tag to `!== "object"` instead of `=== "string"`.

* Do not special case variants with only 1 case with payload.

Also the comment is not emitted anymore, since there's always a tag.
Not special casing means that the representation is uniform, and does not change when the type is extended. This is important with zero cost ffi, where the runtime representation is exposed to the user, to reduce possible surprises.

* Support @as("foo") to customize the representation of tags.

* Add support for @tag(...) to customize the property used for the tag.

* GenType: removed support for `@genType.as` which has become unnecessary.

* Restore res_syntax analysis

* Remove object converter.

* Remove conversion for variants.

* Emit the correct type for the variants runtime representation.

* Don't auto unbox variants with 1 payload.

* Refactor.

* Remove array converter

* Remove function converter.

* Allow recursive types.

Fixes rescript-association/genType#585

* Remove option converter.

* Remove some conversion code.

* Remove conversion code.

* Cleanup

* Dead code.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants