Comments (5)
Andarist
commented on April 27, 2024
3
There are 2 mechanisms at play here and in related cases.
getAdjustedTypeWithFacts focuses today on narrowing negated cases (TypeFacts.NEUndefined, TypeFacts.NENull, etc). That's why the other branch above works just fine. It's somewhat special because this kind of logic often can't be expressed easily by intersecting the current type with something else or by discriminating the original constraints etc. You need to handle those cases in a special way here - when something is not null then it has to be either {} or undefined. This has those kind of rules here.
Fixing it here, in a somewhat trivial way, has almost not downsides:
git diff
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index c8eefa7778..620f0ffe49 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -26986,6 +26986,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const reduced = recombineUnknownType(getTypeWithFacts(strictNullChecks && type.flags & TypeFlags.Unknown ? unknownUnionType : type, facts));
if (strictNullChecks) {
switch (facts) {
+ case TypeFacts.EQUndefined:
+ return mapType(reduced, t => getIntersectionType([t, undefinedType]));
+ case TypeFacts.EQNull:
+ return mapType(reduced, t => getIntersectionType([t, nullType]));
case TypeFacts.NEUndefined:
return removeNullableByIntersection(reduced, TypeFacts.EQUndefined, TypeFacts.EQNull, TypeFacts.IsNull, nullType);
case TypeFacts.NENull:
diff --git a/tests/baselines/reference/unknownControlFlow.types b/tests/baselines/reference/unknownControlFlow.types
index 7b320964b8..43cef8a6a2 100644
--- a/tests/baselines/reference/unknownControlFlow.types
+++ b/tests/baselines/reference/unknownControlFlow.types
@@ -214,56 +214,56 @@ function f21<T>(x: T) {
}
else {
x; // T
->x : T
+>x : T & undefined
}
if (x !== null) {
>x !== null : boolean
->x : T
+>x : (T & ({} | null)) | (T & undefined)
x; // T & ({} | undefined)
->x : T & ({} | undefined)
+>x : (T & {}) | (T & undefined)
}
else {
x; // T
->x : T
+>x : T & null
}
if (x !== undefined && x !== null) {
>x !== undefined && x !== null : boolean
>x !== undefined : boolean
->x : T
+>x : (T & null) | (T & {}) | (T & undefined)
>undefined : undefined
>x !== null : boolean
->x : T & ({} | null)
+>x : (T & null) | (T & {})
x; // T & {}
>x : T & {}
}
else {
x; // T
->x : T
+>x : (T & null) | (T & undefined)
}
if (x != undefined) {
>x != undefined : boolean
->x : T
+>x : (T & null) | (T & {}) | (T & undefined)
>undefined : undefined
x; // T & {}
->x : NonNullable<T>
+>x : T & {}
}
else {
x; // T
->x : T
+>x : (T & null) | (T & undefined)
}
if (x != null) {
>x != null : boolean
->x : T
+>x : (T & null) | (T & {}) | (T & undefined)
x; // T & {}
->x : NonNullable<T>
+>x : T & {}
}
else {
x; // T
->x : T
+>x : (T & null) | (T & undefined)
}
}
@@ -281,14 +281,14 @@ function f22<T extends {} | undefined>(x: T) {
}
else {
x; // T
->x : T
+>x : T & undefined
}
if (x !== null) {
>x !== null : boolean
->x : T
+>x : (T & {}) | (T & undefined)
x; // T
->x : T
+>x : (T & {}) | (T & undefined)
}
else {
x; // T
@@ -297,7 +297,7 @@ function f22<T extends {} | undefined>(x: T) {
if (x !== undefined && x !== null) {
>x !== undefined && x !== null : boolean
>x !== undefined : boolean
->x : T
+>x : (T & {}) | (T & undefined)
>undefined : undefined
>x !== null : boolean
>x : T & {}
@@ -307,30 +307,30 @@ function f22<T extends {} | undefined>(x: T) {
}
else {
x; // T
->x : T
+>x : T & undefined
}
if (x != undefined) {
>x != undefined : boolean
->x : T
+>x : (T & {}) | (T & undefined)
>undefined : undefined
x; // T & {}
->x : NonNullable<T>
+>x : T & {}
}
else {
x; // T
->x : T
+>x : T & undefined
}
if (x != null) {
>x != null : boolean
->x : T
+>x : (T & {}) | (T & undefined)
x; // T & {}
->x : NonNullable<T>
+>x : T & {}
}
else {
x; // T
->x : T
+>x : T & undefined
}
}
@@ -348,25 +348,25 @@ function f23<T>(x: T | undefined | null) {
}
if (x !== null) {
>x !== null : boolean
->x : T | null | undefined
+>x : (T & {}) | null | undefined
x; // T & {} | undefined
>x : (T & {}) | undefined
}
if (x != undefined) {
>x != undefined : boolean
->x : T | null | undefined
+>x : (T & {}) | null | undefined
>undefined : undefined
x; // T & {}
->x : NonNullable<T>
+>x : T & {}
}
if (x != null) {
>x != null : boolean
->x : T | null | undefined
+>x : (T & {}) | null | undefined
x; // T & {}
->x : NonNullable<T>
+>x : T & {}
}
}
@@ -957,7 +957,7 @@ function doSomething1<T extends unknown>(value: T): T {
>undefined : undefined
return value;
->value : T
+>value : T & undefined
}
if (value === 42) {
>value === 42 : booleanHowever, it kinda creates some extra type identities and the control flow isn't able to "merge" those types back to just T when they both branches join etc.
The other mechanism - the one that handles the case when T extends {} | null | undefined - is getNarrowableTypeForReference. It avoid the above problem by substituting constraints in constraints positions. Treating an unconstrained type parameter as constrained to unknown (or rather to {} | null | undefined aka unknownUnionType) could fix this here. However, I found that this has some - IMHO - negative effects on printed errors. So at the very least, some extra work around that would be done here. I also had other problem when following this route but I didn't yet have the time to analyze them.
from typescript.
Andarist
commented on April 27, 2024
2
The negated branch gets nicely narrowed down. The observed behavior is quite surprising here (TS playground):
function acceptNull(arg: null) {}
function acceptNonNull(arg: {} | undefined) {}
function test<T>(value: T) {
if (value === null) {
value;
// ^? (parameter) value: T
acceptNull(value) // error
} else {
value;
// ^? (parameter) value: T & ({} | undefined)
acceptNonNull(value) // ok
}
}from typescript.
RyanCavanaugh
commented on April 27, 2024
2
the control flow isn't able to "merge" those types back to just T when they both branches join etc.
This is a big deal, actually. If this invariant doesn't hold, then you get "spooky action at a distance" bugs where adding an if statement causes effects in later code outside the CFA path of the if
Any fix here needs to not have downsides. I'd really prefer there be more concrete upsides too; at the point you've narrowed a value to a unit type, it's not clear why you'd write the identifier over the value literal (i.e. why not just write acceptsNull(null); ?)
from typescript.
danvk
commented on April 27, 2024
1
This issue results in some odd behavior with inferred type predicates:
// ✅ returns x is T & ({} | undefined); good!
function isNonNull<T>(x: T) {
return x !== null;
}
// 😔 returns boolean; would prefer x is T & null
function isNull<T>(x: T) {
return x === null;
}
// 🤔 returns x is T & null
function isNull2<T>(x: T) {
return (typeof x === "object" && x === null);
}in particular see this tweet for the source of this issue.
I'd guess that checking for non-null/undefined is the more common case, so it's nice that type predicates are at least inferred in that direction.
from typescript.
jcalz
commented on April 27, 2024
This is sort of a duplicate of #13995 except that that is closed because it's largely fixed.
Note that the fact that value is still of type T isn't itself a problem. It's that value is not also constrained by null. That is, the real problem is in the const v: null = value line in:
function isNullBad<T>(value: T) {
if (value === null) {
value
// ^? (parameter) value: T // this is okay
const v: null = value; // error, this is not
}
return value === null;
}#13995 was fixed by #43183 and specifically only for the case where the generic type parameter is constrained to a union. So if you want to work around it today you could constrain T to {} | null | undefined, which is a near-synonym of unknown, the implicit constraint of an unconstrained type parameter:
function isNull<T extends null | {} | undefined>(value: T) {
if (value === null) {
value // still T
// ^? (parameter) value: T extends null | {} | undefined // this is still okay
const v: null = value; // okay now also
}
return value === null;
}from typescript.
Related Issues (20)
- checkJs finds hundreds of errors in .JS produced by TypeScript itself HOT 6
- TS7018 error without file and line number HOT 6
- JSDoc tags get lost when inheriting from a grandparent class
- Smarter String includes/endsWith/startsWith using template literal type predicates HOT 3
- Typed key accessor fails to infer type when used with generic HOT 2
- Upstream file not correctly recognized as output of other project when using path mappings HOT 9
- [nightly][regression] Wrong generic parameter inferred for constructed class instance HOT 1
- [nightly][regression] Some emitted imports are syntactically invalid HOT 5
- Array of numbers wrong sort HOT 9
- Mistake in README file
- Inconsistent Behavior with Equality Check Using `Pick<Readonly<T>, K>` in Mapped Type HOT 2
- Improve Omit field type check HOT 1
- Class parameter property with initializer before required property emits non-nullable parameter for declaration emit
- TypeScript emits incorrect type for setters in JSDoc
- Function returning `never` in branch doesn't narrow type if it's not explicitly annotated HOT 3
- Type Intersection incorrect for optional parameters in functions with `exactOptionalPropertyTypes: true`. `undefined` type is flip based off Intersection types
- Inconsistent tsc behavior on TS 5.4 HOT 3
- VSCode TypeScript extension writes tracing to the wrong directory on Mac OS HOT 1
- computed property name in an interface error when using `[]` already HOT 3
- Proposal: Type-side `instanceof` keyword and functionality HOT 8
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from typescript.