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

Discriminated unions do not work when discriminating on a tuple type #56920

Closed
AaronFriel opened this issue Jan 2, 2024 · 9 comments
Closed
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@AaronFriel
Copy link

AaronFriel commented Jan 2, 2024

πŸ”Ž Search Terms

tuplenarrowing
discriminated union tuple

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about type narrowing, discriminated unions

⏯ Playground Link

Playground Link

πŸ’» Code

// Free variables for the test case below, we can try variations of these to see if any of these narrow correctly.
type Value = string;


const Sentinel = null;
type Sentinel = typeof Sentinel; // also tried: Symbol('sentinel')

export type JsonDocument = {
  bar: Value[];
};

// A type representing partially parsed values in the above JSON document:
type ParseEvent =
  | {
      path: ['bar', number, Sentinel];
      value: Value;
    }
  | {
      path: ['bar', Sentinel];
      value: Value[];
    };

const parseEvent: ParseEvent = {
  kind: 'complete',
  path: ['bar', {} as number, Sentinel],
  value: 'hello',
} as ParseEvent;

// Completely specifying the path fails to narrow the type of the value, but succeeds in narrowing the type of the path:
{
  if (typeof parseEvent.path[1] === 'number' && parseEvent.path[2] === Sentinel) {
    // Succeeds.
    const path: ['bar', number, Sentinel] = parseEvent.path;

    // Unexpected error: Type 'Value | Value[]' is not assignable to type 'Value' forall Value.
    const value: Value = parseEvent.value; // Fails: Type 'Value | Value[]' is not assignable to type 'Value' forall Value.
  }
}

// Specifying a unique part of the path fails to narrow the type of the value or the path:
{
  if (parseEvent.path[2] === Sentinel) {
    // Unexpected error: Type '["bar", number, null] | ["bar", null]' is not assignable to type '["bar", number, null]'.
    const path: ['bar', number, Sentinel] = parseEvent.path;

    // Unexpected error: Type 'Value | Value[]' is not assignable to type 'Value' forall Value.
    const value: Value = parseEvent.value;
  }
}

function IsArrayPathKey(part: any): part is number {
  return true;
}

// Using a type guard to narrow the type of the path fails to narrow the type of the value or the path:
{
  if (IsArrayPathKey(parseEvent.path[1]) && parseEvent.path[2] === Sentinel) {
    // Same error as above.
    const path: ['bar', number, Sentinel] = parseEvent.path;

    // Same error as above.
    const value: Value = parseEvent.value;
  }
}

// This works, given the ts-pattern library is installed.

import { match, P } from 'ts-pattern';
{
  match(parseEvent).with({ path: ['bar', P.number, Sentinel] }, (parseEvent) => {
    // Works!? ts-pattern is able to narrow the path and value based on the pattern.
    const path: ['bar', number, Sentinel] = parseEvent.path;

    const value: Value = parseEvent.value;
  })
}

πŸ™ Actual behavior

Discriminating on the parseEvent.path value, a tuple type, in a conditional expression, the if conditions of the code above, does not work to narrow the type of the entire value.

When discriminating on the entire tuple, the parseEvent.path type is narrowed, but parseEvent.value type is not.

When discriminating on a part of the tuple, e.g.: the 3rd element of the tuple which exists in one of the unioned types and not the other, neither the types of parseEvent.path nor parseEvent.value are narrowed.

πŸ™‚ Expected behavior

The code above should have no errors, and using a tuple as a discriminator should largely succeed.

Additional information about the issue

No response

@fatcerberus
Copy link

fatcerberus commented Jan 2, 2024

Duplicate of #48500 (comment) and/or #55246.

The current rule is (consistently, I guess?) that a potential discriminant property has to have at least one literal type in it to be a discriminant.

A tuple is not a literal type, even if it contains literal-typed elements.

@AaronFriel
Copy link
Author

A tuple is not a literal type, even if it contains literal-typed elements.

Is this documented and/or considered not a bug?

@MartinJohns
Copy link
Contributor

Is this documented

GitHub issues act as documentation as well.

@RyanCavanaugh
Copy link
Member

The documentation never claims that referenced-typed discriminants are supported. Not sure how we could document every negative statement about the language.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Jan 2, 2024
@AaronFriel
Copy link
Author

@RyanCavanaugh This is also the case if all of the entries in the tuple are literals. I don't think a referenced type is necessary:

type Union =
  | {
      discriminator: ['foo'];
      value: string;
    }
  | {
      discriminator: ['bar'];
      value: number;
    };

let foo: Union = {} as any;
if (foo.discriminator[0] === 'foo') {
  foo.discriminator; // ["foo"]
  foo.value; // string | number
}

@fatcerberus
Copy link

fatcerberus commented Jan 2, 2024

@AaronFriel That case would be covered by #18758

@AaronFriel
Copy link
Author

It does seem like #18758 is closer in spirit to this issue. I think this issue makes a slightly different, more specific ask. It's a bit of a blocker for a nice JavaScript/ES and TypeScript friendly API for the library I've just published, fn-stream.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2024
@AaronFriel
Copy link
Author

Well that's unfortunate, is there a better label for this so that it remains an open issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

5 participants