[Console] Fix leaky mappings subscription#45646
Conversation
Fix app not never unmounting
|
Pinging @elastic/es-ui |
💚 Build Succeeded |
cjcenizal
left a comment
There was a problem hiding this comment.
Tested locally, code LGTM! Just had a couple questions. Can this be backported to 7.4 so it will ship with the next patch release of that minor? If so then we can add the 7.4.1 label to this PR.
| throw new Error(message); | ||
| } | ||
|
|
||
| let unmount: AppUnmount | Promise<AppUnmount>; |
There was a problem hiding this comment.
Could you clarify for me the scenario in which unmount would be assigned a promise value?
There was a problem hiding this comment.
This was done to satisfy the type constraints introduced by core: https://github.com/elastic/kibana/blob/master/src/core/public/application/types.ts#L82
| if (aceTextAreaElement) { | ||
| document.removeEventListener('keydown', documentKeyDownListener); | ||
| aceTextAreaElement.removeEventListener('keydown', aceKeydownListener); | ||
| document.removeChild(overlayMountNode.current!); |
There was a problem hiding this comment.
Why is this being removed?
There was a problem hiding this comment.
I was running into the first of these two issues: https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild#Errors_thrown
Which indicated that cleanup for this DOM node is happening with the react component unmounting.
There was a problem hiding this comment.
Update: I re-implemented this in a more robust way.
|
@elasticmachine merge upstream |
💚 Build Succeeded |
* Fix mappings subscription (still needs further refactoring) Fix app not never unmounting * More defensive removeChild behaviour on unmount
…ole_app_data_flow * 'master' of github.com:elastic/kibana: [Console] Fix leaky mappings subscription (elastic#45646) TypeScriptify index_patterns/index_patterns/flatten_hit.js (elastic#45269) # Conflicts: # src/legacy/core_plugins/console/np_ready/public/application/containers/main/main.tsx # src/legacy/core_plugins/console/public/quarantined/src/app.js
Summary
Fixes the first issue mentioned here: #45032
Also fixes unmount never being called.
How to review
const POLL_INTERVAL = 60000;to a10000to see the requests at a higher rate).proxy?_alias...requests firing.Release Note
Fixed a subscription in Console that didn't clear even after navigating away from the app.