Fable: Current generated definitions from type script files are not useful

Created on 23 Oct 2017  路  24Comments  路  Source: fable-compiler/Fable

Description

I successfully generated ts2fable import files. Based on type definition. But I guess we need to recheck what we are doing there. Or may I am missing something very obvious.

Repro code

Here is gist of both generated and corrected files.

Expected and actual results

Generated file should work without any correction required.

I need to pass setting into function. Now, if I create a class and implement interface of setting and then pass it, it will not work. As it is creating properties like Id@ instead of id. If I see the generated code in Fable repl there is difference in creating object property like for first [id@]=null and for second this.id=null . And because of that prior code is failing.

Related information

  • Fable version (dotnet fable --version): Current
  • Operating system: Windows 10

PS: I was facing another issue. Even though I am calling global function webpack is not including library files. But if I do import and dynamic feature of Fable (with some combination of static typing) everything works like charm. This issue is separate from above issue but it might be helpful to solving above one. So, putting both together.

Most helpful comment

@MangelMaxime where were you man... I guess this will solve many similar kind of issue. And I guess this allow me to create setting interface for own code. Thanks man... Thanks a lot.

All 24 comments

Hi @kunjee17, thanks for your comments. We're currently updating ts2fable which is placed now in its own repo, please see https://github.com/fable-compiler/ts2fable/pull/19.

We also have there an issue to track ideas for the new ts2fable version https://github.com/fable-compiler/ts2fable/issues/1

@kunjee17, can you update the gist and add the TypeScript input file? I'm currently working on a new major version of ts2fable and would like to test it.

@ctaggart hey, I have updated the gist with d.ts file. Let me know if any thing is required from my end. And we are updating ts2fable.

One more thing came as blocker to me. There are many authors who are now coding in TypeScript only. So, they do have ts files with Types and Definitions. So, they don't bother creating separate file. Instead give one file invoking all other files and exporting global variable. It works in typescript project but surely not helpful to convert things.

@alfonsogarciacaro Is there any reason we need to provide interface. Even typescript started supporting record types (as interface). So, why can't we use that. Because F# interface is little bit different than C# one, and surely TypeScript one.

TypeScript interface just bound class to have some properties so it is more of (record) type in C# clothes.

@ctaggart if you want generated code for this I am trying to do then let me know. I ll paste it from Fable repl. It may give better idea.

@kunjee17 I agree that TypeScript interfaces are not the same as F#/C# interfaces but unfortunately they are still much more different than records. This can be easily seen when checking how Fable translates records to JS. Main differences:

  • F#/Fable records are actual classes so you can do type testing against them, but TS interfaces are structurally typed: they can correspond to either a class instance or a plain JS object containing all (but not necessarily only) the interface members.
  • F#/Fable records always have a constructor, this is not always the case for TS interfaces. Even using Pojo records, it's not practical to use F# records to instantiate big TS interfaces with many optional members (more about that here).
  • F#/Fable records have metadata that can be used for serializing, use reflection methods, etc
  • F#/Fable records guarantee structural equality and comparison

I'm working on izitoast.d.ts as a test for https://github.com/fable-compiler/ts2fable/pull/19. I need to map String Type Literals and the variable still. It currently outputs:

namespace rec Fable.Import
open System
open System.Text.RegularExpressions
open Fable.Core
open Fable.Import.JS


    type Globals =
        class end

    type IziToastPosition =
        obj

    type IziToastTransitionIn =
        obj

    type IziToastTransitionOut =
        U6<obj, obj, obj, obj, obj, obj>

    type [<AllowNullLiteral>] IziToastSettings =
        abstract id: string option with get, set
        abstract ``class``: string option with get, set
        abstract title: string option with get, set
        abstract titleColor: string option with get, set
        abstract titleSize: string option with get, set
        abstract titleLineHeight: string option with get, set
        abstract message: string with get, set
        abstract messageColor: string option with get, set
        abstract messageSize: string option with get, set
        abstract messageLineHeight: string option with get, set
        abstract backgroundColor: string option with get, set
        abstract color: string option with get, set
        abstract icon: string option with get, set
        abstract iconText: string option with get, set
        abstract iconColor: string option with get, set
        abstract image: string option with get, set
        abstract imageWidth: float option option with get, set
        abstract maxWidth: float option option with get, set
        abstract zindex: float option option with get, set
        abstract layout: float option with get, set
        abstract balloon: bool option with get, set
        abstract close: bool option with get, set
        abstract rtl: bool option with get, set
        abstract position: IziToastPosition option with get, set
        abstract target: string option with get, set
        abstract targetFirst: bool option with get, set
        abstract toastOnce: bool option with get, set
        abstract timeout: U2<bool, float> option with get, set
        abstract drag: bool option with get, set
        abstract pauseOnHover: bool option with get, set
        abstract resetOnHover: bool option with get, set
        abstract progressBar: bool option with get, set
        abstract progressBarColor: string option with get, set
        abstract animateInside: bool option with get, set
        abstract buttons: ResizeArray<obj_Par> option with get, set
        abstract transitionIn: IziToastTransitionIn option with get, set
        abstract transitionOut: IziToastTransitionOut option with get, set
        abstract transitionInMobile: IziToastTransitionIn option with get, set
        abstract transitionOutMobile: IziToastTransitionOut option with get, set
        abstract onOpening: Func<IziToastSettings, HTMLDivElement, unit> option with get, set
        abstract onOpened: Func<IziToastSettings, HTMLDivElement, unit> option with get, set
        abstract onClosing: Func<IziToastSettings, HTMLDivElement, string, unit> option with get, set
        abstract onClosed: Func<IziToastSettings, HTMLDivElement, string, unit> option with get, set

    type [<AllowNullLiteral>] IziToast =
        abstract show: settings: IziToastSettings -> unit
        abstract hide: settings: IziToastSettings * toast: HTMLDivElement * closedBy: string -> unit
        abstract info: settings: IziToastSettings -> unit
        abstract error: settings: IziToastSettings -> unit
        abstract warning: settings: IziToastSettings -> unit
        abstract success: settings: IziToastSettings -> unit
        abstract destroy: unit -> unit
        abstract settings: settings: IziToastSettings -> unit

@alfonsogarciacaro got your point. now, here is the case breaking for me.

In a simple scenario

type [<AllowNullLiteral>] IziToastSettings =
        abstract id: int with get, set

type IziToastSettingImpl() =
  interface IziToastSettings with
    member val id = 1 with get, set

will generate js

import { setType } from "fable-core/Symbol";
import _Symbol from "fable-core/Symbol";
export class IziToastSettingImpl {
  [_Symbol.reflection]() {
    return {
      type: "Stdin.IziToastSettingImpl",
      interfaces: ["Stdin.IziToastSettings"],
      properties: {}
    };
  }

  constructor() {
    this["id@"] = 1;
  }

  get id() {
    return this["id@"] | 0;
  }

  set id(v) {
    this["id@"] = v | 0;
  }

}
setType("Stdin.IziToastSettingImpl", IziToastSettingImpl);

Now, that breaks things. As library will search for Id not Id@. I am pretty sure that I may be using it wrong way but can you suggest something for it. Means we can still use interface without breaking generated js?

@kunjee17 I believe IziToastSettings is an "option" you pass to another function or something like that and not a class instance even in JS.

So you should do:

open Fable.Core.JsInterop
// For Fable > 1.2.4 

let options = jsOption<IziToastSettings>(fun o ->
  o.id <- 1
)

// Supported by all version of Fable
let options2 = createEmpty<IziToastSettings>
options2.id <- 1

@MangelMaxime where were you man... I guess this will solve many similar kind of issue. And I guess this allow me to create setting interface for own code. Thanks man... Thanks a lot.

On a side not fable Repl is not updated to Fable > 1.2.4. cc/ @MangelMaxime @alfonsogarciacaro

@kunjee17, how do these bindings look? I added the support for the string literal types and the variables. It can be created by checkout out the latest code from my branch:

cd src
dotnet fable npm-build
node ..\dist\ts2fable.js ..\node_modules\izitoast\dist\izitoast\izitoast.d.ts bin\izitoast.fs

This writes the izitoast.fs file with utf8 encoding:

namespace rec Fable.Import
open System
open System.Text.RegularExpressions
open Fable.Core
open Fable.Import.JS

type [<Erase>] Globals =
    [<Global>] static member iziToast with get(): IziToast = jsNative and set(v: IziToast): unit = jsNative

type [<StringEnum>] [<RequireQualifiedAccess>] IziToastPosition =
    | [<CompiledName "bottomRight">] BottomRight
    | [<CompiledName "bottomLeft">] BottomLeft
    | [<CompiledName "topRight">] TopRight
    | [<CompiledName "topLeft">] TopLeft
    | [<CompiledName "topCenter">] TopCenter
    | [<CompiledName "bottomCenter">] BottomCenter
    | [<CompiledName "center">] Center

type [<StringEnum>] [<RequireQualifiedAccess>] IziToastTransitionIn =
    | [<CompiledName "bounceInLeft">] BounceInLeft
    | [<CompiledName "bounceInRight">] BounceInRight
    | [<CompiledName "bounceInUp">] BounceInUp
    | [<CompiledName "bounceInDown">] BounceInDown
    | [<CompiledName "fadeIn">] FadeIn
    | [<CompiledName "fadeInDown">] FadeInDown
    | [<CompiledName "fadeInUp">] FadeInUp
    | [<CompiledName "fadeInLeft">] FadeInLeft
    | [<CompiledName "fadeInRight">] FadeInRight
    | [<CompiledName "flipInX">] FlipInX

type [<StringEnum>] [<RequireQualifiedAccess>] IziToastTransitionOut =
    | [<CompiledName "fadeOut">] FadeOut
    | [<CompiledName "fadeOutUp">] FadeOutUp
    | [<CompiledName "fadeOutDown">] FadeOutDown
    | [<CompiledName "fadeOutLeft">] FadeOutLeft
    | [<CompiledName "fadeOutRight">] FadeOutRight
    | [<CompiledName "flipOutX">] FlipOutX

type [<AllowNullLiteral>] IziToastSettings =
    abstract id: string option with get, set
    abstract ``class``: string option with get, set
    abstract title: string option with get, set
    abstract titleColor: string option with get, set
    abstract titleSize: string option with get, set
    abstract titleLineHeight: string option with get, set
    abstract message: string with get, set
    abstract messageColor: string option with get, set
    abstract messageSize: string option with get, set
    abstract messageLineHeight: string option with get, set
    abstract backgroundColor: string option with get, set
    abstract color: string option with get, set
    abstract icon: string option with get, set
    abstract iconText: string option with get, set
    abstract iconColor: string option with get, set
    abstract image: string option with get, set
    abstract imageWidth: float option option with get, set
    abstract maxWidth: float option option with get, set
    abstract zindex: float option option with get, set
    abstract layout: float option with get, set
    abstract balloon: bool option with get, set
    abstract close: bool option with get, set
    abstract rtl: bool option with get, set
    abstract position: IziToastPosition option with get, set
    abstract target: string option with get, set
    abstract targetFirst: bool option with get, set
    abstract toastOnce: bool option with get, set
    abstract timeout: U2<bool, float> option with get, set
    abstract drag: bool option with get, set
    abstract pauseOnHover: bool option with get, set
    abstract resetOnHover: bool option with get, set
    abstract progressBar: bool option with get, set
    abstract progressBarColor: string option with get, set
    abstract animateInside: bool option with get, set
    abstract buttons: ResizeArray<obj> option with get, set
    abstract transitionIn: IziToastTransitionIn option with get, set
    abstract transitionOut: IziToastTransitionOut option with get, set
    abstract transitionInMobile: IziToastTransitionIn option with get, set
    abstract transitionOutMobile: IziToastTransitionOut option with get, set
    abstract onOpening: Func<IziToastSettings, HTMLDivElement, unit> option with get, set
    abstract onOpened: Func<IziToastSettings, HTMLDivElement, unit> option with get, set
    abstract onClosing: Func<IziToastSettings, HTMLDivElement, string, unit> option with get, set
    abstract onClosed: Func<IziToastSettings, HTMLDivElement, string, unit> option with get, set

type [<AllowNullLiteral>] IziToast =
    abstract show: settings: IziToastSettings -> unit
    abstract hide: settings: IziToastSettings * toast: HTMLDivElement * closedBy: string -> unit
    abstract info: settings: IziToastSettings -> unit
    abstract error: settings: IziToastSettings -> unit
    abstract warning: settings: IziToastSettings -> unit
    abstract success: settings: IziToastSettings -> unit
    abstract destroy: unit -> unit
    abstract settings: settings: IziToastSettings -> unit

@ctaggart For completion, you still need to import the library. I would change

type [<AllowNullLiteral>] IziToast = 
 // ... 

to

type [<AllowNullLiteral>] IziToastInstance = 
 // ...

and then import the library like this:

let IziToast : IziToastInstance = importAll "iziToast"

Exposing only the value IziToast for usage

@ctaggart things looks way good. But I also agree with @Zaid-Ajaj for importing part. Another thing I was facing is even though Fable.Import.IziToast is working. I can't use Global.iziToast.show method. Because webpack remove it as part of tree shacking. So, I need to write line like

let iziToast: IziToast = importAll "iziToast"

to make it work. This way it is working all ok. So, we can fix the global issue with ts2fable then it would be super awesome.

@ctaggart one more thing. We need to import Fable.Import.Browser in case of HTMLElement is there in generated code. I have no idea that we should be doing that as part of TS2Fable or user can do it. But yes that is one more manual thing I need to do to make it work.

I'm a little confused about what imports should be generated. In this case of:

declare var iziToast: IziToast

For declared variables, instead of:

type [<Erase>] Globals =
    [<Global>] static member iziToast with get(): IziToast = jsNative and set(v: IziToast): unit = jsNative

It should produce this?

let iziToast: IziToast = importAll "iziToast"

I can make that change when I'm back from vacation next week.

@ctaggart I think we should try to use importAll because [<Global>] has already gave us some problem in the past. But I would want the confirmation of @alfonsogarciacaro before using that in the import (which in theory should not contain any runtime code only bindings).

@ctaggart One more little thing: can you please add comment docs to the binding code? that would be really awesome :smile:

Comment docs can be added if the TS type checker is used. Sorry @ctaggart, I haven't been following your PR very closely. Are you using the type checker or just the syntax parser?

About importAll, it's better to avoid that because it's an expression and bindings should avoid actual code. I'd recommend using the attribute version instead:

[<Import("*", "IziToast")>]
let IziToast: IExports = jsNative

The best reference for the bindings is the latest Fable.Import.Node written by @jgrund as we were deciding the new standards when creating them, like using an interface named IExports for the exposed members instead of the old Globals and then making it available using a public module value with the Import attribute as shown above 鈽濓笍

@ctaggart @alfonsogarciacaro one curious question. if index.d.ts is not having all definitions instead they are importing them from various ts files. We are still able to get them using TypeScript AST?

Only with the syntax AST I don't think it's possible, maybe with the type checker, but I'm not sure. There's also ts-simple-ast which can be helpful here.

The develop branch of ts2fable is active development and addresses these issues. I think we can close this issue and track the remaining separate issues in the ts2fable project:

Totally agree, thanks @ctaggart!

@kunjee17 Can you tell me if this import works?

module rec Fable.Import.izitoast
open System
open System.Text.RegularExpressions
open Fable.Core
open Fable.Import.JS

type [<StringEnum>] [<RequireQualifiedAccess>] IziToastPosition =
    | [<CompiledName "bottomRight">] BottomRight
    | [<CompiledName "bottomLeft">] BottomLeft
    | [<CompiledName "topRight">] TopRight
    | [<CompiledName "topLeft">] TopLeft
    | [<CompiledName "topCenter">] TopCenter
    | [<CompiledName "bottomCenter">] BottomCenter
    | [<CompiledName "center">] Center

type [<StringEnum>] [<RequireQualifiedAccess>] IziToastTransitionIn =
    | [<CompiledName "bounceInLeft">] BounceInLeft
    | [<CompiledName "bounceInRight">] BounceInRight
    | [<CompiledName "bounceInUp">] BounceInUp
    | [<CompiledName "bounceInDown">] BounceInDown
    | [<CompiledName "fadeIn">] FadeIn
    | [<CompiledName "fadeInDown">] FadeInDown
    | [<CompiledName "fadeInUp">] FadeInUp
    | [<CompiledName "fadeInLeft">] FadeInLeft
    | [<CompiledName "fadeInRight">] FadeInRight
    | [<CompiledName "flipInX">] FlipInX

type [<StringEnum>] [<RequireQualifiedAccess>] IziToastTransitionOut =
    | [<CompiledName "fadeOut">] FadeOut
    | [<CompiledName "fadeOutUp">] FadeOutUp
    | [<CompiledName "fadeOutDown">] FadeOutDown
    | [<CompiledName "fadeOutLeft">] FadeOutLeft
    | [<CompiledName "fadeOutRight">] FadeOutRight
    | [<CompiledName "flipOutX">] FlipOutX

type [<AllowNullLiteral>] IziToastSettings =
    abstract id: string option with get, set
    abstract ``class``: string option with get, set
    abstract title: string option with get, set
    abstract titleColor: string option with get, set
    abstract titleSize: string option with get, set
    abstract titleLineHeight: string option with get, set
    abstract message: string with get, set
    abstract messageColor: string option with get, set
    abstract messageSize: string option with get, set
    abstract messageLineHeight: string option with get, set
    abstract backgroundColor: string option with get, set
    abstract color: string option with get, set
    abstract icon: string option with get, set
    abstract iconText: string option with get, set
    abstract iconColor: string option with get, set
    abstract image: string option with get, set
    abstract imageWidth: float option option with get, set
    abstract maxWidth: float option option with get, set
    abstract zindex: float option option with get, set
    abstract layout: float option with get, set
    abstract balloon: bool option with get, set
    abstract close: bool option with get, set
    abstract rtl: bool option with get, set
    abstract position: IziToastPosition option with get, set
    abstract target: string option with get, set
    abstract targetFirst: bool option with get, set
    abstract toastOnce: bool option with get, set
    abstract timeout: U2<bool, float> option with get, set
    abstract drag: bool option with get, set
    abstract pauseOnHover: bool option with get, set
    abstract resetOnHover: bool option with get, set
    abstract progressBar: bool option with get, set
    abstract progressBarColor: string option with get, set
    abstract animateInside: bool option with get, set
    abstract buttons: ResizeArray<obj> option with get, set
    abstract transitionIn: IziToastTransitionIn option with get, set
    abstract transitionOut: IziToastTransitionOut option with get, set
    abstract transitionInMobile: IziToastTransitionIn option with get, set
    abstract transitionOutMobile: IziToastTransitionOut option with get, set
    abstract onOpening: (IziToastSettings -> HTMLDivElement -> unit) option with get, set
    abstract onOpened: (IziToastSettings -> HTMLDivElement -> unit) option with get, set
    abstract onClosing: (IziToastSettings -> HTMLDivElement -> string -> unit) option with get, set
    abstract onClosed: (IziToastSettings -> HTMLDivElement -> string -> unit) option with get, set

type [<AllowNullLiteral>] IziToast =
    abstract show: settings: IziToastSettings -> unit
    abstract hide: settings: IziToastSettings * toast: HTMLDivElement * closedBy: string -> unit
    abstract info: settings: IziToastSettings -> unit
    abstract error: settings: IziToastSettings -> unit
    abstract warning: settings: IziToastSettings -> unit
    abstract success: settings: IziToastSettings -> unit
    abstract destroy: unit -> unit
    abstract settings: settings: IziToastSettings -> unit

let [<Import("*","iziToast")>] iziToast: IziToast = jsNative

@ctaggart this may be the most seamless generated import file I have used since the time of inception of my Fable work. It worked without writing any helper file or changing anything in this file. Too good, Too good. Only thing I needed to add is Fable.Import.Browser. I guess that issue already is in consideration at https://github.com/fable-compiler/ts2fable/issues/26 . Other than that it is cool.

How can I use this instead of already installed ts2fable. So, keep using it with other stuff. Will help me to find more issue if they are there.

Again great work friends. 馃憤

@MangelMaxime thanks again for jsoptions tip. Work like charm. Reduced by helper module because of that.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

alfonsogarciacaro picture alfonsogarciacaro  路  3Comments

MangelMaxime picture MangelMaxime  路  3Comments

alfonsogarciacaro picture alfonsogarciacaro  路  3Comments

forki picture forki  路  3Comments

nozzlegear picture nozzlegear  路  3Comments