Repository navigation
Add test for #21317 - #21349
Add test for #21317#21349Deleted user (ghost) wants to merge 1 commit into
Conversation
| //// x/**/!; | ||
| ////} | ||
|
|
||
| verify.typeAtLocation("", "string | undefined"); // TODO: GH#21317: should be "T". |
There was a problem hiding this comment.
You should remove the TODO (and maybe the PR?), the new type at this position is intended, not a bug; see my comment.
We now consider the type at a location of a generic with a nullable constraint that is nonnull-asserted as having been narrowed to the nonnull part of its constraint (since usually you'd do this to access some member or call on it, like a cast).
Also, I'm pretty sure this is exactly what our type baselines test. We knew this changed and intended it.
There was a problem hiding this comment.
Shouldn't it be string and not string | undefined then?
There was a problem hiding this comment.
Wesley Wigham (@weswigham) IMO the test is legitimate, because it tests the type before the assertion (what the issue is about) and not the result of the assertion.
Though maybe that's already covered by the baselines as you mentioned.
Either way the API issue should be fixed.
There was a problem hiding this comment.
The type before the assertion is going to be the constraint (string | undefined, because it's an apparent position thanks to the assertion), and then after the assertion is applied it should just be string. This isn't an API issue, we legitimately interpret the type at that position as just the constraint now, that way the bang after it functions somewhat.
|
Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it. |
3 participants
Adds ability to test
checker.getTypeAtLocation. Enables us to test #21317.