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

null & {} should not simplify to never in a union #28421

Closed
weswigham opened this issue Nov 8, 2018 · 5 comments
Closed

null & {} should not simplify to never in a union #28421

weswigham opened this issue Nov 8, 2018 · 5 comments
Assignees
Labels
Bug A bug in TypeScript Domain: Literal Types Unit types including string literal types, numeric literal types, Boolean literals, null, undefined

Comments

@weswigham
Copy link
Member

While working on #28394 I was able to identify an issue with inference that was causing us to infer {} for a reverse mapped type's property types - the behavior of {} in an intersection was itself very strange and is what actually led to the problem (otherwise the bad inference would probably never have been observable unless someone was using a this type in their component!).

In short, if I have

interface ReactElement<T> {
    $type: string;
}
type ReactText = string | number;
type ReactChild = ReactElement<any> | ReactText;
interface ReactNodeArray extends Array<ReactNode> { }
type ReactFragment = {} | ReactNodeArray;
type ReactNode = ReactChild | ReactFragment | string | number | boolean | null | undefined;

and I write

declare var a: { children?: ReactNode; };
declare var b: { children?: ReactNode; } & { children?: {} };

a = b;
b = a;
//^^
//Type '{ children?: ReactNode; }' is not assignable to type '{ children?: ReactNode; } & { children?: {} | //undefined; }'.
//  Type '{ children?: ReactNode; }' is not assignable to type '{ children?: {} | undefined; }'.
//    Types of property 'children' are incompatible.
//      Type 'ReactNode' is not assignable to type '{} | undefined'.
//        Type 'null' is not assignable to type '{} | undefined'.

both assignments should work (under strictNullChecks), but they do not!

@weswigham weswigham added Bug A bug in TypeScript Domain: Literal Types Unit types including string literal types, numeric literal types, Boolean literals, null, undefined labels Nov 8, 2018
@weswigham
Copy link
Member Author

weswigham commented Nov 8, 2018

cc @ahejlsberg - I remember talking with you about this and weather intersections with {} and unit types can "count" as vacuous or not (since we waffle on weather {} implies an actual object or not) - I think this is a good practical example of why they cannot. Swapping the unconstrained type parameter base type to unknown might also help make this crop up less often, since unknown behaves like you'd expect it to in this case (ref #26796)!

@weswigham weswigham added this to the TypeScript 3.4.0 milestone Dec 8, 2018
@ahejlsberg
Copy link
Member

@weswigham I see the issue in #26796, but I don't understand what the problem is here. The ReactNode type is effectively the same as {} | null | undefined. When you intersect {} | null | undefined with {} | undefined you get {} | undefined (which is correct), and the error is that {} | null | undefined is not assignable to {} | undefined (which is also correct). What am I missing?

@weswigham
Copy link
Member Author

The {} only appeared from an inference failure - so failed inference introduced a type which affected the assignability of the result. Had we selected unknown instead of {}, it would have actually been fine. The issue is phrased like this because we often still treat {} like a top type, but in this case we do not and the comparisons break down because of it.

@ahejlsberg
Copy link
Member

Hmm. I don't think we want to start treating {} as a top type any more than we already do since it really isn't in --strictNullChecks mode. As you're saying above, the correct thing to do would be to infer unknown when we have no inference candidates.

@ahejlsberg
Copy link
Member

I'm going assign this back to you. With #28940 there is at least a way to explicitly request an unknown inference when we have no candidates. Beyond that, you could run the experiment of changing the default to unknown for unconstrained type parameters--but I have a feeling is too big of a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Literal Types Unit types including string literal types, numeric literal types, Boolean literals, null, undefined
Projects
None yet
Development

No branches or pull requests

3 participants