Fix a memory leak in ReactComponentTreeDevtool #6753
Conversation
ghost
added
gaearon
force-pushed
the
fix-6750
branch
2 times, most recently
from
d912a94 to
427cb39
Compare
| @@ -106,6 +106,11 @@ var ReactComponentTreeDevtool = { | |||
| }, | |||
|
|
|||
| purgeUnmountedComponents() { | |||
| if (ReactComponentTreeDevtool._preventPurging) { | |||
There was a problem hiding this comment.
Not a big fan of these but I’m not sure how else we can test the expected tree during server rendering, if ReactDebugTool cleans it up right before the batch. Alternatively we can make those integration tests instead in ReactPerf-test, but I like how comprehensive the suite for ReactComponentDebugTool is, so I don’t want to replicate the same complexity in ReactPerf-test.
There was a problem hiding this comment.
Could you add another devtool in the test that checks the state at an appropriate time? If not or if that's too clunky, I think this seems fine too.
There was a problem hiding this comment.
I’d keep it this way for now. I’m not sure about this reliance on flush events anyway (#6046 (comment)), so I’d rather avoid relying on the order in which devtools are called for now.
|
Thanks. |
`ReactDebugTool` used to only call `purgeUnmountedComponents()` while profiling, so information about unmounted instances kept accumulating when not profiling. Additionally, unmounting in React Native and rendering to string did not correctly clean up the devtool. Finally, the tests tested the wrong behavior and relied on explicit `purgeUnmountedComponent()` calls. To fix this, we: * Test specifically that unmounting is enough to clean up the tree devtool. * Add missing `onBeginFlush` and `onEndFlush` calls to server and native rendering so `ReactDebugTool` knows when to copy the tree. Fixes #6750
|
@gaearon updated the pull request. |
Fix a memory leak in ReactComponentTreeDevtool (cherry picked from commit de1bb7a)
ReactDebugToolused to only callpurgeUnmountedComponents()while profiling, so information about unmounted instances kept accumulating when not profiling.Additionally, unmounting in React Native and rendering to string did not correctly clean up the devtool.
Finally, the tests tested the wrong behavior and relied on explicit
purgeUnmountedComponent()calls.To fix this, we:
onBeginFlushandonEndFlushcalls to server and native rendering soReactDebugToolknows when to copy the tree.Fixes #6750.
Reviewers: @sebmarkbage @spicyj.