4.8.2 -> 4.8.3 regression: type narrowing fails with a discriminant type of never

[shigma]

Bug Report

🔎 Search Terms

regression, narrow, union, control flow

🕗 Version & Regression Information

• This changed between versions 4.8.2 and 4.8.3

⏯ Playground Link

Note: playground does not have 4.8.3 support

Playground link with relevant code

💻 Code

🙁 Actual behavior

4.8.3

🙂 Expected behavior

4.8.2

[jcalz]

Note: playground does not have 4.8.3 support

It does although you sometimes need to manually modify the url because the version you want isn't in the dropdown

Also, it's kind of hard to go back and forth looking at images of someone's IDE with boxes around things, and then at a playground link, and trying to match those up. Could you please put the code here as plaintext and use code comments to call attention to anything you want people to notice?

[shigma]

It does although you sometimes need to manually modify the url because the version you want isn't in the dropdown

@jcalz Thanks a lot for your quick response and useful tip.

It looks that the playground above does not report an error. Maybe I need to provide more specific code but the typings from source code is a bit too long. I'll provide some key typings first, and if it doesn't help I'll try to give a small-scale reproduction later.

/** https://discord.com/developers/docs/topics/gateway#payloads-gateway-payload-structure */
export interface GatewayPayloadStructure<O extends GatewayOpcode, T extends keyof GatewayEvents, D> {
  /** opcode for the payload */
  op: O
  /** event data */
  d?: D
  /** the event name for this payload */
  t?: T
}

// this gives out a union like
// GatewayPayloadStructure<GatewayOpcode.DISPATCH, "READY", ReadyEvent> | ... 55 more
export type GatewayPayload = {
  [O in GatewayOpcode]: O extends GatewayOpcode.DISPATCH
    ? {
      [T in keyof GatewayEvents]: GatewayPayloadStructure<GatewayOpcode.DISPATCH, T, GatewayEvents[T]>
    }[keyof GatewayEvents]
    : GatewayPayloadStructure<O, never, O extends keyof GatewayParams ? GatewayParams[O] : never>
}[GatewayOpcode]

// expect a `Message` here
export async function adaptMessage(bot: DiscordBot, meta: Message, session: Partial<Session> = {}) {}

export async function adaptSession(bot: DiscordBot, input: GatewayPayload) {
  const session = bot.session()
  if (input.t === 'MESSAGE_CREATE') {
    session.type = 'message'
    // `GatewayEvents['MESSAGE_CREATE']` is assignable to `Message`
    // In 4.8.2 `input` is narrowed but in 4.8.3 it isn't
    await adaptMessage(bot, input.d, session)
  }
}

And there is a failing CI with above error.

[shigma]

I'm now reproducing the error with strictNullChecks set to false: playground link

[andrewbranch]

Full repro for @typescript-bot:

// @strictNullChecks: false

/** https://discord.com/developers/docs/topics/gateway#payloads-gateway-payload-structure */
export interface GatewayPayloadStructure<O extends GatewayOpcode, T extends keyof GatewayEvents, D> {
  /** opcode for the payload */
  op: O
  /** event data */
  d: D
  /** the event name for this payload */
  t?: T
  /** sequence number, used for resuming sessions and heartbeats */
  s?: number
}

export type GatewayPayload = {
  [O in GatewayOpcode]: O extends GatewayOpcode.DISPATCH
    ? {
      [T in keyof GatewayEvents]: GatewayPayloadStructure<GatewayOpcode.DISPATCH, T, GatewayEvents[T]>
    }[keyof GatewayEvents]
    : GatewayPayloadStructure<O, never, O extends keyof GatewayParams ? GatewayParams[O] : never>
}[GatewayOpcode]

export interface GatewayParams {
  [GatewayOpcode.HELLO]: { b: 1 }
}

/** https://discord.com/developers/docs/topics/opcodes-and-status-codes#gateway-gateway-opcodes */
export enum GatewayOpcode {
  /** An event was dispatched. */
  DISPATCH = 0,
  /** Fired periodically by the client to keep the connection alive. */
  HEARTBEAT = 1,
  /** Starts a new session during the initial handshake. */
  IDENTIFY = 2,
  /** Update the client's presence. */
  PRESENCE_UPDATE = 3,
  /** Used to join/leave or move between voice channels. */
  VOICE_STATE_UPDATE = 4,
  /** Resume a previous session that was disconnected. */
  RESUME = 6,
  /** You should attempt to reconnect and resume immediately. */
  RECONNECT = 7,
  /** Request information about offline guild members in a large guild. */
  REQUEST_GUILD_MEMBERS = 8,
  /** The session has been invalidated. You should reconnect and identify/resume accordingly. */
  INVALID_SESSION = 9,
  /** Sent immediately after connecting, contains the `heartbeat_interval` to use. */
  HELLO = 10,
  /** Sent in response to receiving a heartbeat to acknowledge that it has been received. */
  HEARTBEAT_ACK = 11,
}

/** https://discord.com/developers/docs/topics/gateway#commands-and-events-gateway-events */
export interface GatewayEvents {
    /** message was created */
    MESSAGE_CREATE: { a: 1 }
    /** message was edited */
    MESSAGE_UPDATE: { a: 2 }
    /** message was deleted */
    MESSAGE_DELETE: { a: 3 }
}

function assertMessage(event: { a: 1 }) {}

export async function adaptSession(input: GatewayPayload) {
  if (input.t === 'MESSAGE_CREATE') {
    assertMessage(input.d)
  }
}

[typescript-bot]

The change between v4.8.2 and v4.8.3 occurred at 10870b4.

[ahejlsberg]

Simpler repro:

type Foo = { kind?: 'a', a: number } | { kind?: 'b' } | { kind?: never };

function gg(foo: Foo) {
    if (foo.kind === 'a') {
        foo.a;  // Error with 4.8.3 when strictNullChecks is off
    }
}

Appears to be caused by #50592 (where we now bi-directionally check comparability in discriminant narrowing).

[ahejlsberg]

I guess the bot got there a few seconds faster than me. 😄

[ahejlsberg]

Anyways, I'll come up with a fix.

[ahejlsberg]

Another way to write the repro is:

type Foo = { kind: 'a' | undefined, a: number } | { kind: 'b' | undefined } | { kind: undefined };

function gg(foo: Foo) {
    if (foo.kind === 'a') {
        foo.a;
    }
}

Since undefined is assignable and comparable to any type in non-strictNullChecks mode (strange yes, but that's how it is), it sort of becomes obvious what the issue is: In non-strictNullChecks mode, a type with a discriminant of undefined is always included. However, it just so happened that the uni-directional discriminant check sometimes excluded undefined, but now it consistently includes it.

So, I'm tempted to say this is working as intended.

[shigma]

I don't think it is about undefined, I removed the ? and still see this error:

Property 'a' does not exist on type '{ kind: "a"; a: number; } | { kind: never; }'.
  Property 'a' does not exist on type '{ kind: never; }'.

link

[typescript-bot]

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.

---

Comment by @andrewbranch

❌ Failed: -

• Argument of type '{ a: 1; } | { b: 1; }' is not assignable to parameter of type '{ a: 1; }'. Property 'a' is missing in type '{ b: 1; }' but required in type '{ a: 1; }'.

Historical Information

Version	Reproduction Outputs
4.4.2, 4.5.2, 4.6.2, 4.7.2, 4.8.2	👍 Compiled

[ahejlsberg]

@shigma Hmm, yes, this example fails with 4.8.3, but didn't with 4.8.2, regardless of strictNullChecks settings:

type Foo = { kind: 'a', a: number } | { kind: 'b' } | { kind: never };

function gg(foo: Foo) {
    if (foo.kind === 'a') {
        foo.a;  // Error with 4.8.3
    }
}

The issue appears to be that a discriminant type of never was matched by nothing before, but now is matched by anything due to the bi-directional comparability change in #50592. We definitely need to fix that.

[fatcerberus]

@ahejlsberg Out of curiosity, why is never considered a discriminant in the first place? It’s not a unit type; in fact being the bottom type it’s the exact opposite (initial object vs. terminal object)

[ahejlsberg]

Out of curiosity, why is never considered a discriminant in the first place?

It isn't. To be considered a discriminant property of a union type, at least one of the constituent properties must have a literal type, and at least two constituents must have different types for the property. But a property with a never type doesn't disqualify the union.

[fatcerberus]

Ah, that makes sense. I was under the impression that all union constituents must have a literal type (or union of literals) for the property to qualify as a DU. Thanks for the clarification.

[segevfiner]

@ahejlsberg It's still erroring on v4.8.4: https://www.typescriptlang.org/play?ts=4.8.4#code/C4TwDgpgBAYg9nKBeKBvKBrAlgOwCYBcUA5AIbEA0UpROArgLYBGEATlAL5QA+amuhEk2KcefbPloQAbm04BuAFCKAZnRwBjYFjg4oAc30AKFQiLw4ASjSKodqFhVQTCAHQS8yJCjLFrqW3sg0zhXUnk7AHpIqABRVlY4dgB3LGAACygAFlcADlcAZkC7DkVSoA

[ahejlsberg]

@segevfiner Yeah, you're right, the fix was tagged for 4.8.4 but somehow didn't make it into the release. It's definitely fixed in the main branch and any 4.9 release.