-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] Memoize editor frame #89865
[Lens] Memoize editor frame #89865
Conversation
@@ -514,6 +522,12 @@ export function App({ | |||
} | |||
}; | |||
|
|||
const lastKnownDocRef = useRef(state.lastKnownDoc); |
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.
Those are save to pass in via refs because they are only used in the onChange
callback to compare
Pinging @elastic/kibana-app (Team:KibanaApp) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
Code LGTM, tested it locally, this is a nice improvement!
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
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.
Tested on Chrome and FF 👍 . It is much more fluid the editor experience.
Just realised I pressed submit on merge already happened :D |
Memoizes editor frame rendering in Lens app. This avoid re-rendering the frame twice on every state change (because the
onChange
hook in the app is called which is causing a subsequent re-render of the frame.Originally I planned to do this as part of #89690 but it's a quick win we can get in easily and IMHO the improvement is relevant enough to change it now
old:
new (note the missing "mount" block):