FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

unconstrained type parameters are consistently checked for `{} | null | undefined` in `strictNullChecks` by iisaduan · Pull Request #59059 · microsoft/TypeScript · GitHub

/ TypeScript Public

unconstrained type parameters are consistently checked for {} | null | undefined in strictNullChecks - #59059

Closed
Isabel Duan (iisaduan) wants to merge 8 commits into
microsoft:mainfrom
iisaduan:unconstrainedTypeParameter
ConversationCommits (8)ChecksFiles changed
Closed

unconstrained type parameters are consistently checked for {} | null | undefined in strictNullChecks#59059
Isabel Duan (iisaduan) wants to merge 8 commits into
microsoft:mainfrom
iisaduan:unconstrainedTypeParameter

Conversation

Isabel Duan (iisaduan) commented Jun 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

fixes #50603

This behavior is due to a case that was unchecked in #49119. (Relevant summary of 49119:

  • "An unconstrained type parameter is no longer assignable to {}";
  • an unconstrained type parameter will be unknown, which is equivalent to extends {} | null | undefined;
  • related CFA)

The behavior was previously inconsistent, as shown in the cases below (5.5.3 playground):

function f<T>(a: T, b: T) {
   return a > b; \\ okay
}

function g<T extends {} | undefined | null>(a: T, b: T) {
   return a > b; \\ error!! 
      \\ 'a' is possibly 'null' or 'undefined'.ts(18049)
      \\ 'b' is possibly 'null' or 'undefined'.ts(18049)
}
    function f<T, K extends keyof T>(obj: T, key: K) {
        return obj[key]; \\ okay
    }


    function g<T extends {} | null | undefined , K extends keyof T>(obj: T, key: K) {
        return obj[key];
               ~~~
!!! error TS18049: 'obj' is possibly 'null' or 'undefined'.
               ~~~~~~~~
!!! error TS2536: Type 'K' cannot be used to index type '{}'.
    }

Implementation:
This PR

  • changes the way TypeFacts returns for unconstrained parameters. Instead of finding that unconstrained type parameters have UnknownFacts, we allow unconstrained type parameters to have all facts. Since UnknownFacts = AllFacts & ~UndefinedOrNull, this change means that unconstrained type parameters now have the possibility to be treated as undefined | null even when not explicitly unioned.
  • defers more cases of checking indexTypes, and defers the related checkNonNullExpression

Considerations left
-- in function f<T, U extends T>, U is not an unconstrained parameter
-- (CFA needed?) to check previously ok cases (see tests/baselines/reference/isomorphicMappedTypeInference.errors.txt)
-- const b = "foo" in obj[key]; is no longer okay when we have parameter (obj: T) (see keyofAndIndexedAccessErrors.txt)

TypeScript Bot (typescript-bot) added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Jun 27, 2024

Isabel Duan (iisaduan) commented Jun 27, 2024

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) pack this

TypeScript Bot (typescript-bot) commented Jun 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Command Status Results
pack this ✅ Started ✅ Results

Isabel Duan (iisaduan) commented Jun 27, 2024
View reviewed changes
Comment thread tests/baselines/reference/voidUndefinedReduction.types
> : ^^^^^^^
>value : (T & {}) | null
> : ^^^^^^^^^^^^^^^
>value : (T & ({} | null)) | null

Isabel Duan (iisaduan) Jun 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this seems to be the only weirder/"I may need to fix that" break that happens in the test suite. This test pre-dates #49119, which adds "An unconstrained type parameter is no longer assignable to {}" and was the cause of the original regression that this PR fixes, so the code in this test doesn't do what the original author probably intended.

TypeScript Bot (typescript-bot) commented Jun 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey Isabel Duan (@iisaduan), 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/162485/artifacts?artifactName=tgz&fileId=AF28E66001C4F2A54E0E06BA6353BC28CB019DB36741936508D6896D4A5C5F1102&fileName=/typescript-5.6.0-insiders.20240627.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/pr-build@5.6.0-pr-59059-2".;

Isabel Duan (iisaduan) commented Jun 28, 2024

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test it

TypeScript Bot (typescript-bot) commented Jun 28, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 (typescript-bot) commented Jun 28, 2024

Copy link
Copy Markdown
Contributor

Hey Isabel Duan (@iisaduan), the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

TypeScript Bot (typescript-bot) commented Jun 28, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan) Here are the results of running the user tests with tsc comparing main and refs/pull/59059/merge:

Something interesting changed - please have a look.

Details

fp-ts

tsconfig.json

tsconfig.build-es6.json

examples/tsconfig.json

dtslint/ts3.5/tsconfig.json

puppeteer

packages/puppeteer-core/tsconfig.json

webpack

tsconfig.types.json

TypeScript Bot (typescript-bot) commented Jun 28, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan)
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 30 30 ~ ~ ~ p=1.000 n=6
Symbols 62,153 62,153 ~ ~ ~ p=1.000 n=6
Types 50,242 50,242 ~ ~ ~ p=1.000 n=6
Memory used 193,959k (± 1.03%) 192,260k (± 0.10%) ~ 192,122k 192,522k p=0.423 n=6
Parse Time 1.31s (± 0.75%) 1.31s (± 0.31%) ~ 1.31s 1.32s p=0.673 n=6
Bind Time 0.71s (± 0.57%) 0.71s ~ ~ ~ p=0.405 n=6
Check Time 9.43s (± 0.31%) 9.43s (± 0.25%) ~ 9.39s 9.45s p=1.000 n=6
Emit Time 2.74s (± 0.52%) 2.75s (± 1.52%) ~ 2.67s 2.78s p=0.195 n=6
Total Time 14.19s (± 0.20%) 14.19s (± 0.44%) ~ 14.07s 14.24s p=0.376 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 17 🔻+12 (+240.00%) ~ ~ p=0.001 n=6
Symbols 944,114 944,114 ~ ~ ~ p=1.000 n=6
Types 407,050 407,067 +17 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 1,218,360k (± 0.01%) 1,218,355k (± 0.00%) ~ 1,218,279k 1,218,435k p=0.810 n=6
Parse Time 6.65s (± 0.56%) 6.66s (± 0.61%) ~ 6.59s 6.70s p=0.418 n=6
Bind Time 1.86s (± 0.56%) 1.86s (± 0.55%) ~ 1.85s 1.87s p=0.931 n=6
Check Time 30.61s (± 0.29%) 30.75s (± 0.27%) +0.14s (+ 0.47%) 30.65s 30.88s p=0.020 n=6
Emit Time 13.62s (± 0.14%) 13.57s (± 0.22%) -0.05s (- 0.39%) 13.52s 13.61s p=0.010 n=6
Total Time 52.75s (± 0.20%) 52.85s (± 0.21%) ~ 52.71s 52.97s p=0.228 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 32 🔻+32 (+ ∞%) ~ ~ p=0.001 n=6
Symbols 2,133,050 2,133,050 ~ ~ ~ p=1.000 n=6
Types 926,444 926,496 +52 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,115,442k (± 0.00%) 2,115,498k (± 0.01%) ~ 2,115,339k 2,115,668k p=0.298 n=6
Parse Time 6.65s (± 0.26%) 6.66s (± 0.12%) ~ 6.65s 6.67s p=0.932 n=6
Bind Time 2.34s (± 0.80%) 2.35s (± 1.07%) ~ 2.32s 2.38s p=0.622 n=6
Check Time 70.85s (± 0.28%) 70.29s (± 1.24%) ~ 68.78s 71.03s p=0.336 n=6
Emit Time 0.14s (± 3.77%) 0.14s (± 3.60%) ~ 0.14s 0.15s p=0.069 n=6
Total Time 79.97s (± 0.23%) 79.44s (± 1.09%) ~ 77.93s 80.18s p=0.261 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 4,466 🔻+4,466 (+ ∞%) ~ ~ p=0.001 n=6
Symbols 1,224,331 1,419,174 🔻+194,843 (+15.91%) ~ ~ p=0.001 n=6
Types 261,007 259,544 -1,463 (- 0.56%) ~ ~ p=0.001 n=6
Memory used 2,338,206k (± 0.03%) 3,010,661k (± 0.79%) 🔻+672,455k (+28.76%) 2,994,872k 3,042,334k p=0.005 n=6
Parse Time 5.03s (± 0.87%) 7.47s (± 0.81%) 🔻+2.45s (+48.66%) 7.37s 7.54s p=0.005 n=6
Bind Time 1.89s (± 0.85%) 2.77s (± 1.34%) 🔻+0.88s (+46.34%) 2.74s 2.84s p=0.004 n=6
Check Time 34.19s (± 0.39%) 34.24s (± 0.40%) ~ 34.04s 34.41s p=0.470 n=6
Emit Time 2.72s (± 2.60%) 2.10s (± 1.20%) 🟩-0.63s (-22.94%) 2.07s 2.13s p=0.005 n=6
Total Time 43.85s (± 0.49%) 46.60s (± 0.32%) 🔻+2.75s (+ 6.27%) 46.36s 46.80s p=0.005 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 4,466 🔻+4,466 (+ ∞%) ~ ~ p=0.001 n=6
Symbols 1,224,331 1,419,174 🔻+194,843 (+15.91%) ~ ~ p=0.001 n=6
Types 261,007 259,544 -1,463 (- 0.56%) ~ ~ p=0.001 n=6
Memory used 2,423,875k (± 1.02%) 3,084,651k (± 0.61%) 🔻+660,776k (+27.26%) 3,076,216k 3,123,002k p=0.005 n=6
Parse Time 7.79s (± 0.19%) 11.50s (± 0.35%) 🔻+3.72s (+47.76%) 11.46s 11.57s p=0.005 n=6
Bind Time 2.51s (± 1.13%) 3.76s (± 0.93%) 🔻+1.25s (+49.60%) 3.70s 3.80s p=0.005 n=6
Check Time 50.14s (± 0.40%) 50.47s (± 0.28%) +0.33s (+ 0.66%) 50.25s 50.63s p=0.013 n=6
Emit Time 3.89s (± 2.46%) 3.19s (± 1.09%) 🟩-0.71s (-18.14%) 3.14s 3.23s p=0.005 n=6
Total Time 64.32s (± 0.37%) 68.92s (± 0.24%) 🔻+4.59s (+ 7.14%) 68.68s 69.18s p=0.005 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,166 258,167 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 104,689 104,693 +4 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 427,353k (± 0.01%) 427,334k (± 0.00%) ~ 427,321k 427,345k p=0.378 n=6
Parse Time 3.30s (± 0.75%) 3.27s (± 0.46%) ~ 3.26s 3.30s p=0.061 n=6
Bind Time 1.32s (± 1.25%) 1.31s (± 1.12%) ~ 1.29s 1.33s p=0.512 n=6
Check Time 17.83s (± 0.36%) 17.82s (± 0.28%) ~ 17.74s 17.86s p=0.809 n=6
Emit Time 1.40s (± 1.88%) 1.41s (± 0.73%) ~ 1.40s 1.42s p=0.285 n=6
Total Time 23.85s (± 0.34%) 23.81s (± 0.23%) ~ 23.76s 23.87s p=0.470 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,738 +4 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 369,465k (± 0.03%) 369,601k (± 0.03%) ~ 369,402k 369,768k p=0.093 n=6
Parse Time 2.77s (± 0.71%) 2.77s (± 0.53%) ~ 2.75s 2.79s p=0.742 n=6
Bind Time 1.58s (± 0.69%) 1.57s (± 0.52%) ~ 1.57s 1.59s p=0.177 n=6
Check Time 15.41s (± 0.31%) 15.44s (± 0.42%) ~ 15.36s 15.54s p=0.419 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 19.76s (± 0.31%) 19.79s (± 0.36%) ~ 19.71s 19.89s p=0.418 n=6
vscode - node (v18.15.0, x64)
Errors 0 5 🔻+5 (+ ∞%) ~ ~ p=0.001 n=6
Symbols 2,878,631 2,878,631 ~ ~ ~ p=1.000 n=6
Types 975,177 975,218 +41 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 3,042,131k (± 0.00%) 3,042,190k (± 0.00%) +59k (+ 0.00%) 3,042,152k 3,042,249k p=0.045 n=6
Parse Time 13.58s (± 0.32%) 13.58s (± 0.15%) ~ 13.54s 13.60s p=0.686 n=6
Bind Time 4.20s (± 0.13%) 4.20s (± 0.10%) ~ 4.19s 4.20s p=0.282 n=6
Check Time 74.09s (± 2.27%) 73.40s (± 0.24%) ~ 73.19s 73.70s p=0.810 n=6
Emit Time 23.42s (± 6.69%) 24.12s (± 1.09%) ~ 23.73s 24.39s p=0.423 n=6
Total Time 115.29s (± 0.40%) 115.29s (± 0.19%) ~ 115.07s 115.55s p=0.873 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,117 267,117 ~ ~ ~ p=1.000 n=6
Types 108,775 108,775 ~ ~ ~ p=1.000 n=6
Memory used 411,548k (± 0.01%) 411,563k (± 0.01%) ~ 411,505k 411,637k p=0.873 n=6
Parse Time 3.83s (± 0.61%) 3.85s (± 0.61%) ~ 3.82s 3.88s p=0.331 n=6
Bind Time 1.70s (± 0.32%) 1.70s (± 0.64%) ~ 1.69s 1.72s p=0.476 n=6
Check Time 16.77s (± 0.16%) 16.75s (± 0.53%) ~ 16.59s 16.85s p=0.809 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.30s (± 0.18%) 22.30s (± 0.46%) ~ 22.12s 22.39s p=0.517 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 2 🔻+2 (+ ∞%) ~ ~ p=0.001 n=6
Symbols 525,251 525,251 ~ ~ ~ p=1.000 n=6
Types 178,574 178,576 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 462,783k (± 0.06%) 462,703k (± 0.06%) ~ 462,380k 462,995k p=0.689 n=6
Parse Time 3.17s (± 0.65%) 3.17s (± 0.75%) ~ 3.13s 3.20s p=0.570 n=6
Bind Time 1.17s (± 0.35%) 1.16s (± 1.05%) ~ 1.14s 1.17s p=1.000 n=6
Check Time 17.90s (± 0.36%) 17.95s (± 0.24%) ~ 17.90s 18.01s p=0.199 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.24s (± 0.34%) 22.29s (± 0.26%) ~ 22.21s 22.38s p=0.228 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 (typescript-bot) commented Jun 28, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan) Here are the results of running the top 400 repos with tsc comparing main and refs/pull/59059/merge:

Something interesting changed - please have a look.

Details

ag-grid/ag-grid

31 of 110 projects failed to build with the old tsc and were ignored

enterprise-modules/viewport-row-model/tsconfig.watch.json

enterprise-modules/viewport-row-model/tsconfig.types.watch.json

enterprise-modules/status-bar/tsconfig.watch.json

enterprise-modules/status-bar/tsconfig.types.watch.json

enterprise-modules/sparklines/tsconfig.watch.json

enterprise-modules/sparklines/tsconfig.types.watch.json

enterprise-modules/side-bar/tsconfig.watch.json

enterprise-modules/side-bar/tsconfig.types.watch.json

enterprise-modules/set-filter/tsconfig.watch.json

enterprise-modules/set-filter/tsconfig.types.watch.json

enterprise-modules/server-side-row-model/tsconfig.watch.json

enterprise-modules/server-side-row-model/tsconfig.types.watch.json

enterprise-modules/row-grouping/tsconfig.watch.json

enterprise-modules/row-grouping/tsconfig.types.watch.json

enterprise-modules/rich-select/tsconfig.watch.json

enterprise-modules/rich-select/tsconfig.types.watch.json

enterprise-modules/range-selection/tsconfig.watch.json

enterprise-modules/range-selection/tsconfig.types.watch.json

enterprise-modules/multi-filter/tsconfig.watch.json

enterprise-modules/multi-filter/tsconfig.types.watch.json

enterprise-modules/master-detail/tsconfig.watch.json

enterprise-modules/master-detail/tsconfig.types.watch.json

enterprise-modules/filter-tool-panel/tsconfig.watch.json

enterprise-modules/filter-tool-panel/tsconfig.types.watch.json

enterprise-modules/core/tsconfig.watch.json

enterprise-modules/core/tsconfig.types.watch.json

enterprise-modules/advanced-filter/tsconfig.watch.json

enterprise-modules/advanced-filter/tsconfig.types.watch.json

community-modules/vue3/tsconfig.watch.json

community-modules/vue3/tsconfig.types.watch.json

community-modules/infinite-row-model/tsconfig.watch.json

community-modules/infinite-row-model/tsconfig.types.watch.json

community-modules/csv-export/tsconfig.watch.json

community-modules/csv-export/tsconfig.types.watch.json

community-modules/core/tsconfig.watch.json

community-modules/core/tsconfig.types.watch.json

community-modules/client-side-row-model/tsconfig.watch.json

community-modules/client-side-row-model/tsconfig.types.watch.json

compiler-explorer/compiler-explorer

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

tsconfig.tests.json

tsconfig.json

static/tsconfig.json

desktop/desktop

1 of 4 projects failed to build with the old tsc and were ignored

tsconfig.json

discordjs/discord.js

39 of 62 projects failed to build with the old tsc and were ignored

packages/collection/tsconfig.json

packages/collection/tsconfig.eslint.json

packages/collection/tsconfig.docs.json

gcanti/fp-ts

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

tsconfig.build-es6.json

Infisical/infisical

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

backend/tsconfig.json

microsoft/vscode

4 of 54 projects failed to build with the old tsc and were ignored

src/tsconfig.tsec.json

src/tsconfig.json

motion-canvas/motion-canvas

13 of 18 projects failed to build with the old tsc and were ignored

packages/core/tsconfig.json

packages/core/tsconfig.build.json

TypeScript Bot (typescript-bot) commented Jun 28, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan) Here are some more interesting changes from running the top 400 repos suite

Details

mui/material-ui

22 of 79 projects failed to build with the old tsc and were ignored

docs/tsconfig.json

piotrwitek/react-redux-typescript-guide

playground/tsconfig.test.json

playground/tsconfig.json

portainer/portainer

tsconfig.json

react-hook-form/react-hook-form

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

tsconfig.json

Redocly/redoc

tsconfig.lib.json

tsconfig.json

sequelize/sequelize

13 of 16 projects failed to build with the old tsc and were ignored

packages/utils/tsconfig.json

shoelace-style/shoelace

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

tsconfig.prod.json

supabase/supabase

31 of 37 projects failed to build with the old tsc and were ignored

apps/studio/tsconfig.json

tusen-ai/naive-ui

3 of 6 projects failed to build with the old tsc and were ignored

src/tsconfig.demo.json

usablica/intro.js

1 of 3 projects failed to build with the old tsc and were ignored

tsconfig.release.json

tsconfig.json

vuejs/vue

7 of 8 projects failed to build with the old tsc and were ignored

tsconfig.json

Isabel Duan (iisaduan) commented Jul 1, 2024

Copy link
Copy Markdown
Member Author

With this PR, this lint check is necessary, as it was what was causing the self-check to fail earlier https://github.com/microsoft/TypeScript/actions/runs/9751008069/job/26911678446

Daniel Rosenwasser (DanielRosenwasser) commented Jul 2, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

So one of the things that no longer works is the following:

function f<T, K extends keyof T>(obj: T, key: K) {
    return obj[key];
//         ~~~
// error! 'obj' is possibly 'null' or 'undefined'.
}

One of the reasons we've typically said that this is okay is because keyof undefined or keyof null is never; so even it's not really unsafe because it's supposed to be impossible to call f in a reasonable way (with some caveats).

f(undefined, "hello");
//           ~~~~~~~
// error! Argument of type 'string' is not assignable to parameter of type 'never'.

I know Anders Hejlsberg (@ahejlsberg) was considering changing this for other reasons around the time that #56652 was out. Maybe he has some thoughts here or can give some pointers.

I think we can fix #50603 well enough without disrupting the behaviors in place, but I'd have to take a deeper look to help out there.

Isabel Duan (iisaduan) commented Jul 2, 2024

Copy link
Copy Markdown
Member Author

I think we can fix #50603 well enough without disrupting the behaviors in place, but I'd have to take a deeper look to help out there.

Thanks for the insight! I think so too, I will fix that. Those test cases looked alright at first, but after you brought the above up, I was reminded that the following is similarish and fine

function f<T>(obj: T) {
    for (const key in obj) {
        obj[key]; // no errors
    }
}

Isabel Duan (iisaduan) commented Jul 13, 2024

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test it

TypeScript Bot (typescript-bot) commented Jul 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 (typescript-bot) commented Jul 13, 2024

Copy link
Copy Markdown
Contributor

Hey Isabel Duan (@iisaduan), the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

TypeScript Bot (typescript-bot) commented Jul 13, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan) Here are the results of running the user tests with tsc comparing main and refs/pull/59059/merge:

Something interesting changed - please have a look.

Details

fp-ts

tsconfig.json

  • [NEW] error TS2536: Type 'Extract<keyof NonNullable<A>, string>' cannot be used to index type 'F'.

tsconfig.build-es6.json

  • [NEW] error TS2536: Type 'Extract<keyof NonNullable<A>, string>' cannot be used to index type 'F'.

webpack

tsconfig.types.json

TypeScript Bot (typescript-bot) commented Jul 13, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan)
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 30 30 ~ ~ ~ p=1.000 n=6
Symbols 62,153 62,153 ~ ~ ~ p=1.000 n=6
Types 50,242 50,242 ~ ~ ~ p=1.000 n=6
Memory used 192,290k (± 0.08%) 192,257k (± 0.03%) ~ 192,195k 192,315k p=1.000 n=6
Parse Time 1.31s (± 0.80%) 1.29s (± 0.40%) ~ 1.29s 1.30s p=0.051 n=6
Bind Time 0.71s 0.71s ~ ~ ~ p=1.000 n=6
Check Time 9.47s (± 0.42%) 9.48s (± 0.42%) ~ 9.43s 9.55s p=1.000 n=6
Emit Time 2.75s (± 0.37%) 2.77s (± 1.80%) ~ 2.68s 2.82s p=0.192 n=6
Total Time 14.23s (± 0.36%) 14.25s (± 0.56%) ~ 14.14s 14.38s p=1.000 n=6
angular-1 - node (v18.15.0, x64)
Errors 5 15 🔻+10 (+200.00%) ~ ~ p=0.001 n=6
Symbols 944,114 944,118 +4 (+ 0.00%) ~ ~ p=0.001 n=6
Types 407,050 407,072 +22 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 1,218,408k (± 0.00%) 1,218,631k (± 0.00%) +223k (+ 0.02%) 1,218,580k 1,218,670k p=0.005 n=6
Parse Time 6.70s (± 0.74%) 6.71s (± 0.52%) ~ 6.68s 6.78s p=0.871 n=6
Bind Time 1.87s (± 0.28%) 1.87s (± 0.53%) ~ 1.86s 1.88s p=0.931 n=6
Check Time 30.75s (± 0.45%) 30.89s (± 0.28%) ~ 30.75s 31.01s p=0.173 n=6
Emit Time 13.60s (± 0.36%) 13.66s (± 0.23%) +0.06s (+ 0.47%) 13.64s 13.72s p=0.005 n=6
Total Time 52.91s (± 0.21%) 53.13s (± 0.26%) +0.22s (+ 0.41%) 52.98s 53.38s p=0.013 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 16 🔻+16 (+ ∞%) ~ ~ p=0.001 n=6
Symbols 2,148,730 2,149,120 +390 (+ 0.02%) ~ ~ p=0.001 n=6
Types 929,320 929,525 +205 (+ 0.02%) ~ ~ p=0.001 n=6
Memory used 2,116,359k (± 0.00%) 2,116,596k (± 0.00%) +237k (+ 0.01%) 2,116,529k 2,116,649k p=0.005 n=6
Parse Time 6.59s (± 0.16%) 6.59s (± 0.21%) ~ 6.57s 6.61s p=0.738 n=6
Bind Time 2.32s (± 0.22%) 2.32s (± 0.22%) ~ 2.32s 2.33s p=1.000 n=6
Check Time 70.64s (± 0.54%) 70.81s (± 0.24%) ~ 70.65s 71.10s p=0.521 n=6
Emit Time 0.14s 0.14s (± 2.95%) ~ 0.13s 0.14s p=0.405 n=6
Total Time 79.70s (± 0.48%) 79.86s (± 0.22%) ~ 79.69s 80.16s p=0.521 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,948 1,225,949 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 261,589 261,606 +17 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,339,160k (± 0.02%) 2,339,276k (± 0.03%) ~ 2,337,783k 2,339,885k p=0.471 n=6
Parse Time 6.07s (± 1.06%) 6.06s (± 0.87%) ~ 5.98s 6.13s p=0.470 n=6
Bind Time 2.25s (± 1.03%) 2.25s (± 0.91%) ~ 2.22s 2.28s p=0.630 n=6
Check Time 40.30s (± 0.38%) 40.14s (± 0.44%) ~ 39.95s 40.40s p=0.128 n=6
Emit Time 3.80s (± 1.23%) 3.85s (± 1.72%) ~ 3.79s 3.95s p=0.377 n=6
Total Time 52.42s (± 0.39%) 52.32s (± 0.27%) ~ 52.14s 52.49s p=0.378 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,225,948 1,225,949 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 261,589 261,606 +17 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 2,412,787k (± 0.03%) 2,413,264k (± 0.04%) ~ 2,412,033k 2,414,312k p=0.378 n=6
Parse Time 6.36s (± 0.44%) 6.34s (± 1.08%) ~ 6.26s 6.43s p=0.688 n=6
Bind Time 2.02s (± 0.97%) 2.03s (± 0.74%) ~ 2.01s 2.05s p=0.256 n=6
Check Time 40.69s (± 0.23%) 40.66s (± 0.20%) ~ 40.54s 40.77s p=0.630 n=6
Emit Time 3.94s (± 1.39%) 3.93s (± 0.78%) ~ 3.89s 3.97s p=1.000 n=6
Total Time 53.00s (± 0.23%) 52.97s (± 0.22%) ~ 52.75s 53.07s p=0.810 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 258,230 258,231 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 104,789 104,798 +9 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 427,070k (± 0.02%) 427,001k (± 0.02%) ~ 426,904k 427,122k p=0.230 n=6
Parse Time 4.10s (± 0.39%) 4.10s (± 0.52%) ~ 4.07s 4.13s p=0.870 n=6
Bind Time 1.64s (± 1.08%) 1.63s (± 0.55%) ~ 1.62s 1.64s p=0.557 n=6
Check Time 22.10s (± 0.27%) 22.01s (± 0.37%) ~ 21.91s 22.09s p=0.173 n=6
Emit Time 2.05s (± 1.01%) 2.01s (± 1.46%) -0.03s (- 1.63%) 1.96s 2.04s p=0.042 n=6
Total Time 29.88s (± 0.09%) 29.75s (± 0.37%) -0.13s (- 0.42%) 29.59s 29.89s p=0.031 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 35 35 ~ ~ ~ p=1.000 n=6
Symbols 224,565 224,565 ~ ~ ~ p=1.000 n=6
Types 93,734 93,742 +8 (+ 0.01%) ~ ~ p=0.001 n=6
Memory used 369,590k (± 0.03%) 369,580k (± 0.03%) ~ 369,495k 369,791k p=0.689 n=6
Parse Time 3.44s (± 1.17%) 3.44s (± 0.97%) ~ 3.40s 3.50s p=0.809 n=6
Bind Time 1.94s (± 1.43%) 1.93s ~ ~ ~ p=1.000 n=6
Check Time 19.14s (± 0.38%) 19.20s (± 0.32%) ~ 19.11s 19.27s p=0.127 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 24.52s (± 0.34%) 24.58s (± 0.25%) ~ 24.49s 24.64s p=0.297 n=6
vscode - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,891,160 2,891,166 +6 (+ 0.00%) ~ ~ p=0.001 n=6
Types 978,184 978,210 +26 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 3,054,933k (± 0.00%) 3,055,597k (± 0.00%) +664k (+ 0.02%) 3,055,517k 3,055,720k p=0.005 n=6
Parse Time 11.47s (± 0.30%) 11.47s (± 0.47%) ~ 11.39s 11.54s p=0.809 n=6
Bind Time 3.53s (± 0.29%) 3.54s (± 0.21%) ~ 3.53s 3.55s p=0.063 n=6
Check Time 65.46s (± 2.64%) 64.67s (± 2.35%) ~ 63.85s 67.75s p=0.936 n=6
Emit Time 18.77s (±10.20%) 19.89s (± 7.10%) ~ 17.02s 20.75s p=0.810 n=6
Total Time 99.24s (± 0.25%) 99.56s (± 0.34%) ~ 99.25s 100.19s p=0.230 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 267,239 267,239 ~ ~ ~ p=1.000 n=6
Types 108,815 108,816 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 411,814k (± 0.02%) 411,929k (± 0.01%) +116k (+ 0.03%) 411,887k 411,993k p=0.020 n=6
Parse Time 3.81s (± 0.32%) 3.83s (± 0.59%) ~ 3.81s 3.87s p=0.210 n=6
Bind Time 1.69s (± 0.69%) 1.68s (± 0.84%) ~ 1.66s 1.70s p=0.357 n=6
Check Time 16.69s (± 0.44%) 16.82s (± 0.21%) +0.13s (+ 0.76%) 16.77s 16.88s p=0.005 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 22.19s (± 0.30%) 22.33s (± 0.10%) +0.13s (+ 0.60%) 22.30s 22.36s p=0.005 n=6
xstate-main - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 510,479 510,479 ~ ~ ~ p=1.000 n=6
Types 161,600 161,600 ~ ~ ~ p=1.000 n=6
Memory used 447,881k (± 0.08%) 447,807k (± 0.07%) ~ 447,580k 448,337k p=0.575 n=6
Parse Time 3.92s (± 0.50%) 3.90s (± 0.45%) ~ 3.87s 3.92s p=0.122 n=6
Bind Time 1.46s (± 1.76%) 1.47s (± 1.32%) ~ 1.45s 1.50s p=0.809 n=6
Check Time 21.04s (± 0.38%) 21.08s (± 0.38%) ~ 21.00s 21.22s p=0.260 n=6
Emit Time 0.00s (±244.70%) 0.00s ~ ~ ~ p=0.405 n=6
Total Time 26.41s (± 0.24%) 26.45s (± 0.25%) ~ 26.37s 26.56s p=0.628 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 (typescript-bot) commented Jul 13, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan) Here are the results of running the top 400 repos with tsc comparing main and refs/pull/59059/merge:

Something interesting changed - please have a look.

Details

ag-grid/ag-grid

31 of 110 projects failed to build with the old tsc and were ignored

enterprise-modules/viewport-row-model/tsconfig.watch.json

enterprise-modules/viewport-row-model/tsconfig.types.watch.json

enterprise-modules/status-bar/tsconfig.watch.json

enterprise-modules/status-bar/tsconfig.types.watch.json

enterprise-modules/sparklines/tsconfig.watch.json

enterprise-modules/sparklines/tsconfig.types.watch.json

enterprise-modules/side-bar/tsconfig.watch.json

enterprise-modules/side-bar/tsconfig.types.watch.json

enterprise-modules/set-filter/tsconfig.watch.json

enterprise-modules/set-filter/tsconfig.types.watch.json

enterprise-modules/server-side-row-model/tsconfig.watch.json

enterprise-modules/server-side-row-model/tsconfig.types.watch.json

enterprise-modules/row-grouping/tsconfig.watch.json

enterprise-modules/row-grouping/tsconfig.types.watch.json

enterprise-modules/rich-select/tsconfig.watch.json

enterprise-modules/rich-select/tsconfig.types.watch.json

enterprise-modules/range-selection/tsconfig.watch.json

enterprise-modules/range-selection/tsconfig.types.watch.json

enterprise-modules/multi-filter/tsconfig.watch.json

enterprise-modules/multi-filter/tsconfig.types.watch.json

enterprise-modules/master-detail/tsconfig.watch.json

enterprise-modules/master-detail/tsconfig.types.watch.json

enterprise-modules/filter-tool-panel/tsconfig.watch.json

enterprise-modules/filter-tool-panel/tsconfig.types.watch.json

enterprise-modules/core/tsconfig.watch.json

enterprise-modules/core/tsconfig.types.watch.json

enterprise-modules/advanced-filter/tsconfig.watch.json

enterprise-modules/advanced-filter/tsconfig.types.watch.json

community-modules/vue3/tsconfig.watch.json

community-modules/vue3/tsconfig.types.watch.json

community-modules/infinite-row-model/tsconfig.watch.json

community-modules/infinite-row-model/tsconfig.types.watch.json

community-modules/csv-export/tsconfig.watch.json

community-modules/csv-export/tsconfig.types.watch.json

community-modules/core/tsconfig.watch.json

community-modules/core/tsconfig.types.watch.json

community-modules/client-side-row-model/tsconfig.watch.json

community-modules/client-side-row-model/tsconfig.types.watch.json

compiler-explorer/compiler-explorer

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

tsconfig.tests.json

tsconfig.json

static/tsconfig.json

desktop/desktop

1 of 4 projects failed to build with the old tsc and were ignored

tsconfig.json

discordjs/discord.js

39 of 62 projects failed to build with the old tsc and were ignored

packages/collection/tsconfig.json

packages/collection/tsconfig.eslint.json

packages/collection/tsconfig.docs.json

gcanti/fp-ts

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

tsconfig.json

  • error TS2536: Type 'Extract<keyof NonNullable<A>, string>' cannot be used to index type 'F'.

tsconfig.build-es6.json

  • error TS2536: Type 'Extract<keyof NonNullable<A>, string>' cannot be used to index type 'F'.

Infisical/infisical

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

backend/tsconfig.json

sequelize/sequelize

13 of 16 projects failed to build with the old tsc and were ignored

packages/utils/tsconfig.json

Isabel Duan (iisaduan) commented Jul 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Daniel Rosenwasser (@DanielRosenwasser) interestingly, this is also a current issue (5.5.3, playground) when T explicitly extends {} | null | undefined:

    function f<T extends {} | null | undefined , K extends keyof T>(obj: T, key: K) {
        return obj[key];
               ~~~
!!! error TS18049: 'obj' is possibly 'null' or 'undefined'.
               ~~~~~~~~
!!! error TS2536: Type 'K' cannot be used to index type '{}'.
    }

Isabel Duan (iisaduan) changed the title unconstrained type parameters consistently check for {} | null | undefined in strictNullChecks unconstrained type parameters are consistently checked for {} | null | undefined in strictNullChecks Jul 17, 2024

Isabel Duan (iisaduan) commented Jul 18, 2024

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test top400
TypeScript Bot (@typescript-bot) user test this

TypeScript Bot (typescript-bot) commented Jul 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Command Status Results
test top400 ✅ Started
user test this ✅ Started 👀 Results

TypeScript Bot (typescript-bot) commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

Isabel Duan (@iisaduan) Here are the results of running the user tests with tsc comparing main and refs/pull/59059/merge:

Something interesting changed - please have a look.

Details

azure-sdk

/mnt/ts_downloads/_/m/azure-sdk/build.sh

  • [NEW] error TS2322: Type 'string' is not assignable to type 'never'.
    • /mnt/ts_downloads/_/m/azure-sdk/test/narrowedTypes.ts(59,9)
    • /mnt/ts_downloads/_/m/azure-sdk/test/narrowedTypes.ts(213,9)
    • /mnt/ts_downloads/_/m/azure-sdk/test/narrowedTypes.ts(254,11)

bluebird

/mnt/ts_downloads/_/m/bluebird/tsconfig.json

  • [NEW] error TS2684: The 'this' context of type '((...args: any[]) => Process) | ((name: any, ...args: any[]) => boolean)' is not assignable to method's 'this' of type '(this: null, name?: any, ...args_1: any[]) => Process'.
    • /mnt/ts_downloads/_/m/bluebird/node_modules/bluebird/js/release/debuggability.js(301,28)
  • [MISSING] error TS2684: The 'this' context of type '((...args: any[]) => Process) | ((name: any, ...args: any[]) => boolean)' is not assignable to method's 'this' of type '(this: null, name?: any, ...args: any[]) => Process'.
    • /mnt/ts_downloads/_/m/bluebird/node_modules/bluebird/js/release/debuggability.js(301,28)

effect

tsconfig.json

tsconfig.build.json

tsconfig.base.json

fp-ts

tsconfig.json

  • [NEW] error TS2536: Type 'Extract<keyof NonNullable<A>, string>' cannot be used to index type 'F'.

tsconfig.build-es6.json

  • [NEW] error TS2536: Type 'Extract<keyof NonNullable<A>, string>' cannot be used to index type 'F'.

lodash

/mnt/ts_downloads/_/m/lodash/tsconfig.json

  • [NEW] error TS2322: Type 'SetCache' is not assignable to type 'any[]'.
    • /mnt/ts_downloads/_/m/lodash/node_modules/lodash/_baseUniq.js(39,5)
  • [MISSING] error TS2740: Type 'SetCache' is missing the following properties from type 'any[]': length, pop, concat, join, and 34 more.
    • /mnt/ts_downloads/_/m/lodash/node_modules/lodash/_baseUniq.js(39,5)

webpack

tsconfig.types.json

xterm.js

src/tsconfig-library-base.json

Closed

Anders Hejlsberg (ahejlsberg) commented Jul 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Looks like #50603 is being addressed in #59352 (and I'll comment on that separately there). Meanwhile, in the example

function f<T, K extends keyof T>(obj: T, key: K) {
    return obj[key];  // Ok
}

function g<T extends {} | null | undefined , K extends keyof T>(obj: T, key: K) {
    return obj[key];  // Error, 'obj' is possibly 'null' or 'undefined'.
}

the second error really shouldn't be there. As Daniel Rosenwasser (@DanielRosenwasser) points out above, for any T that includes null or undefined, keyof T is never, so it isn't possible to have a value of type K that would cause the code to fail. In fact, in an obj[key] expression, when key is a valid property name for obj, it must be because obj is of a non-nullable type. That said, the non-nullability check produces nicer and more understandable error messages, so we want to keep it when obj is of a non-generic type. I will put up a PR that fixes that.

This was referenced Jul 24, 2024
Closed
Closed
Closed
github-project-automation Bot moved this from Waiting on reviewers to Done in PR Backlog Mar 24, 2026

TypeScript Bot (typescript-bot) commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies).

Next steps for PRs:

  • For crash bugfixes or language service improvements, PRs are currently accepted at the typescript-go repo
  • Changes to type system behavior should wait until after 7.0, at which point mainline TypeScript development will resume in this repository with the Go codebase
  • Library file updates (lib.d.ts etc) continue to live in this repo or the DOM Generator repo as appropriate

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

Isabel Duan (iisaduan)

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

PR Backlog
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No error on unconstrained type parameter in > comparison

5 participants

Back | FazBrowse Home | New Git URL