-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
tsc(perf): avoid large unions #30847
Conversation
test fix(tsc): fix large booleanfield union fix(tsc): fix a few other components test
Can't see the notion document right now, but wondering if we could split this out into a few different PRs for sets of changes? |
Ah yes, sorry it was still private 🤦🏼♂️ - https://www.notion.so/sentry/Typescript-performance-audit-ac375cf3d9374e66998a6eab2d5539e4 I gave access to engineering |
@@ -129,7 +131,7 @@ class BaseButton extends React.Component<ButtonProps, {}> { | |||
// Doing this instead of using `Tooltip`'s `disabled` prop so that we can minimize snapshot nesting | |||
if (title) { | |||
return ( | |||
<Tooltip skipWrapper={!disabled} {...tooltipProps} title={title}> |
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.
can we keep this, i think tooltips don't work on disabled buttons otherwise because of mouse events. maybe thats outdated, not sure.
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.
@scttcper yes for sure, this branch is more of a proof of concept and we should pick whatever is useful from here. There’s a solid chance I went a bit overboard with some changes…
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
This is a draft PR that will not get merged, it is posted just as a proof of concept
TLDR: we can cut TSC time almost in half and have a nicer and more responsive development experience
Longer story:
I wanted to understand why the typescript development flow on my machine was slow - code actions on save would take several seconds before type checking was done and errors due to incorrect types would pop-up, this was problematic because it decreased my development speed and resulted in multiple back and fourths between CI and TSC failing to build.
I've written down the investigation process and findings in https://www.notion.so/sentry/Typescript-performance-audit-ac375cf3d9374e66998a6eab2d5539e4
Before ~36s build time:
After ~21s
--diagnostics result before:
--diagnostics result after
The symbols, types, instantiations and memory usage are probably the most notable drops.