-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a const
type parameter for Object.freeze
.
#52317
Conversation
@typescript-bot pack this |
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite (tsserver) on this PR at 241b3f2. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at 241b3f2. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at 241b3f2. You can monitor the build here. |
Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 241b3f2. You can monitor the build here. |
Heya @DanielRosenwasser, I've started to run the diff-based user code test suite (tsserver) on this PR at 241b3f2. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the parallelized Definitely Typed test suite on this PR at 241b3f2. You can monitor the build here. |
Heya @DanielRosenwasser, I've started to run the diff-based user code test suite on this PR at 241b3f2. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite on this PR at 241b3f2. 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 are the results of running the user test suite comparing Something interesting changed - please have a look. Details
|
@DanielRosenwasser Here are the results of running the user test suite comparing Everything looks good! |
Heya @DanielRosenwasser, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
@DanielRosenwasser Here they are:Comparison Report - main..52317
System
Hosts
Scenarios
Developer Information: |
@typescript-bot pack this |
Heya @DanielRosenwasser, I've started to run the diff-based user code test suite (tsserver) on this PR at bedfac1. You can monitor the build here. |
Heya @DanielRosenwasser, I've started to run the diff-based user code test suite on this PR at bedfac1. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite on this PR at bedfac1. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite (tsserver) on this PR at bedfac1. 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 bedfac1. You can monitor the build here. |
Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at bedfac1. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the extended test suite on this PR at bedfac1. 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 |
@@ -9,7 +9,7 @@ tests/cases/compiler/weird.js(9,17): error TS7006: Parameter 'error' implicitly | |||
someFunction(function(BaseClass) { | |||
~~~~~~~~~~~~ | |||
!!! error TS2552: Cannot find name 'someFunction'. Did you mean 'Function'? | |||
!!! related TS2728 /.ts/lib.es5.d.ts:321:13: 'Function' is declared here. | |||
!!! related TS2728 /.ts/lib.es5.d.ts:315:13: 'Function' is declared here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really want to send a PR which changes the baselines to filter out all line numbers from lib.*.d.ts
files; it really sucks to modify a baseline and have it all be line numbers that don't matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the same - however, maybe start conservative and only do that on related spans.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other problem is that when you add a new overload to a lib, more than just line numbers change; any use gets more symbols too, so it's not a perfect solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielRosenwasser Here are the results of running the user test suite comparing Something interesting changed - please have a look. Details
|
@DanielRosenwasser Here are the results of running the user test suite comparing Everything looks good! |
Heya @DanielRosenwasser, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
This one's actually interesting! |
There were already mapped through |
@DanielRosenwasser Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Detailsbabel/babel
|
@DanielRosenwasser Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
1 similar comment
@DanielRosenwasser Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
@DanielRosenwasser Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Detailsbabel/babel
|
@DanielRosenwasser Here they are:Comparison Report - main..52317
System
Hosts
Scenarios
Developer Information: |
Literally talked myself out of this during today's design meeting. It almost makes sense until you realize that nested objects aren't frozen. While lots of users would like for |
This PR removes an unnecessary signature from
Object.freeze
by using TypeScript 5.0's newconst
type parameters.I think if we want to try this out, 5.0 beta is the right time.