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

Infer non-narrowing predicates when the contextual signature is a type predicate #60834

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

addresses the concerns raised in the comments in #60741

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 20, 2024
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@Andarist Andarist force-pushed the infer-type-predicates-based-on-context branch from 709b33d to 655602d Compare December 20, 2024 23:22
@Andarist Andarist closed this Dec 20, 2024
@Andarist Andarist reopened this Dec 20, 2024
@Andarist Andarist force-pushed the infer-type-predicates-based-on-context branch from 655602d to bb1dd39 Compare December 20, 2024 23:39
@Andarist Andarist force-pushed the infer-type-predicates-based-on-context branch from bb1dd39 to 332ecc3 Compare December 20, 2024 23:41
@RyanCavanaugh
Copy link
Member

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 3, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started 👀 Results
user test this ✅ Started 👀 Results
run dt ✅ Started 👀 Results
perf test this faster ✅ Started 👀 Results

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the user tests with tsc comparing main and refs/pull/60834/merge:

Something interesting changed - please have a look.

Details

effect

tsconfig.json

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are some more interesting changes from running the user tests suite

Details

fp-ts

tsconfig.eslint.json

@typescript-bot
Copy link
Collaborator

Hey @RyanCavanaugh, the results of running the DT tests are ready.

There were interesting changes:

Branch only errors:

Package: lodash
Error:

Error: 
/mnt/vss/_work/1/DefinitelyTyped/types/lodash/lodash-tests.ts
  2900:5  error  TypeScript@local expected type to be:
  [AbcObject[], AbcObject[]]
got:
  [AbcObject[], never[]]              @definitelytyped/expect
  2950:5  error  TypeScript@local expected type to be:
  [AbcObject[], AbcObject[]]
got:
  [AbcObject[], never[]]              @definitelytyped/expect
  7297:5  error  TypeScript@local expected type to be:
  (...args: number[]) => boolean
got:
  (arg: number) => arg is number  @definitelytyped/expect
  7313:5  error  TypeScript@local expected type to be:
  (...args: number[]) => boolean
got:
  (arg: number) => arg is number  @definitelytyped/expect

✖ 4 problems (4 errors, 0 warnings)

    at combineErrorsAndWarnings (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/dist/index.js:194:28)
    at runTests (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/dist/index.js:186:20)

You can check the log here.

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - node (v18.15.0, x64)
Errors 34 34 ~ ~ ~ p=1.000 n=6
Symbols 62,363 62,363 ~ ~ ~ p=1.000 n=6
Types 50,395 50,395 ~ ~ ~ p=1.000 n=6
Memory used 193,638k (± 0.77%) 193,794k (± 0.73%) ~ 192,998k 196,665k p=0.575 n=6
Parse Time 1.31s (± 1.22%) 1.31s (± 0.84%) ~ 1.29s 1.32s p=0.933 n=6
Bind Time 0.72s 0.72s ~ ~ ~ p=1.000 n=6
Check Time 9.78s (± 0.52%) 9.78s (± 0.26%) ~ 9.76s 9.83s p=1.000 n=6
Emit Time 2.73s (± 0.85%) 2.72s (± 0.38%) ~ 2.71s 2.74s p=1.000 n=6
Total Time 14.53s (± 0.46%) 14.53s (± 0.21%) ~ 14.51s 14.59s p=0.936 n=6
angular-1 - node (v18.15.0, x64)
Errors 37 0 ~ ~ ~ p=1.000 n=6+0
Symbols 947,936 0 ~ ~ ~ p=1.000 n=6+0
Types 410,955 0 ~ ~ ~ p=1.000 n=6+0
Memory used 1,225,791k (± 0.00%) 0k ~ ~ ~ p=1.000 n=6+0
Parse Time 6.69s (± 0.72%) 0s ~ ~ ~ p=1.000 n=6+0
Bind Time 1.88s (± 0.43%) 0s ~ ~ ~ p=1.000 n=6+0
Check Time 32.00s (± 0.22%) 0s ~ ~ ~ p=1.000 n=6+0
Emit Time 15.15s (± 0.25%) 0s ~ ~ ~ p=1.000 n=6+0
Total Time 55.73s (± 0.17%) 0s ~ ~ ~ p=1.000 n=6+0
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6+0
Symbols 2,446,528 0 ~ ~ ~ p=1.000 n=6+0
Types 897,081 0 ~ ~ ~ p=1.000 n=6+0
Memory used 2,311,140k (± 0.00%) 0k ~ ~ ~ p=1.000 n=6+0
Parse Time 8.98s (± 0.54%) 0s ~ ~ ~ p=1.000 n=6+0
Bind Time 2.12s (± 0.64%) 0s ~ ~ ~ p=1.000 n=6+0
Check Time 73.01s (± 0.58%) 0s ~ ~ ~ p=1.000 n=6+0
Emit Time 0.29s (± 1.92%) 0s ~ ~ ~ p=1.000 n=6+0
Total Time 84.40s (± 0.51%) 0s ~ ~ ~ p=1.000 n=6+0
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,226,880 1,226,883 +3 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,745 266,749 +4 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 2,479,167k (±12.00%) 2,479,454k (±11.98%) ~ 2,357,505k 3,086,305k p=0.689 n=6
Parse Time 5.23s (± 1.15%) 5.22s (± 1.83%) ~ 5.13s 5.40s p=0.575 n=6
Bind Time 1.77s (± 0.85%) 1.75s (± 1.05%) ~ 1.72s 1.77s p=0.117 n=6
Check Time 35.24s (± 0.57%) 35.30s (± 0.45%) ~ 35.15s 35.57s p=0.470 n=6
Emit Time 2.96s (± 2.27%) 3.03s (± 4.51%) ~ 2.94s 3.30s p=0.810 n=6
Total Time 45.20s (± 0.41%) 45.31s (± 0.68%) ~ 45.04s 45.71s p=0.630 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,226,880 1,226,883 +3 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,745 266,749 +4 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 2,938,889k (±11.44%) 2,670,499k (±14.00%) ~ 2,428,572k 3,154,186k p=0.066 n=6
Parse Time 6.95s (± 1.31%) 6.88s (± 1.93%) ~ 6.78s 7.10s p=0.261 n=6
Bind Time 2.17s (± 0.63%) 2.20s (± 2.39%) ~ 2.10s 2.25s p=0.077 n=6
Check Time 42.74s (± 0.77%) 42.73s (± 0.60%) ~ 42.40s 43.07s p=0.936 n=6
Emit Time 3.46s (± 0.92%) 3.47s (± 1.64%) ~ 3.38s 3.53s p=0.688 n=6
Total Time 55.32s (± 0.76%) 55.29s (± 0.68%) ~ 54.99s 55.81s p=1.000 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 262,439 262,441 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Types 106,628 106,629 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 440,214k (± 0.01%) 440,305k (± 0.02%) ~ 440,200k 440,383k p=0.066 n=6
Parse Time 3.54s (± 1.17%) 3.54s (± 1.02%) ~ 3.49s 3.59s p=0.747 n=6
Bind Time 1.31s (± 1.12%) 1.32s (± 0.62%) ~ 1.31s 1.33s p=0.284 n=6
Check Time 18.95s (± 0.50%) 18.97s (± 0.47%) ~ 18.84s 19.10s p=0.810 n=6
Emit Time 1.53s (± 1.35%) 1.52s (± 1.30%) ~ 1.49s 1.55s p=0.291 n=6
Total Time 25.32s (± 0.49%) 25.34s (± 0.34%) ~ 25.22s 25.44s p=0.688 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 70 70 ~ ~ ~ p=1.000 n=6
Symbols 226,062 226,062 ~ ~ ~ p=1.000 n=6
Types 94,488 94,488 ~ ~ ~ p=1.000 n=6
Memory used 371,662k (± 0.05%) 371,639k (± 0.01%) ~ 371,599k 371,720k p=0.936 n=6
Parse Time 2.91s (± 0.85%) 2.91s (± 0.64%) ~ 2.89s 2.94s p=0.807 n=6
Bind Time 1.60s (± 0.92%) 1.59s (± 1.85%) ~ 1.56s 1.64s p=0.629 n=6
Check Time 16.48s (± 0.48%) 16.52s (± 0.27%) ~ 16.46s 16.57s p=0.423 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.98s (± 0.40%) 21.03s (± 0.29%) ~ 20.92s 21.08s p=0.377 n=6
vscode - node (v18.15.0, x64)
Errors 3 0 ~ ~ ~ p=1.000 n=6+0
Symbols 3,230,418 0 ~ ~ ~ p=1.000 n=6+0
Types 1,112,109 0 ~ ~ ~ p=1.000 n=6+0
Memory used 3,294,842k (± 0.01%) 0k ~ ~ ~ p=1.000 n=6+0
Parse Time 14.16s (± 0.38%) 0s ~ ~ ~ p=1.000 n=6+0
Bind Time 4.63s (± 3.02%) 0s ~ ~ ~ p=1.000 n=6+0
Check Time 89.85s (± 2.94%) 0s ~ ~ ~ p=1.000 n=6+0
Emit Time 28.03s (± 3.18%) 0s ~ ~ ~ p=1.000 n=6+0
Total Time 136.66s (± 1.58%) 0s ~ ~ ~ p=1.000 n=6+0
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 291,562 291,561 -1 (- 0.00%) ~ ~ p=0.001 n=6
Types 118,971 118,971 ~ ~ ~ p=1.000 n=6
Memory used 445,370k (± 0.02%) 445,482k (± 0.04%) ~ 445,250k 445,674k p=0.230 n=6
Parse Time 4.09s (± 0.93%) 4.11s (± 0.82%) ~ 4.05s 4.15s p=0.377 n=6
Bind Time 1.78s (± 1.30%) 1.79s (± 0.92%) ~ 1.76s 1.80s p=0.871 n=6
Check Time 18.72s (± 0.41%) 18.91s (± 0.74%) +0.19s (+ 1.01%) 18.77s 19.14s p=0.037 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.60s (± 0.34%) 24.81s (± 0.56%) +0.21s (+ 0.84%) 24.62s 24.97s p=0.020 n=6
xstate-main - node (v18.15.0, x64)
Errors 5 5 ~ ~ ~ p=1.000 n=6
Symbols 555,017 555,033 +16 (+ 0.00%) ~ ~ p=0.001 n=6
Types 186,115 186,116 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 494,042k (± 0.01%) 494,045k (± 0.01%) ~ 493,928k 494,123k p=0.936 n=6
Parse Time 4.24s (± 0.60%) 4.23s (± 0.35%) ~ 4.21s 4.25s p=0.360 n=6
Bind Time 1.47s (± 0.82%) 1.45s (± 1.28%) ~ 1.42s 1.47s p=0.438 n=6
Check Time 24.31s (± 0.35%) 24.22s (± 0.40%) ~ 24.05s 24.32s p=0.196 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 30.01s (± 0.25%) 29.91s (± 0.30%) ~ 29.75s 30.01s p=0.064 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

@RyanCavanaugh Here are the results of running the top 400 repos with tsc comparing main and refs/pull/60834/merge:

Something interesting changed - please have a look.

Details

darkreader/darkreader

2 of 5 projects failed to build with the old tsc and were ignored

src/tsconfig.json

mui/material-ui

23 of 85 projects failed to build with the old tsc and were ignored

packages/api-docs-builder-core/tsconfig.json

packages/api-docs-builder/tsconfig.json

@Andarist
Copy link
Contributor Author

Andarist commented Jan 4, 2025

could I get a preview build for this one? it would make it easier to investigate some of the above changes, cc @jakebailey


  1. I'm not sure what happens with Effect. It's rather unlikely this PR is at fault here - it reports that it can't find its typedefs at all
  2. darkreader's repro can be seen here. It's related to the inferred false return type in that filter. It has no effect on tab's narrowing but given it always returns false it now has been inferred as a type predicate that rejects everything (at all times). It boils down to accessing an index signature type that isn't typed as optional (with | undefined in its type). Is it an acceptable break? The workaround is simple, the return type can be annotated as boolean (or the index signature can be "fixed"). It's not unlikely that some .filters in the wild might get affected by this - the same happened with the original Dan's PR.
  3. material-ui's repro can be seen here. This is basically the same case as the darkreader's one.

@jakebailey
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 4, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 4, 2025

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/164470/artifacts?artifactName=tgz&fileId=5E8897D74FF24E8FC340FBB531CB44D4A4CF8253508A926AA310D2E4FFF719B402&fileName=/typescript-5.8.0-insiders.20250104.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]".;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Not started
Development

Successfully merging this pull request may close these issues.

4 participants