[Experiment] fix(40617): handle uninitialized class member with computed key #45974
Conversation
This file contains 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
typescript-bot
added
the
For Uncommitted Bug
a-tarasyuk
force-pushed
the
fix/40617
branch
2 times, most recently
from
7dc42e8
to
fed9253
Compare
typescript-bot
added
For Backlog Bug
|
@typescript-bot pack this |
|
Heya @andrewbranch, I've started to run the perf test suite on this PR at fed9253. You can monitor the build here. Update: The results are in! |
|
Heya @andrewbranch, I've started to run the tarball bundle task on this PR at fed9253. You can monitor the build here. |
|
Hey @andrewbranch, 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 |
|
@andrewbranch Here they are:Comparison Report - main..45974
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
a-tarasyuk
force-pushed
the
fix/40617
branch
2 times, most recently
from
0d8ea00
to
5966bca
Compare
This was referenced Sep 28, 2021
TS does not automatically understand object with accessor type if not extracted to a variable
#46111
Closed
a-tarasyuk
force-pushed
the
fix/40617
branch
from
5966bca
to
e3d3857
Compare
easyrider
reviewed
Sep 28, 2021
a-tarasyuk
force-pushed
the
fix/40617
branch
2 times, most recently
from
7532268
to
d62e104
Compare
sandersn
added
the
Experiment
weswigham
requested changes
Sep 29, 2021
a-tarasyuk
force-pushed
the
fix/40617
branch
2 times, most recently
from
674f8ac
to
f62eed2
Compare
a-tarasyuk
force-pushed
the
fix/40617
branch
from
591ce0f
to
c1ea660
Compare
a-tarasyuk
force-pushed
the
fix/40617
branch
from
c1ea660
to
5a5a04a
Compare
|
@typescript-bot pack this |
|
Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at 5a5a04a. 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 5a5a04a. You can monitor the build here. |
|
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 - main..45974
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@weswigham @andrewbranch @DanielRosenwasser is this good to go into 4.7? Looks like we've got a recent performance run for it now. |
|
This is something I wanted to figure out - whether the check time increases on the most recent change are within the margin of error. |
|
Between zero and seven hundreths of a second (depending on the test, mapping to between zero and one point four percent) sounds like noise if not noise-adjacent numbers to me. |
|
@typescript-bot perf test this |
|
Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at 5a5a04a. You can monitor the build here. Update: The results are in! |
|
@DanielRosenwasser Here they are:Comparison Report - main..45974
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yeah, this second run makes it look more like noise. We see check time decrease on some and then increase on the same repo with a different Node version. |
|
From the design meeting:
|
|
Weighing in as one user: supporting symbol-named properties in CFA (without temporary variables) seems very nice and is definitely what I would expect. For me, it's easily worth this tiny performance penalty, though I probably like/use symbol named properties more than many. |
|
(happy to move this to a new issue) If I read the release blog post correctly, the following test case should work, but it still produces an error. interface Foo {
[key: string]: number | string;
}
declare function generateKey(): string;
function fn(foo: Foo) {
const key = generateKey();
if (typeof foo[key] === 'number') {
foo[key].toExponential(2);
// Error: Property 'toExponential' does not exist on type 'string | number'.
}
}Is this a bug? TS Playground Link v4.7.0-dev.20220420 |
|
I'd definitely file that as a separate issue, otherwise we're not going to be able to track or discuss it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #40617
Fixes #45965
Fixes #36230
Fixes #29042
Fixes #46111
Fixes #14137