Fix memory leak within the @react-facet/dom-fiber #136
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ja-ni
changed the title
pirelenito
changed the title
|
Tests to be produced within the as a Test screen |
…fter-leaving-screens' of github.com:Mojang/ore-ui into core-ui/1166334-shared-facets-are-still-subscribed-to-after-leaving-screens
…cribed-to-after-leaving-screens
…fter-leaving-screens' of github.com:Mojang/ore-ui into core-ui/1166334-shared-facets-are-still-subscribed-to-after-leaving-screens
marlonicus
approved these changes
May 16, 2024
ja-ni
deleted the
core-ui/1166334-shared-facets-are-still-subscribed-to-after-leaving-screens
branch
marlonicus
added a commit
that referenced
this pull request
Aug 12, 2024
* reverts package json changes * reverts package json changes again... * Add unit test for unsubscribing from insertBefore/fast-* component. * updates performance benchmark, formats code * removes only test and deferred mount override * updates broken test file --------- Co-authored-by: Marlon Huber-Smith <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As the renderer handles subscriptions, its important that whenever a Facet data is no longer needed, the renderer should make sure that the cleanup is called.
We have identified two scenarios where the custom React renderer wouldn't correctly unsubscribe from a Facet.
Untracked properties
Some properties that were introduced recently to support SVG didn't had their subscription cleanup correctly called when React components were unmounted.
Appending nodes in specific index positions
On most scenarios, nodes are simply "appended to a child", and for those, the prior implementation was correctly tracking parent-child relationships so that cleanup was handled correctly.
However, the prior implementation would also assume incorrectly that both
insertBeforeandinsertInContainerBeforewould be called after aappendChild, so relationship tracking was not being done correctly and we had some child nodes that were never being cleaned-up.