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
[Polymorphic] Improve performance #784
Conversation
<As = IntrinsicElementString>( | ||
props: As extends '' | ||
? { as: keyof JSX.IntrinsicElements } |
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.
Not super familiar with these typings. Is the gist that the conditionals are progressively narrowing from simple to more complex only as necessary?
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.
Yes, and we do this instead of checking As extends React.ElementType
(for both types) because that breaks event typing for intrinsics.
Never used that before, TIL 😅 |
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.
Looks good from my side, works well and the perf improvement is mega 👍
It's really hard to know for sure if tweaks to polymorphic types are improving performance but I picked a huge component (
ScrollArea
) and recorded a video of me tweaking its types (with and without these changes) to show how TS is able to more quickly report issues now.The main change here is to remove the
extends keyof JSX.IntrinsicElements
from the generic which would have been slowing down all components regardless of whether theas
prop was being used.Instead, I check to see if
as
extends an empty string and if so, I type it askeyof JSX.IntrinsicElements
to enable intellisense when used.INTELLISENSE
CleanShot.2021-07-16.at.19.30.35.mp4
BEFORE (~8s)
CleanShot.2021-07-16.at.19.13.12.mp4
AFTER (~5s)
CleanShot.2021-07-16.at.19.14.08.mp4
I'll continue to think of ways to improve it further but this definitely feels better.