TypeScript
TypeScript copied to clipboard
Infer type predicates from function bodies using control flow analysis
Fixes #16069 Fixes #38390 Fixes #10734 Fixes #50734
This PR uses the TypeScript's existing control flow analysis to infer type predicates for boolean-returning functions where appropriate. For example:
function isString(x: string | number) {
return typeof x === 'string';
}
This currently has an inferred return type of boolean
, but with this PR it becomes a type predicate:
I filed #16069 seven years ago (!) and thought it would be interesting to try and fix it. It turned out to be cleaner and simpler than I thought: only ~65 LOC in one new function. I think it's a nice win!
How this works
A function is a candidate for an inferred type guard if:
- It does not have an explicit return type or type predicate.
- Its inferred return type is
boolean
. - It has a single
return
statement and no implicit returns (this could potentially be relaxed later). - It does not mutate its parameter.
If so, then the function looks something like this:
function f(p: T, p2: T2, ...) {
// ...
return expr;
}
For each parameter, this PR determine what its flow type would be in each branch if the function looked like this instead:
function f(p: T, p2: T2, ...) {
// ...
if (expr) {
p1; // trueType
} else {
p2; // falseType
}
}
The "else" branch is important because of the semantics of type predicates. If we have:
declare function isString(x: string | number): x is string;
then x
is a string
if this function returns true
. But if it returns false
then x
must be a number
. The exact condition is a little subtle because it depends on which negated types TypeScript is able to represent.
We can safely infer a type guard for a parameter if falseType = Exclude<T, trueType>
.
Update: Actually a little more subtle even than that, see https://github.com/microsoft/TypeScript/pull/57465#issuecomment-1957867140
Wins
TypeScript is now able to infer type guards in many places where it's convenient, e.g. calls to filter
:
const nums = [12, "foo", 23, "bar"].filter(x => typeof x === 'number');
// ^? const nums: number[]
Since this piggybacks off of the existing flow type code, all forms of narrowing that TypeScript understands will work.
const foos = [new Foo(), new Bar(), new Foo(), new Bar()].filter(x => x instanceof Foo);
// ^? const foos: Foo[]
There are a few other non-obvious wins:
Type guards now flow
const isString = (o: string | undefined): o is string => !!o;
// - (o: string | undefined) => boolean
// + (o: string | undefined) => o is string
const secondIsString = (o: string | undefined) => myGuard(o);
This is a gentle nudge away from truthiness footguns
We don't infer a type guard here if you check for truthiness, only if you check for non-nullishness:
const numsTruthy = [0, 1, 2, null, 3].filter(x => !!x);
// ^? const numsTruthy: (number | null)[]
const numsNonNull = [0, 1, 2, null, 3].filter(x => x !== null);
// ^? const numsNonNull: number[]
This is because of the false
case: if the truthiness test returns false
, then x
could be 0
. Until TypeScript can represent "numbers other than 0" or it has a way to return distinct type predicates for the true
and false
cases, there's nothing that can be inferred from the truthiness test here.
If you're working with object types, on the other hand, there is no footgun and a truthiness test will infer a predicate:
const datesTruthy = [new Date(), null, new Date(), null].filter(d => !!d);
// ^? const datesTruthy: Date[]
This provides a tangible incentive to do non-null checks instead of truthiness checks in the cases where you should be doing that anyway, so I call this a win. Notably the example in the original issue tests for truthiness rather than non-null.
Type guards are more discoverable
Type predicates are an incredibly useful feature, but you'd never learn about them without reading the documentation or seeing one in a declaration file. Now you can discover them by inspecting symbols in your own code:
const isString = (x: unknown) => typeof x === 'string';
// ^? const isString: (x: unknown) => x is string
This makes them feel like they're more a part of the language.
Inferred type guards in interfaces are checked
While this PR defers to explicit type predicates, it will check an inferred predicate in this case:
interface NumberInferrer {
isNumber(x: number | string): x is number;
}
class Inferrer implements NumberInferrer {
isNumber(x: number | string) { // this is checked!!!
return typeof x === 'number';
}
}
Interesting cases
The identity function on booleans is, in theory, a type guard:
// boolId: (b: boolean): b is true?
const boolId = (b: boolean) => b;
This seems correct but not very useful: why not just test the boolean? I've specifically prohibited inferring type predicates on boolean parameters in this PR. If nothing else this significantly reduces the number of diffs in the baselines.
This is an interesting case:
function flakyIsString(x: string | number) {
return typeof x === 'string' && Math.random() > 0.5;
}
If this returns true
then x
is a string
. But if it returns false
then x
could still be a string
. So it would not be valid to infer a type predicate in this case. This is why we have to check the falseType as well as the trueType. In general, combining conditions like this will prevent inference of a type predicate. It would be nice if there were a way for a function to return distinct type predicates for the true and false cases. This would make inference much more powerful. But that would be a bigger change.
I remember RyanC saying once that a function's return type shouldn't be a restatement of its implementation in the type system. In some cases this can feel like a move in that direction:
// function hasAB(x: unknown): x is object & Record<"a", unknown> & Record<"b", unknown>
function hasAB(x: unknown) {
return x !== null && typeof x === 'object' && 'a' in x && 'b' in x;
}
Performance
We're doing additional work to infer type predicates, so performance is certainly a concern. I've run this with --extendedDiagnostics
on a few of my own repos and didn't observe any significant changes. But TS Bot will have to be the judge here.
If there are performance concerns with the current implementation, there are a few options for reducing its scope:
- Only run this on arrow functions
- Only run this on contextually-typed arrow functions
- Only run this on contextually-typed arrow functions where the context would use the type predicate (e.g.
Array.prototype.filter
).
Possible extensions
There are a few possible extensions of this that I've chosen to keep out of scope for this PR:
- Check explicit type predicates. This PR defers to explicit type predicates, but you could imagine checking them instead. This would bring some type safety to user-defined type guards, which are currently no safer than type assertions.
- Infer assertion functions. If you throw instead of returning a boolean, it should be possible to perform an analogous form of inference.
- Infer type predicates on
this
. It's possible for a boolean-returning method to be athis
predicate. This should be a straightforward extension of this PR. - Infer type predicates on functions that return something other than
boolean
. Ifdates
is(Date|null)[]
, thendates.filter(d => d)
is a fine way to filter thenull
s. This PR makes you writedates.filter(d => !!d)
. I believe this is a limitation of type predicates in general, not of this PR. - Handle multiple returns. It should be possible to infer a type guard for this function, for example, but it would require more bookkeeping:
function isString(x: string | number) {
if (typeof x === 'string') {
return true;
}
return false;
}
The TypeScript team hasn't accepted the linked issue #16069. If you can get it accepted, this PR will have a better chance of being reviewed.
@typescript-bot perf test this faster
Heya @RyanCavanaugh, I've started to run the faster perf test suite on this PR at e2684f128975dac725f4af4f9e6f03d4e765cfbe. You can monitor the build here.
Update: The results are in!
I'm thinking this will break things like...
let isNumberable = (x: string | number) => typeof x === 'number';
isNumberable = x => !isNaN(parseInt(String(x)));
...as a function returning boolean
is not assignable to a type predicate. This example is pretty contrived, but might be relevant for class inheritance (i.e. legal subclasses become illegal because the base class method now gets inferred as a typeguard).
@RyanCavanaugh The results of the perf run you requested are in!
Here they are:
tsc
Comparison Report - baseline..prMetric | baseline | pr | Delta | Best | Worst | p-value |
---|---|---|---|---|---|---|
Angular - node (v18.15.0, x64) | ||||||
Memory used | 295,666k (± 0.01%) | 295,656k (± 0.01%) | ~ | 295,638k | 295,678k | p=0.630 n=6 |
Parse Time | 2.66s (± 0.24%) | 2.66s (± 0.28%) | ~ | 2.65s | 2.67s | p=0.718 n=6 |
Bind Time | 0.83s (± 0.99%) | 0.83s (± 1.18%) | ~ | 0.82s | 0.85s | p=0.383 n=6 |
Check Time | 8.26s (± 0.31%) | 8.26s (± 0.52%) | ~ | 8.21s | 8.32s | p=0.872 n=6 |
Emit Time | 7.11s (± 0.28%) | 7.12s (± 0.35%) | ~ | 7.08s | 7.15s | p=0.418 n=6 |
Total Time | 18.86s (± 0.20%) | 18.87s (± 0.32%) | ~ | 18.82s | 18.96s | p=0.809 n=6 |
Compiler-Unions - node (v18.15.0, x64) | ||||||
Memory used | 193,496k (± 1.54%) | 196,079k (± 1.56%) | ~ | 194,078k | 200,129k | p=0.128 n=6 |
Parse Time | 1.36s (± 1.43%) | 1.36s (± 1.27%) | ~ | 1.33s | 1.38s | p=1.000 n=6 |
Bind Time | 0.72s (± 0.00%) | 0.72s (± 0.00%) | ~ | 0.72s | 0.72s | p=1.000 n=6 |
Check Time | 9.35s (± 0.47%) | 9.71s (± 0.26%) | +0.36s (+ 3.89%) | 9.69s | 9.76s | p=0.005 n=6 |
Emit Time | 2.61s (± 0.47%) | 2.66s (± 0.21%) | +0.04s (+ 1.53%) | 2.65s | 2.66s | p=0.004 n=6 |
Total Time | 14.05s (± 0.44%) | 14.45s (± 0.28%) | +0.40s (+ 2.88%) | 14.41s | 14.51s | p=0.005 n=6 |
Monaco - node (v18.15.0, x64) | ||||||
Memory used | 347,475k (± 0.01%) | 347,539k (± 0.01%) | +64k (+ 0.02%) | 347,509k | 347,566k | p=0.005 n=6 |
Parse Time | 2.48s (± 0.47%) | 2.48s (± 0.66%) | ~ | 2.46s | 2.50s | p=0.568 n=6 |
Bind Time | 0.93s (± 0.44%) | 0.93s (± 0.00%) | ~ | 0.93s | 0.93s | p=0.405 n=6 |
Check Time | 6.92s (± 0.27%) | 7.03s (± 0.37%) | +0.11s (+ 1.54%) | 7.00s | 7.07s | p=0.005 n=6 |
Emit Time | 4.04s (± 0.34%) | 4.06s (± 0.43%) | ~ | 4.03s | 4.07s | p=0.161 n=6 |
Total Time | 14.37s (± 0.12%) | 14.50s (± 0.16%) | +0.13s (+ 0.88%) | 14.47s | 14.53s | p=0.005 n=6 |
TFS - node (v18.15.0, x64) | ||||||
Memory used | 302,868k (± 0.00%) | 302,836k (± 0.01%) | -33k (- 0.01%) | 302,784k | 302,864k | p=0.016 n=6 |
Parse Time | 2.02s (± 0.70%) | 2.01s (± 0.58%) | ~ | 2.00s | 2.03s | p=0.161 n=6 |
Bind Time | 1.00s (± 0.75%) | 1.01s (± 0.97%) | ~ | 1.00s | 1.02s | p=0.300 n=6 |
Check Time | 6.35s (± 0.31%) | 6.34s (± 0.46%) | ~ | 6.29s | 6.37s | p=0.627 n=6 |
Emit Time | 3.60s (± 0.29%) | 3.59s (± 0.43%) | ~ | 3.57s | 3.60s | p=0.797 n=6 |
Total Time | 12.96s (± 0.18%) | 12.94s (± 0.23%) | ~ | 12.91s | 12.98s | p=0.222 n=6 |
material-ui - node (v18.15.0, x64) | ||||||
Memory used | 511,290k (± 0.00%) | 511,308k (± 0.01%) | ~ | 511,283k | 511,383k | p=0.296 n=6 |
Parse Time | 2.66s (± 0.46%) | 2.65s (± 0.75%) | ~ | 2.63s | 2.68s | p=0.557 n=6 |
Bind Time | 1.00s (± 0.75%) | 0.99s (± 0.41%) | ~ | 0.99s | 1.00s | p=0.100 n=6 |
Check Time | 17.30s (± 0.67%) | 17.32s (± 0.38%) | ~ | 17.19s | 17.36s | p=0.467 n=6 |
Emit Time | 0.00s (± 0.00%) | 0.00s (± 0.00%) | ~ | 0.00s | 0.00s | p=1.000 n=6 |
Total Time | 20.95s (± 0.52%) | 20.96s (± 0.32%) | ~ | 20.84s | 21.02s | p=0.377 n=6 |
mui-docs - node (v18.15.0, x64) | ||||||
Memory used | 2,293,373k (± 0.00%) | 2,293,569k (± 0.00%) | +196k (+ 0.01%) | 2,293,468k | 2,293,639k | p=0.005 n=6 |
Parse Time | 11.98s (± 0.92%) | 11.96s (± 0.69%) | ~ | 11.88s | 12.10s | p=0.871 n=6 |
Bind Time | 2.65s (± 0.32%) | 2.64s (± 0.31%) | ~ | 2.63s | 2.65s | p=1.000 n=6 |
Check Time | 102.52s (± 0.59%) | 101.24s (± 0.98%) | -1.29s (- 1.25%) | 99.81s | 102.66s | p=0.045 n=6 |
Emit Time | 0.32s (± 1.28%) | 0.32s (± 1.60%) | ~ | 0.32s | 0.33s | p=0.114 n=6 |
Total Time | 117.47s (± 0.51%) | 116.16s (± 0.89%) | -1.31s (- 1.12%) | 114.71s | 117.73s | p=0.037 n=6 |
self-build-src - node (v18.15.0, x64) | ||||||
Memory used | 2,413,189k (± 0.02%) | 868,471k (± 0.02%) | 🟩-1,544,717k (-64.01%) | 868,289k | 868,849k | p=0.005 n=6 |
Parse Time | 4.90s (± 0.79%) | 5.50s (± 0.80%) | 🔻+0.60s (+12.27%) | 5.45s | 5.57s | p=0.005 n=6 |
Bind Time | 1.86s (± 0.92%) | 2.34s (± 0.36%) | 🔻+0.48s (+25.96%) | 2.34s | 2.36s | p=0.004 n=6 |
Check Time | 33.63s (± 0.43%) | 15.99s (± 0.39%) | 🟩-17.64s (-52.45%) | 15.90s | 16.08s | p=0.005 n=6 |
Emit Time | 2.68s (± 1.18%) | 0.03s (± 0.00%) | 🟩-2.65s (-98.88%) | 0.03s | 0.03s | p=0.003 n=6 |
Total Time | 43.08s (± 0.46%) | 23.88s (± 0.36%) | 🟩-19.20s (-44.57%) | 23.80s | 24.04s | p=0.005 n=6 |
self-compiler - node (v18.15.0, x64) | ||||||
Memory used | 418,962k (± 0.01%) | 414,976k (± 0.01%) | -3,986k (- 0.95%) | 414,938k | 415,064k | p=0.005 n=6 |
Parse Time | 2.82s (± 2.15%) | 2.79s (± 3.64%) | ~ | 2.63s | 2.90s | p=0.256 n=6 |
Bind Time | 1.10s (± 5.27%) | 1.12s (± 5.76%) | ~ | 1.08s | 1.21s | p=0.505 n=6 |
Check Time | 15.19s (± 0.25%) | 15.44s (± 0.28%) | +0.26s (+ 1.69%) | 15.40s | 15.52s | p=0.005 n=6 |
Emit Time | 1.13s (± 0.67%) | 0.02s (±18.82%) | 🟩-1.11s (-98.08%) | 0.02s | 0.03s | p=0.003 n=6 |
Total Time | 20.23s (± 0.19%) | 19.37s (± 0.27%) | 🟩-0.86s (- 4.27%) | 19.31s | 19.45s | p=0.005 n=6 |
vscode - node (v18.15.0, x64) | ||||||
Memory used | 2,847,216k (± 0.00%) | 2,847,890k (± 0.00%) | +674k (+ 0.02%) | 2,847,837k | 2,848,034k | p=0.005 n=6 |
Parse Time | 10.78s (± 0.70%) | 10.74s (± 0.43%) | ~ | 10.70s | 10.83s | p=0.466 n=6 |
Bind Time | 3.43s (± 0.29%) | 3.43s (± 0.62%) | ~ | 3.41s | 3.47s | p=0.402 n=6 |
Check Time | 60.82s (± 0.46%) | 60.87s (± 0.37%) | ~ | 60.50s | 61.15s | p=0.873 n=6 |
Emit Time | 16.90s (± 8.34%) | 16.27s (± 0.42%) | ~ | 16.16s | 16.36s | p=0.126 n=6 |
Total Time | 91.93s (± 1.77%) | 91.31s (± 0.26%) | ~ | 90.93s | 91.58s | p=0.689 n=6 |
webpack - node (v18.15.0, x64) | ||||||
Memory used | 395,919k (± 0.01%) | 396,017k (± 0.01%) | +98k (+ 0.02%) | 395,982k | 396,095k | p=0.005 n=6 |
Parse Time | 3.12s (± 0.33%) | 3.13s (± 0.26%) | ~ | 3.12s | 3.14s | p=0.112 n=6 |
Bind Time | 1.40s (± 0.58%) | 1.40s (± 0.37%) | ~ | 1.39s | 1.40s | p=0.752 n=6 |
Check Time | 14.02s (± 0.35%) | 14.13s (± 0.24%) | +0.10s (+ 0.74%) | 14.08s | 14.18s | p=0.006 n=6 |
Emit Time | 0.00s (± 0.00%) | 0.00s (± 0.00%) | ~ | 0.00s | 0.00s | p=1.000 n=6 |
Total Time | 18.55s (± 0.27%) | 18.66s (± 0.23%) | +0.11s (+ 0.62%) | 18.60s | 18.72s | p=0.005 n=6 |
xstate - node (v18.15.0, x64) | ||||||
Memory used | 513,455k (± 0.01%) | 513,468k (± 0.01%) | ~ | 513,404k | 513,507k | p=0.630 n=6 |
Parse Time | 3.28s (± 0.37%) | 3.28s (± 0.19%) | ~ | 3.27s | 3.29s | p=0.403 n=6 |
Bind Time | 1.54s (± 0.71%) | 1.54s (± 0.49%) | ~ | 1.53s | 1.55s | p=0.604 n=6 |
Check Time | 2.87s (± 1.06%) | 2.85s (± 0.89%) | ~ | 2.82s | 2.89s | p=0.258 n=6 |
Emit Time | 0.08s (± 4.99%) | 0.07s (± 0.00%) | 🟩-0.01s (-14.29%) | 0.07s | 0.07s | p=0.002 n=6 |
Total Time | 7.77s (± 0.45%) | 7.75s (± 0.40%) | ~ | 7.72s | 7.80s | p=0.334 n=6 |
- node (v18.15.0, x64)
- Angular - node (v18.15.0, x64)
- Compiler-Unions - node (v18.15.0, x64)
- Monaco - node (v18.15.0, x64)
- TFS - node (v18.15.0, x64)
- material-ui - node (v18.15.0, x64)
- mui-docs - node (v18.15.0, x64)
- self-build-src - node (v18.15.0, x64)
- self-compiler - node (v18.15.0, x64)
- vscode - node (v18.15.0, x64)
- webpack - node (v18.15.0, x64)
- xstate - node (v18.15.0, x64)
Benchmark | Name | Iterations |
---|---|---|
Current | pr | 6 |
Baseline | baseline | 6 |
Developer Information:
@fatcerberus true, but that can also cut the other way! See the deleted errors file for javascriptThisAssignmentInStaticBlock.ts
.
Ryan pointed out on another issue that inferring a type guard breaks this sort of code as well https://github.com/microsoft/TypeScript/issues/38390#issuecomment-626019466
const a = [1, "foo", 2, "bar"].filter(x => typeof x === "string");
a.push(10); // ok now, error with my PR
I assume the +3.89% check time on compiler-unions is bad? Is there any information on how I can run this locally and see what's going on?
The CI/self-check found a flaw in my criteria for inferring a type predicate. I actually need to be even more strict! I should not infer a type predicate for this function, even though Exclude<unknown, string> = unknown
.
function isShortString(x: unknown) {
return typeof x === 'string' && x.length < 10;
}
declare let str: string;
if (isShortString(str)) {
str; // string
} else {
str; // never
}
This is actually quite interesting. My approach has no way of distinguishing isShortString
from:
function isString(x: unknown) {
return typeof x === 'string';
}
They both have initType=unknown
, trueType=string
and falseType=unknown
. It would be valid to infer a type guard for isShortString
if it were only ever called with unknown
types. But if you call it with something like string
or string | number
then you can see that one is a valid type guard while the other is not.
I need to think a little more about whether this is fixable or if it's a flaw with this whole approach.
Ouch, that's tricky. If there's a &&
present in the condition then you have to make sure there are no additional non-narrowing checks (or that the additional checks didn't narrow something else instead)
@fatcerberus yeah, see updated comment. I need to think about it a little more, but this might be a deal-breaker for this approach.
~~Negated types would be a great help here because the falseType would then be unknown & !string
and the Exclude
-based test would be sufficient.~~
nevermind, I'm dumb, you haven't ruled out all strings. ignore this
I'm feeling somewhat hopeful that this approach can be salvaged. Instead of requiring that:
falseType = Exclude<initType, trueType>
what we actually need is:
falseType = Exclude<T, trueType> \forall T <: initType
i.e. that this relationship holds for all subtypes of the declared type. I'm hoping that in practice this just means I need to check that the relationship holds for T=initType and T=trueType, i.e. add one more check.
Again, this would be much easier if a function could return a different type predicate for the true and false cases!
That fix seemed to work and all tests pass, so we should be back in business. This is going to be a bit slower than the version that @typescript-bot tested earlier but I'm feeling more confident that it's correct.
@typescript-bot test top200 @typescript-bot user test this @typescript-bot run dt
@typescript-bot perf test this faster @typescript-bot pack this
Heya @jakebailey, I've started to run the tarball bundle task on this PR at d0e385e8c781be33d05aaeccc7dcb594d230c296. You can monitor the build here.
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at d0e385e8c781be33d05aaeccc7dcb594d230c296. You can monitor the build here.
Update: The results are in!
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at d0e385e8c781be33d05aaeccc7dcb594d230c296. You can monitor the build here.
Update: The results are in!
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at d0e385e8c781be33d05aaeccc7dcb594d230c296. You can monitor the build here.
Update: The results are in!
Heya @jakebailey, I've started to run the faster perf test suite on this PR at d0e385e8c781be33d05aaeccc7dcb594d230c296. You can monitor the build here.
Update: The results are in!
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json
like so:
{
"devDependencies": {
"typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/159974/artifacts?artifactName=tgz&fileId=6697AFDD16758083B1CD91AF9516D7A3DB7DCC2B13099E445566B57CD913996302&fileName=/typescript-5.5.0-insiders.20240221.tgz"
}
}
and then running npm install
.
There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]"
.;
@jakebailey Here are the results of running the user test suite comparing main
and refs/pull/57465/merge
:
There were infrastructure failures potentially unrelated to your change:
- 1 instance of "Package install failed"
Otherwise...
Something interesting changed - please have a look.
Details
follow-redirects
/mnt/ts_downloads/follow-redirects/tsconfig.json
- [NEW]
error TS2345: Argument of type 'string | String' is not assignable to parameter of type 'string'.
- /mnt/ts_downloads/follow-redirects/node_modules/follow-redirects/index.js(651,66)
puppeteer
packages/browsers/test/src/tsconfig.json
- [MISSING]
error TS2307: Cannot find module '@pptr/testserver' or its corresponding type declarations.
pyright
/mnt/ts_downloads/pyright/build.sh
- [NEW]
error TS2339: Property 'nodeType' does not exist on type 'never'.
- /mnt/ts_downloads/pyright/pyright: ../pyright-internal/src/analyzer/testWalker.ts(78,47)
- /mnt/ts_downloads/pyright/pyright-internal: src/analyzer/testWalker.ts(78,47)
- /mnt/ts_downloads/pyright/vscode-pyright: ../pyright-internal/src/analyzer/testWalker.ts(78,47)
@jakebailey The results of the perf run you requested are in!
Here they are:
tsc
Comparison Report - baseline..prMetric | baseline | pr | Delta | Best | Worst | p-value |
---|---|---|---|---|---|---|
Angular - node (v18.15.0, x64) | ||||||
Memory used | 295,649k (± 0.01%) | 295,682k (± 0.01%) | ~ | 295,641k | 295,730k | p=0.109 n=6 |
Parse Time | 2.66s (± 0.15%) | 2.67s (± 0.21%) | ~ | 2.66s | 2.67s | p=0.282 n=6 |
Bind Time | 0.83s (± 0.49%) | 0.84s (± 0.97%) | ~ | 0.83s | 0.85s | p=0.056 n=6 |
Check Time | 8.26s (± 0.44%) | 8.28s (± 0.31%) | ~ | 8.26s | 8.33s | p=0.328 n=6 |
Emit Time | 7.12s (± 0.09%) | 7.12s (± 0.25%) | ~ | 7.09s | 7.14s | p=0.737 n=6 |
Total Time | 18.87s (± 0.19%) | 18.90s (± 0.19%) | ~ | 18.87s | 18.97s | p=0.291 n=6 |
Compiler-Unions - node (v18.15.0, x64) | ||||||
Memory used | 191,595k (± 0.02%) | 195,558k (± 1.26%) | +3,962k (+ 2.07%) | 194,102k | 200,105k | p=0.005 n=6 |
Parse Time | 1.36s (± 0.80%) | 1.35s (± 2.08%) | ~ | 1.32s | 1.39s | p=0.453 n=6 |
Bind Time | 0.72s (± 0.00%) | 0.72s (± 0.00%) | ~ | 0.72s | 0.72s | p=1.000 n=6 |
Check Time | 9.36s (± 0.46%) | 9.78s (± 0.80%) | 🔻+0.42s (+ 4.45%) | 9.70s | 9.93s | p=0.005 n=6 |
Emit Time | 2.62s (± 0.76%) | 2.65s (± 1.03%) | ~ | 2.62s | 2.69s | p=0.053 n=6 |
Total Time | 14.07s (± 0.28%) | 14.50s (± 0.56%) | +0.44s (+ 3.10%) | 14.44s | 14.66s | p=0.005 n=6 |
Monaco - node (v18.15.0, x64) | ||||||
Memory used | 347,461k (± 0.00%) | 347,527k (± 0.00%) | +67k (+ 0.02%) | 347,514k | 347,543k | p=0.005 n=6 |
Parse Time | 2.48s (± 0.71%) | 2.48s (± 0.49%) | ~ | 2.46s | 2.49s | p=0.280 n=6 |
Bind Time | 0.93s (± 0.44%) | 0.93s (± 0.44%) | ~ | 0.92s | 0.93s | p=1.000 n=6 |
Check Time | 6.96s (± 0.66%) | 7.05s (± 0.59%) | +0.09s (+ 1.32%) | 7.00s | 7.11s | p=0.020 n=6 |
Emit Time | 4.06s (± 0.46%) | 4.06s (± 0.53%) | ~ | 4.04s | 4.10s | p=0.683 n=6 |
Total Time | 14.42s (± 0.40%) | 14.53s (± 0.20%) | +0.11s (+ 0.76%) | 14.48s | 14.56s | p=0.010 n=6 |
TFS - node (v18.15.0, x64) | ||||||
Memory used | 302,868k (± 0.01%) | 302,848k (± 0.00%) | ~ | 302,837k | 302,857k | p=0.065 n=6 |
Parse Time | 2.01s (± 1.33%) | 2.01s (± 0.96%) | ~ | 1.99s | 2.04s | p=0.934 n=6 |
Bind Time | 1.00s (± 1.47%) | 1.01s (± 0.81%) | ~ | 1.00s | 1.02s | p=0.284 n=6 |
Check Time | 6.36s (± 0.30%) | 6.33s (± 0.42%) | ~ | 6.29s | 6.37s | p=0.139 n=6 |
Emit Time | 3.58s (± 0.33%) | 3.60s (± 0.27%) | +0.02s (+ 0.56%) | 3.58s | 3.61s | p=0.023 n=6 |
Total Time | 12.95s (± 0.24%) | 12.95s (± 0.24%) | ~ | 12.90s | 12.99s | p=0.872 n=6 |
material-ui - node (v18.15.0, x64) | ||||||
Memory used | 511,290k (± 0.00%) | 511,320k (± 0.01%) | ~ | 511,275k | 511,465k | p=0.689 n=6 |
Parse Time | 2.65s (± 0.66%) | 2.65s (± 0.55%) | ~ | 2.63s | 2.67s | p=0.510 n=6 |
Bind Time | 1.00s (± 0.55%) | 0.99s (± 1.11%) | ~ | 0.98s | 1.01s | p=0.227 n=6 |
Check Time | 17.30s (± 0.43%) | 17.32s (± 0.37%) | ~ | 17.20s | 17.37s | p=0.629 n=6 |
Emit Time | 0.00s (± 0.00%) | 0.00s (± 0.00%) | ~ | 0.00s | 0.00s | p=1.000 n=6 |
Total Time | 20.94s (± 0.40%) | 20.96s (± 0.31%) | ~ | 20.84s | 21.03s | p=0.378 n=6 |
mui-docs - node (v18.15.0, x64) | ||||||
Memory used | 2,293,496k (± 0.00%) | 2,293,665k (± 0.00%) | +169k (+ 0.01%) | 2,293,606k | 2,293,713k | p=0.005 n=6 |
Parse Time | 11.97s (± 0.86%) | 11.96s (± 1.02%) | ~ | 11.85s | 12.17s | p=0.808 n=6 |
Bind Time | 2.64s (± 0.50%) | 2.64s (± 0.15%) | ~ | 2.63s | 2.64s | p=0.446 n=6 |
Check Time | 102.54s (± 0.61%) | 101.79s (± 0.85%) | ~ | 100.66s | 102.72s | p=0.109 n=6 |
Emit Time | 0.32s (± 1.28%) | 0.32s (± 0.00%) | ~ | 0.32s | 0.32s | p=0.405 n=6 |
Total Time | 117.47s (± 0.50%) | 116.70s (± 0.79%) | ~ | 115.57s | 117.84s | p=0.128 n=6 |
self-build-src - node (v18.15.0, x64) | ||||||
Memory used | 2,413,290k (± 0.02%) | 2,415,358k (± 0.02%) | +2,068k (+ 0.09%) | 2,414,631k | 2,415,917k | p=0.005 n=6 |
Parse Time | 4.92s (± 0.70%) | 4.91s (± 1.03%) | ~ | 4.82s | 4.97s | p=1.000 n=6 |
Bind Time | 1.86s (± 0.74%) | 1.87s (± 0.76%) | ~ | 1.85s | 1.89s | p=0.101 n=6 |
Check Time | 33.62s (± 0.34%) | 33.60s (± 0.45%) | ~ | 33.46s | 33.89s | p=0.471 n=6 |
Emit Time | 2.72s (± 1.73%) | 2.69s (± 1.09%) | ~ | 2.66s | 2.73s | p=0.197 n=6 |
Total Time | 43.14s (± 0.25%) | 43.10s (± 0.39%) | ~ | 42.93s | 43.38s | p=0.689 n=6 |
self-compiler - node (v18.15.0, x64) | ||||||
Memory used | 418,986k (± 0.02%) | 419,551k (± 0.01%) | +565k (+ 0.13%) | 419,479k | 419,588k | p=0.005 n=6 |
Parse Time | 2.85s (± 0.65%) | 2.80s (± 2.36%) | ~ | 2.67s | 2.85s | p=0.059 n=6 |
Bind Time | 1.08s (± 0.51%) | 1.10s (± 5.36%) | ~ | 1.07s | 1.22s | p=0.476 n=6 |
Check Time | 15.16s (± 0.35%) | 15.38s (± 0.25%) | +0.22s (+ 1.44%) | 15.31s | 15.42s | p=0.005 n=6 |
Emit Time | 1.14s (± 1.02%) | 1.13s (± 1.12%) | ~ | 1.12s | 1.15s | p=0.140 n=6 |
Total Time | 20.22s (± 0.19%) | 20.41s (± 0.21%) | +0.19s (+ 0.95%) | 20.34s | 20.45s | p=0.005 n=6 |
vscode - node (v18.15.0, x64) | ||||||
Memory used | 2,847,902k (± 0.00%) | 2,848,336k (± 0.00%) | +434k (+ 0.02%) | 2,848,193k | 2,848,399k | p=0.005 n=6 |
Parse Time | 10.74s (± 0.35%) | 10.76s (± 0.36%) | ~ | 10.71s | 10.81s | p=0.470 n=6 |
Bind Time | 3.43s (± 0.40%) | 3.43s (± 0.24%) | ~ | 3.42s | 3.44s | p=0.933 n=6 |
Check Time | 60.65s (± 0.90%) | 60.85s (± 0.19%) | ~ | 60.63s | 60.97s | p=0.092 n=6 |
Emit Time | 16.31s (± 0.60%) | 16.28s (± 0.59%) | ~ | 16.17s | 16.42s | p=0.521 n=6 |
Total Time | 91.13s (± 0.63%) | 91.32s (± 0.19%) | ~ | 91.06s | 91.54s | p=0.149 n=6 |
webpack - node (v18.15.0, x64) | ||||||
Memory used | 395,939k (± 0.02%) | 396,031k (± 0.01%) | +92k (+ 0.02%) | 395,955k | 396,088k | p=0.020 n=6 |
Parse Time | 3.11s (± 1.07%) | 3.13s (± 0.77%) | ~ | 3.10s | 3.16s | p=0.418 n=6 |
Bind Time | 1.40s (± 0.54%) | 1.40s (± 0.84%) | ~ | 1.38s | 1.41s | p=0.933 n=6 |
Check Time | 14.05s (± 0.40%) | 14.15s (± 0.19%) | +0.10s (+ 0.72%) | 14.12s | 14.20s | p=0.005 n=6 |
Emit Time | 0.00s (± 0.00%) | 0.00s (± 0.00%) | ~ | 0.00s | 0.00s | p=1.000 n=6 |
Total Time | 18.56s (± 0.36%) | 18.68s (± 0.25%) | +0.12s (+ 0.64%) | 18.62s | 18.76s | p=0.010 n=6 |
xstate - node (v18.15.0, x64) | ||||||
Memory used | 513,416k (± 0.00%) | 513,441k (± 0.01%) | ~ | 513,364k | 513,501k | p=0.173 n=6 |
Parse Time | 3.28s (± 0.26%) | 3.28s (± 0.16%) | ~ | 3.27s | 3.28s | p=0.533 n=6 |
Bind Time | 1.54s (± 0.26%) | 1.54s (± 0.00%) | ~ | 1.54s | 1.54s | p=0.405 n=6 |
Check Time | 2.88s (± 1.03%) | 2.84s (± 0.68%) | -0.04s (- 1.45%) | 2.82s | 2.87s | p=0.037 n=6 |
Emit Time | 0.08s (± 4.99%) | 0.07s (± 5.69%) | 🟩-0.01s (-12.24%) | 0.07s | 0.08s | p=0.008 n=6 |
Total Time | 7.78s (± 0.38%) | 7.73s (± 0.24%) | -0.05s (- 0.64%) | 7.72s | 7.77s | p=0.013 n=6 |
- node (v18.15.0, x64)
- Angular - node (v18.15.0, x64)
- Compiler-Unions - node (v18.15.0, x64)
- Monaco - node (v18.15.0, x64)
- TFS - node (v18.15.0, x64)
- material-ui - node (v18.15.0, x64)
- mui-docs - node (v18.15.0, x64)
- self-build-src - node (v18.15.0, x64)
- self-compiler - node (v18.15.0, x64)
- vscode - node (v18.15.0, x64)
- webpack - node (v18.15.0, x64)
- xstate - node (v18.15.0, x64)
Benchmark | Name | Iterations |
---|---|---|
Current | pr | 6 |
Baseline | baseline | 6 |
Developer Information:
Hey @jakebailey, the results of running the DT tests are ready. There were interesting changes:
Branch only errors:
Package: lodash Error:
Error:
/home/vsts/work/1/DefinitelyTyped/types/lodash/lodash-tests.ts
1813:5 error TypeScript@local expected type to be:
string[]
got:
"a"[] @definitelytyped/expect
1814:5 error TypeScript@local expected type to be:
string[]
got:
"a"[] @definitelytyped/expect
✖ 2 problems (2 errors, 0 warnings)
at combineErrorsAndWarnings (/home/vsts/work/1/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/dist/index.js:194:28)
at runTests (/home/vsts/work/1/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/dist/index.js:186:20)
@jakebailey Here are the results of running the top-repos suite comparing main
and refs/pull/57465/merge
:
Something interesting changed - please have a look.
Details
microsoft/vscode
3 of 54 projects failed to build with the old tsc and were ignored
src/tsconfig.json
-
error TS2345: Argument of type 'ChatTreeItem' is not assignable to parameter of type 'IChatResponseViewModel'.
src/tsconfig.tsec.json
-
error TS2345: Argument of type 'ChatTreeItem' is not assignable to parameter of type 'IChatResponseViewModel'.
prisma/prisma
80 of 115 projects failed to build with the old tsc and were ignored
packages/client/tsconfig.build.json
-
error TS2502: 'client' is referenced directly or indirectly in its own type annotation.
-
error TS2456: Type alias 'Client' circularly references itself.
Some errors reported by the bot could be seen as mistakes in the user's code. Why the result of a array, filtered to be an array of numbers, should be typed as anything else than an array of number ?
Building on top of the current approach, but increasing complexity :
A "read" type and a "write" type, for mutable objects.
The read type would represent the latest known type at a point in the control flow. In the case of the array filter, this would mean "this array can hold strings and numbers, but at this point, it only holds numbers". The write type is the type as we know it today.
This is clearly a whole new thing.
Bonus : at the end of the function, if the read type is the same as the write type, and the object is returned, then you can infer a type predicate. This part is out of the scope of your current PR, if I understood correctly.
This idea is great! I seem to have come up with an implementation for fallback that doesn't require compilation support.
declare const notMatched: unique symbol
function isWhat<Input = unknown, T = never>(
match: (input: Input, _: typeof notMatched) => T | typeof notMatched
): (
(x: Input) => x is [T] extends [Input] ? Input & T : never
) {
return ((x: any): boolean => {
try {
return match(x, notMatched) !== notMatched
} catch (e) {
if ([notMatched, void 0, null, TypeError].includes(e as any)) {
return false
}
throw e
}
}) as any
}
const strs0 = [1, '1', true].filter(
// ^? string[]
isWhat((t, _) => typeof t === 'string' ? t : _)
)
const strs1 = [1, '1', true].filter(
// ^? string[]
isWhat(t => {
if (typeof t === 'string') return t
throw void 0
})
)
However, this requires an import. But we can simplify the usage of this function by using webpack or built-in global functions.
If this pull request can be approved, it would be great. However, if it doesn't get approved, I think using this piece of code can still solve the current issue of the awkward usage of is
.
@danvk So if I'm understanding correctly, this won't solve the OP of https://github.com/microsoft/TypeScript/issues/57457 because that just does .filter(value => value)
and you already stated that
We don't infer a type guard here if you check for truthiness, only if you check for non-nullishness
However, I think this might solve https://github.com/microsoft/TypeScript/issues/57457#issuecomment-1959466319?
@danvk So if I'm understanding correctly, this won't solve the OP of #57457 because that just does
.filter(value => value)
and you already stated thatWe don't infer a type guard here if you check for truthiness, only if you check for non-nullishness
@fatcerberus correct. That was a real plot twist when I realized that the code in the original issue shouldn't infer a type predicate after all. Inferring a type guard for that code would require some way for a type predicate to only apply to the true
case, not the false
case.
However, I think this might solve #57457 (comment)?
Yes, it solves that problem because this PR makes type predicates flow. playground
I love this!
Really hope this gets merged!
Notes on the changes from the various test suites that @typescript-bot ran:
- user test suite
-
1 instance of "Package install failed"
-- is this referring to the puppeteer error? Is there any way to debug this? - follow-redirects
- ✅ This is a win. Here’s a minimal repro.
- They have an
isString
function that checks forstring | String
. My PR correctly infers a predicate ofx is string | String
. Their code callsstr1.endsWith(str2)
, where both arestring | String
thanks to the inferred type guard. TypeScript does not allow aString
argument toendsWith
. This works at runtime, though it seems to fine to prohibit it. This isn't detecting a bug but it does make type checking more consistent.
- puppeteer
- 🐟 This was an existing infrastructure issue, see comment below.
- pyright
- ✅ This is a win. Minimal repro.
- There's a
default
case in aswitch
that references a property on an object that's (correctly) narrowed tonever
based on an inferred type guard. This just feels like improved type inference.
-
- DefinitelyTyped
- lodash
- ✅ This is a win.
-
This expectation on
_.filter
breaks because it's assertingstring[]
whereas the type is now (correctly)"a"[]
. This is an increase in precision and a win for my PR. The test could be changed to$ExpectType string[] || "a"[]
to match both before and after this PR.
- lodash
- top-repos
- microsoft/vscode
- ~✅ This is a win. Minimal repro.~
- ⚠️ Neutral See Ryan's comment below.
- There's a type guard that's passed indirectly to
filter
:items.filter(i => isResponseVM(i))
. The wrapper function prevented the predicate from being used, but with this PR TypeScript is not so easily fooled and the return type is narrowed which produces a type error. - The original code is fine, my change pushes it into a known nuisance involving
indexOf
. See my comment for the suggested workaround, which is to add a type annotation.
- prisma/prisma
- 💥 This was a bug in my PR. Minimal repro, though I only get an error in VS Code, not in the playground.
- My PR reports a circularity error whereas
main
does not. This seems to only happen when aconstructor
has areturn
statement. If my code is running on constructor functions, then that's bad news! I'll investigate. -
Update My code was, indeed, running on constructor functions. But only constructor functions that had a single
return
statement that returned a value, i.e. very, very few constructors. The solution is to not run on constructors. Fix in https://github.com/microsoft/TypeScript/pull/57465/commits/ef2d4650473493b3f26a0b06dfa6fba1135b1dfe, see comment for an explanation.
- microsoft/vscode
There was one other error in the public Compiler-Unions PR https://github.com/microsoft/TypeScript/pull/54148 from code like this:
let capturedLeft = isSyntheticReference(left) ? left.expression : left; // Expression
if (!isSimpleCopiableExpression(capturedLeft)) { // detected as type guard
capturedLeft = factory.createTempVariable(hoistVariableDeclaration);
}
let rightExpression = capturedLeft; // SimpleCopiableExpression <: Expression
// ...
rightExpression = factory.createAssignment(thisArg, rightExpression); // Expression
isSimpleCopiableExpression
becomes a type guard with this PR, which lets the type of capturedLeft
narrow. This means that the inferred type of rightExpression
goes from Expression
to something narrower, which results in errors when you try to assign to it.
Since the intent is for rightExpression
to be an Expression
, the fix is to give it an explicit type annotation:
- let rightExpression = capturedLeft;
+ let rightExpression: Expression = capturedLeft;
Here's the fix: https://github.com/microsoft/TypeScript/commit/b54491c649fc3b243e0875ac042ec2138edd6982
1 instance of "Package install failed" -- is this referring to the puppeteer error? Is there any way to debug this?
Ignore this; I just fixed it yesterday (hopefully), after I queued this PR's run.