-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Fix assertion functions accessed via wildcard imports #51324
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
Conversation
|
@typescript-bot test this |
|
Heya @jakebailey, I've started to run the extended test suite on this PR at d5be437. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the abridged perf test suite on this PR at d5be437. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at d5be437. You can monitor the build here. Update: The results are in! |
|
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at d5be437. You can monitor the build here. |
|
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at d5be437. You can monitor the build here. Update: The results are in! |
|
@jakebailey Here are the results of running the user test suite comparing Everything looks good! |
|
@jakebailey Here they are:
Comparison Report - main..51324
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Heya @jakebailey, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
|
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hm. @typescript-bot perf test this |
|
Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at d5be437. You can monitor the build here. Update: The results are in! |
|
@DanielRosenwasser Here they are:
CompilerComparison Report - main..51324
System
Hosts
Scenarios
TSServerComparison Report - main..51324
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Seems like the perf difference was just noise. |
|
Of course, I am left wondering why our deltas don't have error bars, only the raw numbers. |
|
Using this PR to test my new benchmark. @typescript-bot perf test this |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
I oopsed. @typescript-bot perf test this |
|
Heya @jakebailey, I've started to run the perf test suite on this PR at d5be437. You can monitor the build here. Update: The results are in! |
|
@jakebailey Here they are:
CompilerComparison Report - main..51324
System
Hosts
Scenarios
TSServerComparison Report - main..51324
System
Hosts
Scenarios
StartupComparison Report - main..51324
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Well, I give up on that benchmark for now. Will fix it later. Merging. |
Fixes #35004
This comes down to us resolving the symbol in some conditions but not others, which led to an inconsistent experience; these assertions were legal so long as the leftmost symbol wasn't a namespace import. If you imported a wildcard import via another module's named exports, it was legal, but using a wildcard import to access that same export was not.
Hoping to get this in to 4.9 so it can in be our LKG for 5.0; this will allow me to convert the Debug namespace into a module (which will then enable scope hoisting and perf boosts).