Remove special any assignability for numeric index signatures#41660
Remove special any assignability for numeric index signatures#41660DanielRosenwasser merged 8 commits intomasterfrom
any assignability for numeric index signatures#41660Conversation
|
@typescript-bot pack this |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
A couple of breaks:
|
| interface Lifecycle<Attrs, State> { | ||
| oninit?(vnode: Vnode<Attrs, State>): number; | ||
| ~~~~~ | ||
| !!! error TS2344: Type 'State' does not satisfy the constraint 'Lifecycle<Attrs, State>'. |
There was a problem hiding this comment.
This was a bug. State needs the constraint Lifecycle<Attrs, State>.
| class C implements ClassComponent<MyAttrs> { | ||
| view(v: Vnode<MyAttrs>) { return 0; } | ||
| ~~~~ | ||
| !!! error TS2416: Property 'view' in type 'C' is not assignable to the same property in base type 'ClassComponent<MyAttrs>'. |
There was a problem hiding this comment.
This basically comes down to the following break:
interface Indexable {
[x: number]: any;
}
class C implements Indexable {
}In earlier versions, implementing C without re-declaring the index signature was valid.
I would argue that this behavior is still surprising or incorrect for string index signatures, but that might be very breaky.
|
|
|
Updates:
|
|
@typescript-bot pack this |
|
Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at 7dd8232. You can monitor the build here. |
|
Heya @DanielRosenwasser, I've started to run the parallelized Definitely Typed test suite on this PR at 7dd8232. You can monitor the build here. |
|
Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at 7dd8232. You can monitor the build here. Update: The results are in! |
|
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 7dd8232. You can monitor the build here. |
|
Heya @DanielRosenwasser, I've started to run the parallelized community code test suite on this PR at 7dd8232. You can monitor the build here. |
|
Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at 7dd8232. You can monitor the build here. Update: The results are in! |
|
Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@DanielRosenwasser Here they are:Comparison Report - master..41660
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@DanielRosenwasser Here they are:Comparison Report - master..41660
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I don't think we're correctly running the community test suite anymore (#42875), so I can't confidently comment on if anything new broke. But I think this is now a "serious PR". |
|
Assuming there's nothing too concerning in the user tests, I'm okay with putting this in the nightly and watching for fallout. I stand by the current behavior being a bug 🙂 |
Experiment to fix #18757