Delete the {} and unconstrained type parameter assignability rule #33570
Conversation
|
@typescript-bot run dt If our baselines are any indication, removing this legacy compat rule is preeeeeeety breaky, but, much like the base change of constraint from |
|
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 248cbba. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Heya @weswigham, I've started to run the parallelized community code test suite on this PR at 248cbba. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Heya @weswigham, I've started to run the extended test suite on this PR at 248cbba. You can monitor the build here. It should now contribute to this PR's status checks. |
weswigham
requested review from
DanielRosenwasser,
ahejlsberg and
RyanCavanaugh
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
@weswigham I think the next step is to post an analysis of the breaks. Are there a lot? Are they worth it? |
|
@typescript-bot run dt |
|
Heya @weswigham, I've started to run the parallelized community code test suite on this PR at 0265829. You can monitor the build here. |
|
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 0265829. You can monitor the build here. |
|
Heya @weswigham, I've started to run the extended test suite on this PR at 0265829. You can monitor the build here. |
|
Heya @weswigham, I've started to run the perf test suite on this PR at 0265829. You can monitor the build here. Update: The results are in! |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
@weswigham Here they are:Comparison Report - master..33570
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Yes. Mostly, an unconstrained type parameter is no longer assignable to Honestly, it looks like a lot in the DT error log, but it's not that bad; there are just some incorrect usages in some very depended-upon packages. Y'know, it just seems like anywhere where people went "T is our unconstrained schema definition, right? right" not realizing that
Maybe. Generally speaking, in pretty much every single DT break, the break can be fixed by writing an explicit |
|
Also, ourselves: The sanity check LKG build on pipelines catches us making the same mistake: src/services/shims.ts(574,73): error TS2345: Argument of type '() => T' is not assignable to parameter of type '() => {}'.
Type 'T' is not assignable to type '{}'.
src/services/shims.ts(575,62): error TS2352: Conversion of type '{}' to type 'T' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
'{}' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{}'.
src/services/utilities.ts(2744,88): error TS2769: No overload matches this call.
Overload 1 of 2, '(array: readonly {}[], f: (x: {}, i: number) => U): U[]', gave the following error.
Argument of type '(x: T, i: number) => U' is not assignable to parameter of type '(x: {}, i: number) => U'.
Types of parameters 'x' and 'x' are incompatible.
Type '{}' is not assignable to type 'T'.
'{}' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{}'.
Overload 2 of 2, '(array: readonly {}[] | undefined, f: (x: {}, i: number) => U): U[] | undefined', gave the following error.
Argument of type '(x: T, i: number) => U' is not assignable to parameter of type '(x: {}, i: number) => U'.
Types of parameters 'x' and 'x' are incompatible.
Type '{}' is not assignable to type 'T'.
'{}' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{}'.
src/services/utilities.ts(2744,96): error TS2345: Argument of type 'T | readonly T[]' is not assignable to parameter of type 'T'.
Type 'readonly T[]' is not assignable to type 'T'.
'readonly T[]' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{}'.
src/services/utilities.ts(2751,9): error TS2322: Type '{} | T' is not assignable to type 'T'.
Type '{}' is not assignable to type 'T'.
'{}' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{}'.Namely in |
|
From the design meeting: try to see whether we can at least give an actionable error on this change for users. |
|
@weswigham do you want to keep working on this? Seems like it got stuck adding an error. |
weswigham
force-pushed
the
delete-legacy-empty-obj-type-param-rule
branch
from
977e511
to
bc3d515
Compare
|
@sandersn This is now synced, and has an informative related span that picks out the type parameter causing the error and suggests giving it an |
|
@sandersn do you want to re-milestone this/add it to a design meeting backlog again? |
|
Let's bring it up at the design meeting. |
|
Did we ever bring this up at a design meeting? We miiight be able to squeeze it in before variance tomorrow or more likely next week. |
|
Based on my comments saying I implemented design meeting feedback (a helpful related span), we probably did at least once, but not again after that, given there's no linked notes. |
Fixes #32330