-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Improve search screen perf #3752
Conversation
This is only ever useful if you type it exactly correct. Search now does a better job anyway.
Your Render PR Server URL is https://social-app-pr-3752.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-conrkdmg1b2c73dp1go0. |
|
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.
Amazing difference.
@@ -850,6 +864,7 @@ function AutocompleteResults({ | |||
</> | |||
) | |||
} | |||
AutocompleteResults = React.memo(AutocompleteResults) |
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.
@gaearon why you dont just wrap the component with memo?
maybe I'm missing some context
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.
would need a Babel plugin to keep function names readable without adding an extra indentation level due to writing them twice
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.
make sense thank you!
The first few commits extract components with no changes to their logic and add some
memo
s.The main real change is e46aac2. It renders both the autocomplete and the result list so that switching between those is less laggy. This is especially noticeable on iOS.
Finally, there's a few drive-by changes. 16a13c5 removes subdomain matching — it is not very useful now that we properly rank results, and it looks a bit out-of-place in the list. Also, 36485ea adds some missing hitslops to the recent searches.
Test Plan
The usual for this screen.
bla.mov