Improve TypeScript typing for unions #2096
Comments
@chrisvanderpennen Do you mind elaborating a bit on this, just so I can understand the use case better? I get that the union fields are untyped right now, but are you talking about interaction between Fable-generated TS and other TS, or just better-looking generated TS? If it's for interaction, how are we doing the same interaction in JS with unions, and why do we need a different way in TS, even if we could? I guess what I'm trying to understand is, is the reflection info that is generated for unions perhaps enough to simplify the interaction with unions, or does it need to be extended? |
|
This is about allowing the TS compiler to verify correct interaction between Fable-generated union TS and other hand-written TS. The main use case I have in mind is for using Fable to cross-compile class libraries for consumption in both .NET services and existing TS browser apps. As it currently stands record types and functions are typed really well but for unions a TS developer needs to refer to the F# type definition and their code cannot be verified correct by the TS compiler. The additional type information here gives the TS compiler visibility over valid union cases and their fields. The TS compiler can then verify correct interaction with Fable unions at compile time, and the language service shows detailed type information for cases and the fields tuple when constructing or interacting with union instances. The type changes here don't change the runtime representation of union instances. They do add additional static properties to the union class representing the valid cases, but these cannot collide with other F# defined static members on the union as it's illegal to declare static members that collide with a case label. If it'd help I can get some screenshots of what it looks like in vscode working with the current and proposed type definitions. |
|
Now that you mention it, I might be able to use the return type of the reflection function instead of the additional shape-defining type. I'll try it later and report back. |
|
@chrisvanderpennen Thanks for the clarification. Some random observations:
static ChangeValue: 0 = 0;
static Nothing: 1 = 1;
static Something: 2 = 2;
let msg = new Msg(Msg.Something, "Nothing"); |
oops 😊 I believe the way statics are compiled by TypeScript keeps them off the prototype, they're assigned directly to the class object. I'll confirm that.
I get a compile error if I try that locally, once the tag is passed the name parameter is narrowed to the appropriate literal. I might have missed something in the example code. Edit: Specifically the errors I got testing that were along the lines of "Nothing" is not compatible with "Something", and the function expects 4 arguments but was given 2. |
|
@chrisvanderpennen Nvm, that was me trying to simplify the class declaration. Constructors work as expected when class decl is casted to |
|
The class declaration as is comes out as roughly |
|
@chrisvanderpennen What about when classes are not being used, no change there? |
|
I think it would be largely the same, something along the lines of |
|
@chrisvanderpennen In general, LGTM, just need to figure out if this should go into the current version, or next, as the work on Fable v.next has already started by @alfonsogarciacaro. |
|
This works for when classes aren't used, but I had to remove the export type Msg = FSharpUnionCase<MsgCases>
export const Msg: FSharpUnionType<MsgCases> = declare(function App_View_Msg(
this: Msg,
tag: keyof MsgCases,
name: MsgCases[keyof MsgCases][0],
...fields: MsgCases[keyof MsgCases][1]
) {
this.tag = tag; // previously tag | 0, which widens to number
this.name = name;
this.fields = fields;
},
Union);
Msg.ChangeValue = 0;
Msg.Nothing = 1;
Msg.Something = 2;I can also confirm those statics don't pollute the instances or prototype: // output from tsc
export const Msg = (function () {
var _a;
return _a = class Msg extends Union {
constructor(tag, name, ...fields) {
super(tag, name, ...fields);
}
},
_a.ChangeValue = 0,
_a.Nothing = 1,
_a.Something = 2,
_a;
}());Still working on a way to not need the MsgCases declaration. |
|
@chrisvanderpennen See changes in #2100, hopefully not a big impact besides removing the tag name from the constructor. |
|
Yeah that should be all that's needed. I'd like to use that to also simplify the UnionCases shape but without the names there we can't correctly type the statics. Even if we had the result of cases() as a tuple of literals, I've seen plenty of ugly workarounds for that |
|
Sorry, this has taken longer than I'd hoped, but I think I've got a workable solution. I've changed the definition of FSharpUnionType so that instead of referring to FSharpUnionCase, the construct signature's return type definition is inlined. I was then able to redefine FSharpUnionCase as a conditional type that extracts the instance signature from FSharpUnionType. // Given a shape type as above, produce the following:
export type FSharpUnionType<Cases extends UnionCases> = {
// A construct signature, generic on Tag, that narrows name to the string literal representing the case label and subsequent parameters to the case fields
// Return type is Union<Cases, 0> | Union<Cases, 1>
new<Tag extends (keyof Cases & number)>(tag: Tag, name: Cases[Tag][0], ...fields: Cases[Tag][1]): {[Tag in keyof Cases]: Union<Cases, Tag & number>}[keyof Cases]
}
// Static members: { "label0": 0; "label1": 1 }
& {[Label in TagLabels<Cases>['label']]: Extract<TagLabels<Cases>, {label: Label}>['tag']}
// Extract the instance signature of an FSharpUnionType<>
export type FSharpUnionCase<TUnion> = TUnion extends FSharpUnionType<infer _> ? InstanceType<TUnion> : neverWith these updated definitions, the case shape can be moved inside the IIFE so it does not pollute the module scope: export const Msg = (function() {
type Msg$Cases = {
0: ["ChangeValue", [string]],
1: ["Nothing", []],
2: ["Something", [int32, string]]
}
return class Msg extends Union {
static ChangeValue: 0 = 0
static Nothing: 1 = 1
static Something: 2 = 2
constructor(tag: keyof Msg$Cases, name: Msg$Cases[keyof Msg$Cases][0], ...fields: Msg$Cases[keyof Msg$Cases][1]) {
super(tag, name, ...fields);
}
} as FSharpUnionType<Msg$Cases>
}())
export type Msg = FSharpUnionCase<typeof Msg> |
|
@chrisvanderpennen That's great, thank you. There's quite a bit of a code churn right now in the I've also been reading this old article about implementing discriminated unions using conditional types in TypeScript. So this may be another option. |
|
Possibly related: #2131 |
|
argh! the constructor side type breaks for generic unions :( i'll see if i can fix it |
|
I've been working on typing Result and I've got something working, it's just a bit cumbersome. I inlined the constructor declaration on FSharpUnionType and added the union's generic parameters to the inlined definition. Unfortunately because TypeScript's generic parameter inference is all or nothing, this means the caller needs to set the tag as both a generic argument and function argument to get the constructor types to narrow: new Result<number, string, 0>(0, "Ok", 1234);If expanding on the emitted JavaScript/TypeScript is on the cards for Fable 3, maybe something more along the lines of the C# helper properties would be more useful, with a compiler option to enable since obviously if the entire application is Fable these aren't necessary: type Result$Cases<_T, _U> = {
0: ["Ok", [_T]]
1: ["Error", [_U]]
}
export class Result<_T, _U> extends Union<Result$Cases<_T, _U>> {
IsOk(): this is Union<Result$Cases<_T, _U>, 0> { return this.tag === 0; }
IsError(): this is Union<Result$Cases<_T, _U>, 1> { return this.tag === 1; }
static Ok<_T, _U>(...args: Result$Cases<_T, _U>[0][1]) { return new Result<_T, _U>(0, "Ok", ...args) }
static Error<_T, _U>(...args: Result$Cases<_T, _U>[1][1]) { return new Result<_T, _U>(1, "Error", ...args) }
constructor(tag: keyof Result$Cases<_T, _U>, name: Result$Cases<_T, _U>[keyof Result$Cases<_T, _U>][0], ...fields: Result$Cases<_T, _U>[keyof Result$Cases<_T, _U>][1]) {
super(tag, name, ...fields)
}
} |
|
well that was a misclick, sorry! |
|
@chrisvanderpennen Thanks, much appreciated. I don't think there will be a problem expanding the generated definition, but still waiting to see how the JS/TS emitting without Babel will turn out in Fable 3. |
|
@ncave I have a question about Babel in Fable 3. If we are not going to use babel, does that mean we can't use babel plugins anymore and presets anymore? For example, babel injects polyfills when needed based on usage and allows for custom compilations of the eventual JS |
|
@Zaid-Ajaj Please see discussion in #2109. I don't see a reason to not be able to use Babel plugins and presets downstream after Fable if you need to, but if you have such a case, please add to #2109. |
|
@chrisvanderpennen Not to diminish in any way the outstanding work in your proposal, but here is an alternative approach I'd like to discuss, namely eliminating the problem altogether by generating separate classes for each union case: // for plain JavaScript, we can use a wrapper object as namespace
const MyUnion = {
Case1: class Case1 { constructor(tag, num) { this.tag = tag; this.num = num; } },
Case2: class Case2 { constructor(tag, str) { this.tag = tag; this.str = str; } },
};// for TypeScript, we can use a namespace (which gets compiled to IIFE)
namespace MyUnion {
export class Case1 { constructor(public tag: 1, public num: number) {} }
export class Case2 { constructor(public tag: 2, public str: string) {} }
}
type MyUnion = MyUnion.Case1 | MyUnion.Case2;
function getUnionCaseName(unionCase: MyUnion) {
return unionCase.constructor.name; // no need to keep around case names
}
function getValue(unionCase: MyUnion): number | string {
switch (unionCase.tag) {
case 1: return unionCase.num;
case 2: return unionCase.str;
}
}
const u1 = new MyUnion.Case1(1, 123);
const u2 = new MyUnion.Case2(2, "abc");
console.log(getValue(u1)); // 123
console.log(getValue(u2)); // abc
console.log(getUnionCaseName(u1)); // Case1
console.log(getUnionCaseName(u2)); // Case2What do you think? Obviously there may be some downsides, for example some code duplication if there are overwrites for custom equality, but those could probably be minimized by generating them once and referencing them in all union cases. |
|
Honestly if you're open to breaking changes, that's way neater than my clumsy attempts to add types without changing the runtime representations. Having no common base class prevents Riffing off your suggestion, how about: // --- fable.library stubs ---
abstract class Union {
public abstract tag: number;
public abstract fields: any[];
// Equals
// GetHashCode
// etc
}
function getUnionCaseName(unionCase: Union) {
return unionCase.constructor.name; // no need to keep around case names
}
// --- Generated code ---
namespace Result {
abstract class Result<TOk, TError> extends Union {
public IsOk(): this is Result.Ok<TOk, TError> { return this.tag === 1; }
public IsError(): this is Result.Error<TOk, TError> { return this.tag === 2; }
}
export class Ok<TOk, TError> extends Result<TOk, TError> {
tag = 1 as const;
fields; // inferred below - (property) Result<TOk, TError>.Ok<TOk, TError>.fields: [ok: TOk]
constructor(...fields: [ok: TOk]) { super(); this.fields = fields; } // named tuple elements are a thing now :D
};
export class Error<TOk, TError> extends Result<TOk, TError> {
tag = 2 as const;
fields; // inferred below - (property) Result<TOk, TError>.Error<TOk, TError>.fields: [error: TError]
constructor(...fields: [error: TError]) { super(); this.fields = fields; }
};
}
type Result<TOk, TError> = Result.Ok<TOk, TError> | Result.Error<TOk, TError>;
// --- Usage ---
// constructor completion: Ok(ok: number): Result.Ok<number, string>
let succeed = function (): Result<number, string> { return new Result.Ok<number, string>(1); }
// constructor completion: Error(error: string): Result.Error<number, string>
let fail = function (): Result<number, string> { return new Result.Error<number, string>("bar's error"); }
let success = succeed();
let failure = fail();
switch (success.tag) {
case 1:
// fields tooltip: (property) Result<TOk, TError>.Ok<number, string>.fields: [ok: number]
let [n]: [number] = success.fields;
console.log(n); // log: 1
break;
case 2:
// fields tooltip: (property) Result<TOk, TError>.Error<number, string>.fields: [error: string]
let [e]: [string] = success.fields;
console.log(e);
break;
}
switch (failure.tag) {
case 1:
let [n]: [number] = failure.fields;
console.log(n);
break;
case 2:
let [e]: [string] = failure.fields;
console.log(e); // log: bar's error
break;
}
if (success.IsOk()) {
let [n]: [number] = success.fields;
console.log(n); // log: 1
}
if (success.IsError()) {
let [e]: [string] = success.fields;
console.log(e); // this branch isn't hit
}
if (failure.IsOk()) {
let [n]: [number] = failure.fields;
console.log(n); // this branch isn't hit
}
if (failure.IsError()) {
let [e]: [string] = failure.fields;
console.log(e); // log: bar's error
}
console.log(getUnionCaseName(success)); // log: Ok
console.log(getUnionCaseName(failure)); // log: Error |
|
@chrisvanderpennen I was trying to avoid using a base class for unions, for performance reasons (see #2153). |
|
wait what If it's not too much trouble, would you mind posting how I can reproduce your results? I'd like to try and get to the bottom of that. |
|
@chrisvanderpennen Assuming you're asking about the benchmark results, they are from compiling Fable with Fable-JS: See also some stats in #2056. Some results (machine-specific):
|
|
I would prefer to avoid emitting one class per union case. This would mean a lot of code for Elmish apps were union types are used intensively to represent the messages corresponding to UI events. Would it be possible to use a Typescript object union over name to have some pattern matching capabilities? Like: class MyUnionImpl {
constructor(public tag: number, public fields: [any]) {
}
get name() {
return ["Foo", "Bar"][this.tag];
}
}
type MyUnion = {
name: "Foo",
fields: [number]
} | {
name: "Bar",
fields: [string]
};
function test(x: MyUnion): number {
if (x.name === "Foo") {
const i = x.fields[0];
return i + 5;
} else {
const s = x.fields[0];
return s; // Compilation error, this is a string
}
}
test(new MyUnionImpl(0, [1]) as MyUnion); |
|
@alfonsogarciacaro Sure, although I don't think there will be that much more code, basically just a constructor for each case. |
|
Alright, after reading more v8 design documents than any sane person ever should, and running some experiments with native linux |
|
Closing for now as the design for unions in Fable 3 was already decided. Please reopen (or open a new issue) if necessary to discuss performance of generated code in next major version. Thanks a lot for the great insights @chrisvanderpennen @ncave! |
|
@alfonsogarciacaro @ncave is this something that could be considered for Fable 4? |
|
@kulshekhar We want to improve the Typescript generation for Fable 4 (although it may not be ready for the release), but I don't think we should change the way unions are compiled as this has been working well in JS so far. However, we can generate some extra type declarations (and maybe some helpers) to make it easier to interact with F# unions from TypeScript. I'm thinking of the following (drawback is TS users need to remember to use // F#
// type Union<'T> =
// | Foo of string * int
// | Bar of 'T
const enum Union_Tag { Foo = 0, Bar = 1 }
type Union_Fields<T> = [[string, number], [T]];
type Union_Type<T> = Union<T, Union_Tag.Foo> | Union<T, Union_Tag.Bar>;
class Union<T, Tag extends keyof Union_Fields<T>>{
readonly tag: Tag
readonly fields: Union_Fields<T>[Tag];
constructor(tag: Tag, fields: Union_Fields<T>[Tag]) {
this.tag = tag;
this.fields = fields;
}
}
// Optional helpers to make it easier to instantiate an F# union from TS
function Union_Foo(x: string, y: number) { new Union(Union_Tag.Foo, [x, y]); }
function Union_Bar<T>(x: T) { new Union(Union_Tag.Bar, [x]) };
function test<T>(u: Union_Type<T>, stringify: (x: T) => string): string {
switch (u.tag) {
case Union_Tag.Bar: {
return stringify(u.fields[0]);
}
case Union_Tag.Foo: {
return String(u.fields[1] + 3);
}
}
} |
|
One downside of this approach is obviously that the bundle size would be larger than that of handwritten TypeScript code from which these unions get completely erased at build time as opposed to classes. That said, the mechanism suggested above will make usage type safe to a large extent and I'd happily take this if this is a quick-ish win. Thank you! As an aside, I'd love to help on this front to make the generated code as close to idiomatic TypeScript as possible. I'm experienced but don't have much experience with building language level things. I'll be slow with this but with pointers for first steps and a general direction, I'd be more than happy to help with this if this aligns with the long term goals of Fable. |
|
Thanks a lot for your offer to help @kulshekhar! At the moment @ncave is the one working for the Typescript compilation but I also want to help. Things are a bit messy now for the Fable 4 release, but we'll try to coordinate better when everything calms down.
Adding type annotations should be fine because these are removed by Typescript (except the instantiation helpers, although these should be removed by tree shaking if not used). We had a Fable prototype to erase unions. There was some reduction in bundle size but not too big. We do need the classes because there are many features of F# unions that cannot be easily implemented with TS erased unions, like type testing at runtime, interface implementation or reflection. |
That's a fair point. In light of this, the suggested solution in your previous post looks quite good 🚀 |
|
This is implemented now in main 👍 |
Description
The current generated TypeScript definitions for unions makes them difficult to correctly use or construct with TypeScript.
An example:
is compiled to:
or a close equivalent with class types disabled.
I have been tinkering with this output and the fable-library types, and I've been able to combine a few additional definitions in fable-library with more explicit typing information on the generated union types that enable TypeScript to safely use and construct Fable generated union types without changing the object structure. It's not perfect, it creates an additional type declaration per union type in the compiled output and I'd like to find a way to avoid that, but I think it's good enough to share here for discussion.
The objectives I had were:
Four additional declarations are added to fable-library Types.ts:
Union is then made generic over an extension of UnionCases and possible Tag values:
Here,
keyof Cases & numberrestricts Tag to the index signature of UnionCases, and I provide defaults for both generic parameters to simplify the generated code.I then rewrote the Msg declaration from above to:
With this updated definition, the Msg constructor is narrowed by TypeScript language service as soon as a tag is specified:
let msg = new Msg(Msg.ChangeValue,Once constructed, it can then be tested with an ordinary switch statement:
I hope this is useful. I'm happy to help implementing something along these lines, though I'll likely need some pointers to where to get started.
The text was updated successfully, but these errors were encountered: