V18 2 0 with memory fixes #4
Conversation
| if (fiber.tag === HostComponent) { | ||
| const hostInstance: Instance = fiber.stateNode; | ||
| if (hostInstance !== null) { | ||
| detachDeletedInstance(hostInstance); | ||
| } | ||
| } | ||
| fiber.stateNode = null; |
There was a problem hiding this comment.
abstracting this to detatchFiberStateNode so we can try to run this in a couple of different places.
| // tree, which has its own pointers to children, parents, and siblings. | ||
| // The other host nodes also point back to fibers, so we should detach that | ||
| // one, too. | ||
| detatchFiberStateNode(fiber); |
There was a problem hiding this comment.
always doing this instead of only doing this in certain cases.
| } | ||
| fiberToDetach.stateNode = null; | ||
|
|
||
| if (fiberToDetach.alternate !== null) { |
There was a problem hiding this comment.
From my understanding, fiber.alternate is the Fiber that stores future/new work. Not all fibers will have future work, but if we're unmounting a fiber, we should be sure to cleanup the "future" state too.
| @@ -47,11 +47,35 @@ export function detachDeletedInstance(node: Instance): void { | |||
| // these fields are relevant. | |||
| delete (node: any)[internalInstanceKey]; | |||
| delete (node: any)[internalPropsKey]; | |||
| delete (node: any)[internalContainerInstanceKey]; | |||
There was a problem hiding this comment.
this was something we also cleared out in the previous v17 fix, however its not needed imo. This is only set for the container root node (which I think means the root of react's tree). since v17, you can have multiple roots so react can run multiple versions, but I think we only have one root.
Be always more aggro with cleaning up references to dom nodes in state.