The current generated TypeScript definitions for unions makes them difficult to correctly use or construct with TypeScript.
An example:
type Msg =
| ChangeValue of value: string
| Nothing
| Something of value: int * name: string
is compiled to:
export class Msg extends Union {
constructor(tag: int32, name: string, ...fields: Array<any>) {
super(tag, name, ...fields);
}
}
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:
// Provide an interface that supports declaring F# union cases using the form:
// {
// 0: ["label0", [types, of, fields]],
// 1: ["label1", [types, of, fields]],
// }
interface UnionCases {[tag: number]: [string, any[]]}
// Convert a union case declaration as above into the form:
// { tag: 0; label: "label0" } | { tag: 1; label: "label1" }
// This is then used in FSharpUnionType to type static members mapping case labels to their tag
type TagLabels<Cases extends UnionCases> = {[Tag in keyof Cases]: {tag: Tag; label: Cases[Tag & number][0]}}[keyof Cases]
// Given a shape type as above, produce Union<Cases, 0> | Union<Cases, 1>
export type FSharpUnionCase<Cases extends UnionCases> = {[Tag in keyof Cases]: Union<Cases, Tag & number>}[keyof Cases]
// 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
// It does _not_ return an instance narrowed to the specific case constructed
new<Tag extends (keyof Cases & number)>(tag: Tag, name: Cases[Tag][0], ...fields: Cases[Tag][1]): FSharpUnionCase<Cases>
}
// Static members: { "label0": 0; "label1": 1 }
& {[Label in TagLabels<Cases>['label']]: Extract<TagLabels<Cases>, {label: Label}>['tag']}
Union is then made generic over an extension of UnionCases and possible Tag values:
export class Union<Cases extends UnionCases = UnionCases, Tag extends (keyof Cases & number) = (keyof Cases & number)> extends SystemObject implements IComparable<any> {
public readonly tag: Tag;
public readonly name: Cases[Tag][0];
public readonly fields: Cases[Tag][1];
constructor(tag: Tag, name: Cases[Tag][0], ...fields: Cases[Tag][1]) {
super();
this.tag = tag;
this.name = name;
this.fields = fields;
}
...
Here, keyof Cases & number restricts 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:
// this could collide with other valid f# types - not sure what the solution is
// maybe a computed type that extracts this shape from an FSharpUnionCase<> type
type MsgCases = {
0: ["ChangeValue", [string]],
1: ["Nothing", []],
2: ["Something", [int32, string]]
}
type Msg = FSharpUnionCase<MsgCases>
const Msg: FSharpUnionType<MsgCases> = (function() {
return class Msg extends Union {
static ChangeValue: 0
static Nothing: 1
static Something: 2
constructor(tag: keyof MsgCases, name: MsgCases[keyof MsgCases][0], ...fields: MsgCases[keyof MsgCases][1]) {
super(tag, name, ...fields);
}
} as FSharpUnionType<MsgCases>
}())
export { Msg }
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,
const Msg: new <0>(tag: 0, name: "ChangeValue", fields_0: string) => Union<MsgCases, 0> | Union<MsgCases, 1> | Union<MsgCases, 2>
Once constructed, it can then be tested with an ordinary switch statement:
let msg = new Msg(Msg.ChangeValue, "ChangeValue", "string")
switch (msg.tag) {
case Msg.ChangeValue:
{
let a: string;
[a] = msg.fields;
let len: 1 = msg.fields.length;
break;
}
case Msg.Nothing:
{
let len: 0 = msg.fields.length;
break;
}
case Msg.Something:
{
let a: number, b: string;
[a, b] = msg.fields;
let len: 2 = msg.fields.length;
break;
}
default:
assertNever(msg);
}
function assertNever(_: never) {}
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 current generated TypeScript definitions for unions makes them difficult to correctly use or construct with TypeScript.
@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");
Btw, they do need to be initialized:
oops :blush:
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.
Constructor still doesn't prevent mixing the tag with an incorrect name or type of parameters
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 FSharpUnionType<MsgCases>.
The class declaration as is comes out as roughly class extends Union<MsgCases, 0 | 1 | 2> and unfortunately TS doesn't narrow generic parameters. The cast is necessary to convert from that to Union<MsgCases, 0> | Union<MsgCases, 1> | Union<MsgCases, 2> which TS will happily narrow.
@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 export Msg: FSharpUnionType<MsgCases> = declare(...) but I haven't looked into it too closely yet. Next on the list :)
@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 readonly modifier from Union's fields:
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, keyof on a tuple returns string literals instead of number literals - "0", "1", "2", etc. - so we can't use that to correctly type the static value. At least all the type information is erased once it goes through tsc, and the minified statics shouldn't add much to the bundle.
I've seen plenty of ugly workarounds for that keyof restriction that basically consist of type TupleIndexMap = {"0": 0, "1": 1, "2": 2, ... "99": 99} but yuck...
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> : never
With 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 next branch, so it may take a bit longer to get it in.
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: https://github.com/fable-compiler/Fable/issues/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)); // Case2
What 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 foo instanceof MyUnion but I'm not sure that's much of a loss.
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:
git clone fable_repo
cd Fable
npm install
git checkout some_branch
rm -rf build
npm run build compiler
cd src/fable-standalone/test/bench-compiler
rm -rf dist
rm -rf out-node
rm -rf out-node2
npm run build-dotnet (or `npm run build-dotnet-cls` to compile with classes)
npm run build-node (the FCS running time is logged at the second output line)
Some node.js profiling is available, but inconclusive, at least to me:
npm run profile
npm run prof-process (modify to use the actual isolate file name)
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.
Anyway, it was just an discussion point, we'll stick with the existing union representation for this PR.
Alright, after reading more v8 design documents than any sane person ever should, and running some experiments with native linux perf tracing I'm pleased to report I have a derivative of master where classtypes is down to about 5% overhead compared to factories, measured end-to-end with time. I'll open a separate discussion issue with the details so we don't derail this further :)
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!
Most helpful comment
Alright, after reading more v8 design documents than any sane person ever should, and running some experiments with native linux
perftracing I'm pleased to report I have a derivative of master where classtypes is down to about 5% overhead compared to factories, measured end-to-end withtime. I'll open a separate discussion issue with the details so we don't derail this further :)