-
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
Convert debug namespace into a module, direct import #51455
base: main
Are you sure you want to change the base?
Conversation
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at efa0cae. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - main..51455
System
Hosts
Scenarios
TSServerComparison Report - main..51455
System
Hosts
Scenarios
StartupComparison Report - main..51455
System
Hosts
Scenarios
Developer Information: |
Seems like a couple percent; I'll have to retry this once the benchmarker is stable. |
ad14c9d
to
6032b92
Compare
6032b92
to
59d1a30
Compare
eb2e855
to
e6163e8
Compare
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at 2b510af. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - main..51455
System
Hosts
Scenarios
TSServerComparison Report - main..51455
System
Hosts
Scenarios
StartupComparison Report - main..51455
System
Hosts
Scenarios
Developer Information: |
@a-tarasyuk If you're bored, you may want to try rebasing your direct import PR on top of this and see what happens; I think this should fix the runtime errors due to cycles, but, I can't be totally sure. (I'm not planning on getting this one looked at until after the beta cutoff.) |
@jakebailey I've added changes to #51590 , however, it doesn't solve the issue of running tests with the |
Hm, seems to be just performanceCore; likely some of that can be made lazy as I don't see a major reason why we need to have it available until one of the functions is called. |
I’ve added a couple of changes and the build seems to have passed. 😱 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at 3a67373. You can monitor the build here. Update: The results are in! |
@@ -2,7 +2,8 @@ | |||
|
|||
export * from "../corePublic"; | |||
export * from "../core"; | |||
export * from "../debug"; | |||
import * as Debug from "../debug"; | |||
export { Debug }; |
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.
It turns out that I may actually be able to write:
export { /** @internal */ Debug }
And the comment is preserved and applies, which generally may be a better way to do this in the future.
@jakebailey Here they are:
CompilerComparison Report - main..51455
System
Hosts
Scenarios
TSServerComparison Report - main..51455
System
Hosts
Scenarios
StartupComparison Report - main..51455
System
Hosts
Scenarios
Developer Information: |
@jakebailey Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@jakebailey Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
StartupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the regular perf test suite on this PR at 042b96c. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@typescript-bot perf test this |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
Unfortunately, my work to make our package smaller by reusing the public API basically renders this PR pointless outside of @typescript-bot perf test this |
@typescript-bot perf test this |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
#59544 has actually shown that to not be true, phew! |
1b43f97
to
0296a2c
Compare
@typescript-bot perf test this |
0296a2c
to
2e90314
Compare
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
Now we're talkin |
2e90314
to
9cb279e
Compare
For #51441.
This is best viewed without whitespace: https://github.com/microsoft/TypeScript/pull/51455/files?w=1
All of the real changes are in
src/compiler/debug.ts
.Overall package size
Files
lib/tsc.js
lib/tsserver.js
lib/typescript.js