Discriminating property with never as possible type makes its enclosing object type disappear when narrowing

[jfet97]

🔎 Search Terms

discriminated union never, discriminating property never

🕗 Version & Regression Information

It happens in every version I tried.

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.8.0-dev.20241206#code/C4TwDgpgBAIglgZwMYFUB2cD2aoF4BQAPlAN5QAmiSACgE6ZgBcUaEAbhLQDRQCGzARigBfIqQpU6DZgCIw9MDJ4AjZgCYRYspWRSmUAK5pyEAGZxW5HkmYBmTfhNIANr1rQk2BMCiZm8ZHQsNHx8OFMACkwAOh0aBQBKUnwoXxSoAHoMqAA9AH58YSgIZwRoEnTMdKzcgtEwyIBCGLi9JIrUqtSa-MLi0vLK6uze4SA

💻 Code

type DiscUnion =
| { discProp: never, a: 1 }
| { discProp: "prop", b: 2 }
| { discProp: undefined, c: 3 }

declare const o: DiscUnion

if(o.discProp) {
  o
  // ^? { discProp: "prop"; b: 2; }
} else {
  o
  // ^? { discProp: undefined; c: 3; }
}

if(!o.discProp) {
  o
  // ^? { discProp: undefined; c: 3; }
} else {
  o
  // ^? { discProp: "prop"; b: 2; }
}

🙁 Actual behavior

{ discProp: never, a: 1 } disappears!

🙂 Expected behavior

In the first if, I expect { discProp: never, a: 1 } to be a possible type of o in the else branch. Viceversa, in the second if I expect it to be a possible type of o in the then branch.

Additional information about the issue

No response

[MartinJohns]

Related: #38144

I don't really see how a never discriminant makes sense.

[jfet97]

@MartinJohns the point is not using never as discriminant, but to not lose the enclosing object when narrowing is based on a property that has the never type on purpose between the possibilities.

[jcalz]

(not a TS team member)

This is behaving as intended. You can never witness a value of type never. That either means it's impossible or looking at that value causes an error to be thrown. As soon as you check o.discProp, the never possibility is eliminated. You shouldn't have a (non-optional) never type "on purpose" unless you're trying to say it can't happen. I don't understand the use case that motivates this. Surely if you're checking o.discProp and you expect the never case to be possible and don't expect it to explode at runtime, it's because it's optional, like { discProp?: never, a: 1 } What am I missing?

[jfet97]

One example of using never as type for a "shared" field is: https://github.com/microsoft/TypeScript/blob/main/src/compiler/types.ts#L2759

The problem I opened an issue for doesn't occur with the above for some reason. When a condition is checked on node.name with node having the type FunctionExpression | ClassExpression | ArrowFunction, like here, narrowing does not make ArrowFunction disappear, and that might be because there are no literal types involved so node.name is not considered a discriminant property.

If you add a literal type to the mix like I did the enclosing type of never disappears into nothing.

[jcalz]

Can you show an example where never is not meant to be a discriminant and is not ever read?

Let's say you have an ArrowFunction. What happens when you check its name property? Is there a runtime error? If not, then name is simply not of type never and the type is a lie, and, well, you get what you get. If there's a runtime error, then checking name should eliminate ArrowFunction from any union. I'm simply not seeing how this is supposed to work or where there's a bug in TypeScript.

I guess I'll wait for the TS team to chime in.

[jfet97]

@jcalz In the referenced code they assert the absence of node.name with Debug.assert(!node.name). If node is an ArrowFunction node.name would be never, so it's absent, therefore the case node.kind === SyntaxKind.ArrowFunction is handled.

I mean, we can talk about this usage of never as marker for an absolutely absent field as much as you want. In fact, I share some of your doubts. But that's not the point of my issue.

The point is that when the property is considered discriminant (because of the presence of a literal type?) the enclosing type of never disappears into nothing. Otherwise, like that ArrowFunction, it stays.

[RyanCavanaugh]

It seems like you're using : never when you should be using ?: undefined or ?: never

It's correct behavior for an object type with a property of type never to also become never; the fact that it doesn't always do this is only because it's not in general a computable property of a type.

[jfet97]

While checking the new failures generated by #60718 in mainstream repositories I found the following behaviour when strict is not set and strictNullChecks is set to false:

// adapted from continue codebase

type PackageDocsResult = (
  | { details?: never; error: "disc"; }
  | { details: string; error?: never }
);

declare const pdr: PackageDocsResult;

if (typeof pdr.error === "string") {
  pdr
  // ^? { details?: never; error: "disc"; } (OK)
} else {
  pdr
  // ^? never (???)
}

In this situation the error discriminating property doesn't get set to just never, but to ?: never. Therefore is perfectly possible to craft an instance of { details: string; error?: never }: it is sufficient to not set an error at all. This would be possible even with strict: true and exactOptionalPropertyTypes: true. So, the act of checking pdr.error should not completely eliminate the { details: string; error?: never } alternative. Right?

This is what TS already does, you can check it here.

The problem arises with strictNullChecks set to false, as you can see here. Is this expected behaviour when this flag is set to false for some reason I am missing?

Edit:
After some debugging I've understood that strictNullChecks: false makes null and undefined disappear, kind of. Therefore the above definition becomes equivalent to the following one:

type PackageDocsResult = (
  | { details: never; error: "disc"; }
  | { details: string; error: never }
);

As we discussed, it's okay if the enclosing object of a discriminating property typed as never disappears when narrowing. Therefore { details: string; error: never } will.

[RyanCavanaugh]

Marking in discussion since I think the PR seems much better overall than we'd have expected in terms of perf impact

[jfet97]

@RyanCavanaugh I must admit that both your explanation and @jcalz's one have convinced me, and I now find this behaviour of TypeScript more reasonable. I had opened this issue because, upon encountering the problem with ArrowFunction, I wanted to ensure that the behaviour was both known and reasonable.

It seems that the change to the name prop type from : never to ?: never hasn’t caused any issues, especially since, in the end, the ArrowFunction type appears to be confined to internal use. Therefore I don’t think it’s appropriate to change TypeScript’s behaviour because of this.

It could then be suggested to continue to modify the definition of PackageDocsResult from:

type PackageDocsResult = {
  packageInfo: ParsedPackageInfo;
} & (
  | { error: string; details?: never }
  | { details: PackageDetailsSuccess; error?: never }
);

to:

type PackageDocsResult = {
  packageInfo: ParsedPackageInfo;
} & (
  | { error: string; }
  | { details: PackageDetailsSuccess; }
);

// or the following, that seems to work under their configuration

type PackageDocsResult = {
  packageInfo: ParsedPackageInfo;
} & (
  | { error: string; details?: undefined }
  | { details: PackageDetailsSuccess; error?: undefined }
);

Unfortunately, I don’t think they intend to enable strictNullChecks. I found a note in their tsconfig.json: they have deliberately disabled it because it was causing too many errors.

 

In any case, seeing a constituent of a union disappear into nothing-ness can be an unexpected behavior, even if correct, and my PR certainly increases the cases in which this happens.

It might be worth considering a linter rule to handle these situations. The rule should discourage declaring properties with a never type. As alternatives, it could suggest using ?: never, ?: undefined, or omitting the declaration of the property altogether.