-
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 Diagnostics to a module, direct import #55894
Conversation
[git-generate] set -x npm ci npx eslint . --fix
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the regular perf test suite on this PR at abea810. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison 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: |
I took a look and I know why this happens. Here is one of the diagnostics as an example: export const Show_diagnostic_information: DiagnosticMessage =
/* @__PURE__ */ diag(
6149,
DiagnosticCategory.Message,
"Show_diagnostic_information_6149",
"Show diagnostic information."); The problem is that while the call to One hack to fix this could be to move (or copy) the definition of |
|
Redeclaring it locally (or not using it at all) does work, though; I'll commit that workaround for now. Thanks for the suggestion! |
The problem is a combination of:
This is an esbuild problem of course and not a problem inherent to all bundlers. You could imagine another implementation of esbuild that defers all optimization decisions until link time to handle things like this. That's not how esbuild currently works, however. |
Ah, gotcha. Thanks for the explainer. |
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the bun perf test suite on this PR at c71fe64. You can monitor the build here. Update: The results are in! |
Heya @jakebailey, I've started to run the regular perf test suite on this PR at c71fe64. You can monitor the build here. Update: The results are in! |
I have a bit of a weird bug report for you: #55915. I found it while trying to help you out here. I tried making an incompatible definition of |
Thanks for that; I'm sure it's a bug! TS only "recently" started complaining about relating enums with compatible values, so I'm sure there are just more edge cases there. |
The reason the package gets larger is thanks to this, which is needed in the public API (unfortunately people use this object even though it's internal so I'm afraid to remove it): I had a WIP esbuild change which would have helped improve this; I need to resurrect that change and see if it's still workable. Basically, I had a version of |
@jakebailey Here they are:
CompilerComparison 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:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
StartupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
Perf positive, that's nice. Bun seems to be slightly slower but unsure if that's real as it's so tiny. |
Going to close this; we're moving more in the direction of being ESM, so there's no point in trying to make the diagnostics tree shakable in this way since we'd want to share the code. |
This is the same as #51455 but for the diagnostics module.
I was hoping this would tree shake out a bunch in😅tsc.js
, butesbuild
appears to not notice that a load of diagnostics (370 or so) are unused intsc
and can be removed. I need to report that upstream (could be evanw/esbuild#3256?), though maybe if I just @evanw he might be interestedSee discussion about the tree shaking; I've committed a workaround which has helped.
So far, the change is:
Overall package size
Files
lib/tsc.js
lib/typescript.js