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

Transform methods with union type as arguments to overloaded methods #29

Open
MangelMaxime opened this issue Jan 9, 2024 · 3 comments

Comments

@MangelMaxime
Copy link
Contributor

MangelMaxime commented Jan 9, 2024

TypeScript documentation:

export class Logger {
    log(value : string | number): void
}

translates into

module rec Glutinum

open Fable.Core
open System

[<Erase>]
type Exports =
    interface end

[<AllowNullLiteral>]
type Logger =
    abstract member log: value: U2<string, float> -> unit

We should also tries to convert union type coming from type reference:

type Value = string | number

export class Logger {
    log(value : Value): void
}

translates into

module rec Glutinum

open Fable.Core
open System

[<Erase>]
type Exports =
    interface end

type Value =
    U2<string, float>

[<AllowNullLiteral>]
type Logger =
    abstract member log: value: string -> unit
    abstract member log: value: float -> unit
//    abstract member log: value: Value -> unit // Should we keep this version ?

The Value type should probably be kept if the user needs to write helpers or API depending on that type.

For the first implementation, we are going to try removing the type reference declaration if only used inside of method overloading.

Example of npm package using this features:

Day.js

@goswinr
Copy link

goswinr commented Jan 10, 2024

My intuition is to not have the erased union initially.
It could always be added later, but removing it later would be a braking change.

@MangelMaxime
Copy link
Contributor Author

I don't think we will always be able to remove that type definition. For example, if the type reference i used as the type of a property we will not be able to remove it.

But I updated the original issue to try removing the type reference and we will see where it cause problems.

type PostalCode = string | number

export class City {
	postalCode : PostalCode
}

export class Person {
	postalCode : PostalCode
}

@goswinr
Copy link

goswinr commented Jan 10, 2024

Yes, if a type is used as a return value in the same library or as property then it can not be split into overloads only. But I had the impression such types are most often used just as arguments.
It is then basically the same as using overloads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants