TypeScript
TypeScript copied to clipboard
microsoft
Make function properties context-sensitive based on their return statements
@typescript-bot test this @typescript-bot user test this inline @typescript-bot perf test this faster @typescript-bot test top100
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 0a5da2fdc3dd5f5e7e3192c80b30a5b0b77b64ac. You can monitor the build here.
Update: The results are in!
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 0a5da2fdc3dd5f5e7e3192c80b30a5b0b77b64ac. 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 0a5da2fdc3dd5f5e7e3192c80b30a5b0b77b64ac. You can monitor the build here.
Update: The results are in!
Heya @jakebailey, I've started to run the extended test suite on this PR at 0a5da2fdc3dd5f5e7e3192c80b30a5b0b77b64ac. You can monitor the build here.
@jakebailey Here are the results of running the user test suite comparing main and refs/pull/50903/merge:
Everything looks good!
@jakebailey The results of the perf run you requested are in!
Here they are:
Comparison Report - main..50903
| Metric | main | 50903 | Delta | Best | Worst |
|---|---|---|---|---|---|
| Angular - node (v14.15.1, x64) | |||||
| Memory used | 339,117k (± 0.01%) | 339,137k (± 0.01%) | +20k (+ 0.01%) | 339,068k | 339,232k |
| Parse Time | 2.08s (± 1.20%) | 2.06s (± 0.72%) | -0.02s (- 0.92%) | 2.04s | 2.11s |
| Bind Time | 0.80s (± 0.95%) | 0.80s (± 0.83%) | +0.00s (+ 0.50%) | 0.79s | 0.82s |
| Check Time | 5.88s (± 0.90%) | 5.87s (± 0.46%) | -0.01s (- 0.19%) | 5.83s | 5.93s |
| Emit Time | 6.24s (± 0.42%) | 6.27s (± 0.70%) | +0.03s (+ 0.43%) | 6.18s | 6.37s |
| Total Time | 14.99s (± 0.48%) | 14.99s (± 0.43%) | +0.00s (+ 0.03%) | 14.87s | 15.15s |
| Compiler-Unions - node (v14.15.1, x64) | |||||
| Memory used | 190,223k (± 0.01%) | 190,781k (± 0.67%) | +557k (+ 0.29%) | 190,153k | 195,953k |
| Parse Time | 0.85s (± 0.68%) | 0.86s (± 0.67%) | +0.01s (+ 0.82%) | 0.85s | 0.87s |
| Bind Time | 0.49s (± 0.75%) | 0.48s (± 0.77%) | -0.00s (- 0.21%) | 0.48s | 0.49s |
| Check Time | 6.74s (± 0.50%) | 6.71s (± 0.30%) | -0.03s (- 0.45%) | 6.65s | 6.75s |
| Emit Time | 2.41s (± 1.33%) | 2.40s (± 0.76%) | -0.02s (- 0.70%) | 2.37s | 2.44s |
| Total Time | 10.50s (± 0.34%) | 10.45s (± 0.23%) | -0.05s (- 0.46%) | 10.40s | 10.52s |
| Monaco - node (v14.15.1, x64) | |||||
| Memory used | 326,567k (± 0.01%) | 326,560k (± 0.01%) | -7k (- 0.00%) | 326,532k | 326,609k |
| Parse Time | 1.59s (± 0.47%) | 1.58s (± 0.73%) | -0.00s (- 0.19%) | 1.57s | 1.62s |
| Bind Time | 0.73s (± 1.12%) | 0.73s (± 0.68%) | -0.00s (- 0.14%) | 0.72s | 0.74s |
| Check Time | 5.70s (± 0.74%) | 5.70s (± 0.71%) | +0.00s (+ 0.04%) | 5.62s | 5.82s |
| Emit Time | 3.38s (± 0.67%) | 3.39s (± 1.39%) | +0.01s (+ 0.30%) | 3.31s | 3.53s |
| Total Time | 11.40s (± 0.45%) | 11.40s (± 0.68%) | +0.01s (+ 0.04%) | 11.26s | 11.56s |
| TFS - node (v14.15.1, x64) | |||||
| Memory used | 289,670k (± 0.01%) | 289,682k (± 0.01%) | +12k (+ 0.00%) | 289,631k | 289,734k |
| Parse Time | 1.30s (± 0.81%) | 1.29s (± 0.74%) | -0.01s (- 0.38%) | 1.28s | 1.33s |
| Bind Time | 0.79s (± 0.51%) | 0.79s (± 0.38%) | +0.00s (+ 0.13%) | 0.79s | 0.80s |
| Check Time | 5.37s (± 0.58%) | 5.39s (± 0.45%) | +0.01s (+ 0.24%) | 5.33s | 5.43s |
| Emit Time | 3.59s (± 0.94%) | 3.59s (± 0.54%) | +0.00s (+ 0.03%) | 3.54s | 3.63s |
| Total Time | 11.05s (± 0.45%) | 11.06s (± 0.22%) | +0.01s (+ 0.08%) | 11.00s | 11.12s |
| material-ui - node (v14.15.1, x64) | |||||
| Memory used | 436,853k (± 0.00%) | 436,874k (± 0.00%) | +21k (+ 0.00%) | 436,840k | 436,903k |
| Parse Time | 1.89s (± 0.89%) | 1.88s (± 0.64%) | -0.01s (- 0.64%) | 1.85s | 1.90s |
| Bind Time | 0.58s (± 0.81%) | 0.58s (± 1.01%) | -0.00s (- 0.52%) | 0.57s | 0.59s |
| Check Time | 12.95s (± 0.81%) | 12.88s (± 0.86%) | -0.07s (- 0.52%) | 12.71s | 13.21s |
| Emit Time | 0.00s (± 0.00%) | 0.00s (± 0.00%) | 0.00s ( NaN%) | 0.00s | 0.00s |
| Total Time | 15.42s (± 0.73%) | 15.33s (± 0.67%) | -0.08s (- 0.54%) | 15.16s | 15.64s |
| xstate - node (v14.15.1, x64) | |||||
| Memory used | 547,685k (± 0.00%) | 547,662k (± 0.00%) | -23k (- 0.00%) | 547,593k | 547,716k |
| Parse Time | 2.63s (± 0.49%) | 2.64s (± 0.40%) | +0.01s (+ 0.30%) | 2.61s | 2.66s |
| Bind Time | 0.98s (± 1.16%) | 0.99s (± 1.23%) | +0.01s (+ 0.92%) | 0.96s | 1.02s |
| Check Time | 1.53s (± 0.82%) | 1.52s (± 0.53%) | -0.01s (- 0.52%) | 1.50s | 1.53s |
| Emit Time | 0.07s (± 0.00%) | 0.07s (± 0.00%) | 0.00s ( 0.00%) | 0.07s | 0.07s |
| Total Time | 5.21s (± 0.37%) | 5.22s (± 0.31%) | +0.01s (+ 0.25%) | 5.18s | 5.26s |
| Machine Name | ts-ci-ubuntu |
|---|---|
| Platform | linux 4.4.0-210-generic |
| Architecture | x64 |
| Available Memory | 16 GB |
| Available Memory | 15 GB |
| CPUs | 4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz |
- node (v14.15.1, x64)
- Angular - node (v14.15.1, x64)
- Compiler-Unions - node (v14.15.1, x64)
- Monaco - node (v14.15.1, x64)
- TFS - node (v14.15.1, x64)
- material-ui - node (v14.15.1, x64)
- xstate - node (v14.15.1, x64)
| Benchmark | Name | Iterations |
|---|---|---|
| Current | 50903 | 10 |
| Baseline | main | 10 |
Developer Information:
Heya @RyanCavanaugh, I've started to run the tarball bundle task on this PR at 0a5da2fdc3dd5f5e7e3192c80b30a5b0b77b64ac. You can monitor the build here.
Hey @RyanCavanaugh, 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/135059/artifacts?artifactName=tgz&fileId=9B9E7C827F6AF362B57D1037A86D3FCCF508F8EDBB34F60B1E03344AB433109902&fileName=/typescript-4.9.0-insiders.20220922.tgz"
}
}
and then running npm install.
@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/50903/merge:
Everything looks good!
@RyanCavanaugh all cases from the first post in 47599 work OK since 4.7: TS playground. Do you have any additional test cases for this PR based on that issue? Could that issue be already closed as solved based on this prepared playground?
@weswigham @ahejlsberg I remember from prior conversations that this approach is maybe not quite right (due to CFA in function bodies?), but I wasn't able to craft a demonstration of a problem. Thoughts?
Iirc it's because the isContextSensitive logic isn't smart enough to be able to climb through all the branches/conditions in a block body to check if a return statement is, in a roundabout way, actually context sensitive. So it's more that this is incomplete rather than incorrect - but it was already incomplete by virtue of never marking return statements as context sensitive, so... 🤷
Hey @RyanCavanaugh, I've packed this into an installable tgz. You can install it for testing by referencing it in your
package.jsonlike so:{ "devDependencies": { "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/135059/artifacts?artifactName=tgz&fileId=9B9E7C827F6AF362B57D1037A86D3FCCF508F8EDBB34F60B1E03344AB433109902&fileName=/typescript-4.9.0-insiders.20220922.tgz" } }and then running
npm install. The above link has expired. How can I use this PR?
Those builds expire, but it's supposed to go and build an npm package / playground too. I'll rerun it.
@typescript-bot pack this
Heya @jakebailey, I've started to run the tarball bundle task on this PR at 0a5da2fdc3dd5f5e7e3192c80b30a5b0b77b64ac. You can monitor the build here.
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/137058/artifacts?artifactName=tgz&fileId=103B5574FB8B73C99E6F5538EA42227C79B100281BED4F249032FC4A0B473E9202&fileName=/typescript-4.9.0-insiders.20221025.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]".;
function fn<ToInferred>(
callback: (unused: string) => {
key: ToInferred;
keyCallback: (key: ToInferred) => number;
},
) {}
fn(() => ({
key: 5,
keyCallback: (key) => {
return key; // Success, key is number
},
}));
fn((unused) => ({
key: 5,
keyCallback: (key) => {
return key; // Error, key is unknown.
},
}));
The above code is not working. Should I create new issue?
I would say that yes, this might deserve a new issue - it doesn't exactly look like the one that is being fixed here.
Hey - is there any update on this? Would be good to get this merged :pray:
Heya @weswigham, I've started to run the diff-based top-repos suite on this PR at a5ffd7e92876203cb6273beb091ad8658e8fb628. You can monitor the build here.
Update: The results are in!
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at a5ffd7e92876203cb6273beb091ad8658e8fb628. You can monitor the build here.
Update: The results are in!
@weswigham Here are the results of running the top-repos suite comparing main and refs/pull/50903/merge:
Everything looks good!
Hey @weswigham, the results of running the DT tests are ready. Everything looks the same! You can check the log here.