Skip to content

Discriminated unions do not work with never / optional discriminant #38144

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

Closed
RoboPhred opened this issue Apr 23, 2020 · 9 comments
Closed

Discriminated unions do not work with never / optional discriminant #38144

RoboPhred opened this issue Apr 23, 2020 · 9 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@RoboPhred
Copy link

RoboPhred commented Apr 23, 2020

TypeScript Version: 3.8.3

Search Terms:

  • Discriminated Unions
  • Negative Case

I am trying to write type specifications for the W3C Thing Definition spec, particularly DataSchema, which is a subset of json-schema.

DataSchema has several subclasses identified by a type key, but it can also be used without a type key, in which case only the base type properties should be allowed.

I have been unable to type this in such a way that a unifying type of all sub-interfaces plus base interface blocks the use of derived interface properties when no type is specified.

Code
Here is a simplified example:

interface Base {
    title: string;
}

interface DerivedFoo extends Base {
    type: "foo"
    foo: number;
}

type MyType = DerivedFoo | Base;

// Expected an error here:
//  `Base` does not define `foo`, and
//  `DerivedFoo` does not match due to no `type` property.
// Actual behavior: Intellisense offers up 'foo'
const test: MyType = {
    title: "test",
    foo: 42
}

// We get an error here, as expected.
const whatIsFoo = test.foo;

Expected behavior:

There should be an error on test, as foo is not defined in Base and DerivedFoo should not be matched due to no type: "foo".

Actual behavior:

No error is reported when defining properties for test. Intellisense recognizes foo despite the lack of

Playground Link: example

@MartinJohns
Copy link
Contributor

Looks like a duplicate of #38024.

@RoboPhred
Copy link
Author

RoboPhred commented Apr 23, 2020

I'm not sure that issue is exactly the same, or it didn't explore far enough. Typescript clearly knows foo is not allowed when attempting to access it, giving you an error. Why let you define properties only to refuse to let you access them later?

I did try explicitly defining the negative case by defining an interface with type: never, however, this has the opposite of the intended effect in that type now becomes required, despite the indication that it should not be present:

interface Base {
    title: string;
}

interface DerivedFoo extends Base {
    type: "foo"
    foo: number;
}

interface DefaultBase extends Base {
    type: never;
}

type MyType = DerivedFoo | DefaultBase;

// We specified that `type` is never, but
//  the typings error here demanding that we
//  supply one anyway
// Error: property `type` is required in DefaultBase
const test: MyType = {
    title: "test"
}

The end result is there is no possible way to represent a case where a property adds on additional

playground link

@MartinJohns
Copy link
Contributor

Typescript clearly knows foo is not allowed when attempting to access it, giving you an error. Why let you define properties only to refuse to let you access them later?

Because TypeScript has a structural type system. The type of your object literal { title: string; foo: number } is structurally compatible with the type of Base. That's why it allows you to assign this object literal to your variable.

Normally the excess property checks would prevent you from providing additional properties in your object literal, but that doesn't work due to #38024.

https://github.com/microsoft/TypeScript/wiki/FAQ#what-is-structural-typing

@RoboPhred
Copy link
Author

Fair enough, so I need to be able to specify to the type system that having a type property with any value should mismatch against the default case. I had thought I could do that with

interface DefaultBase extends Base {
  type: never;
}

But as seen on the playground, this does the opposite and starts demanding a type property, rather than excluding it. Perhaps the issue here is the excess property check should honor never properties.

@RyanCavanaugh
Copy link
Member

An object with an unoptional never field can't be constructed; this is the intended behavior.

The proposed mechanism in the OP wouldn't be sound. Consider

interface Base {
    title: string;
}

interface OtherFoo extends Base {
  type: "foo";
  foo: string;
}

interface DerivedFoo extends Base {
    type: "foo"
    foo: number;
}

An OtherFoo is a valid Base, so seeing type === "foo" is insufficient evidence to say that foo is number

@RoboPhred
Copy link
Author

RoboPhred commented Apr 24, 2020

I don't follow how that example relates to the OP. In such a case I would expect foo to be string | number.
Anyway, this already happens, typescript knows exactly what the type is in this case.

But that doesn't seem related to the feature I am requesting: The ability to specify a negative case / 'lack-of-property' discriminant.

The desired behavior here isn't about duplicate discriminants over a type, but about the ability to use the lack of a discriminant property as a choice in the discriminated types list. Currently it is possible to use discriminant types to select between various consts applied to a type key, I would expect there to also be a method of using the lack-of-key as a discriminant as well.

In my use case, W3C DataSchema has several subtypes, each defined by a type property. To use the properties of these subtypes, the type property must be present. However, that type property is optional. The lack of a type property means all DataSchema properties will work, but no subtype properties can be used.

I can model this fine for property access; the subtype properties only show up when gated with a check for the appropriate type value. The excess property check fails to handle it, however, and protecting against inadvertently bad DataSchema objects is the primary use case of this typing.

Edit: Perhaps my original example is too simplified. Here is something that might show the issue better

interface TypeIsNumber {
  type: "number";
  value: number;
}
interface TypeIsString {
  type: "string";
  value: string;
}
interface DefaultTypeIsBoolean {
  type: undefined; // How can we specify the lack of `type` is the discriminant?
  value: boolean;
  extraProp: string;
}

type MyType = TypeIsNumber | TypeIsString | DefaultTypeIsBoolean;

const asNumber: MyType = {
  type: "number";
  value: 42 // value should be typed number
  // extraProp should not be allowed here.
}

const asString: MyType = {
  type: "string";
  value: "hello"; // value should be typed string
  // extraProp should not be allowed here.
}

const asDefault: MyType = {
  value: false // value should be typed boolean
  extraProp: "foobar"; // extraProp should be recognized here.
}

@kgscialdone
Copy link

kgscialdone commented Apr 28, 2020

I'm dealing with the same issue in a slightly different use case. I want to narrow the input types of a function based on a discriminant, defaulting to the supertype if the discriminant isn't present; however, I currently have to define the discriminant as undefined explicitly, despite it being an optional property which intuitively should default to undefined. The issue is inconsistent on modification, and I can't pin down a cause; using interfaces as opposed to type aliases for the parameter types seems to reduce the issues but not remove them entirely, and sometimes even explicitly specifying undefined doesn't work consistently.

In other words, this feels like a buggy, frustrating mess.

type Supertype = {}
type Subtype1 = Supertype & { type: 't1' }
type Subtype2 = Supertype & { type: 't2' }

// type Test['requireType'] = 't1' | 't2' | undefined
type Test =
  { requireType?: undefined, eval: (param: Supertype) => void} |
  { requireType: 't1',       eval: (param: Subtype1) => void} |
  { requireType: 't2',       eval: (param: Subtype2) => void}

// Both of these work fine...
let op1: Test = {
  requireType: 't1',
  eval: p => // p: Subtype1
}

let op2: Test = {
  requireType: 't2',
  eval: p => // p: Subtype2
}

// This is a little inconsistent.,.
let op3: Test = {
  requireType: undefined,
  eval: p => // p: Supertype (inconsistent, sometimes p: any)
}

// And intuitively this should work, but doesn't.
let op4: Test = {
  eval: p => // should be p: Supertype ; actually p: any
}

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jun 1, 2020
@RyanCavanaugh
Copy link
Member

This basically boils down to a lack of exact types - with structural subtyping, there isn't a way to say "This property can't be present", nor a way to guarantee it doesn't happen through aliasing.

@dlthomas
Copy link

there isn't a way to say "This property can't be present"

Unless I misunderstand something, b?: never is exactly such a way as far as the logic of the type system goes: if this key is present, it must be a type that cannot exist; therefore, this key cannot be present.

The refinement system doesn't recognize that this the case (which may or may not be for strong technical reasons around implementation), which leads to rejecting valid programs that exploit it. It's a shame because it's a way of closing a hole that the refinement system creates by assuming unspecified fields are not present. Consider, similar to the above:

type A = { a: string };
type BC = { b: string, c: string };
type ABC = A | BC;

let ab: { a: string, b: string } = { a: 'a', b: 'b' };

This is all valid, should be treated as such, and is.

Now let's write a function that takes an ABC:

function foo(x: ABC): string {
    if ('b' in x) {
        return x.c;
    }
    return x.a;
}

This is a function that says it returns a string, but when passed our ab above will instead return undefined. From a type-theory perspective, this is clearly an error in refinement. From a pragmatic perspective, it may be an appropriate tradeoff (:shrug:).

If I add b?: never to the first option in the ABC type, then the call to foo is rejected appropriately, but the body of foo is inappropriately rejected.

All of that said, I'm not sure how the ergonomics ultimately work out of having to sometimes establish that those properties are missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

5 participants