This repository has been archived by the owner on Dec 13, 2023. It is now read-only.
Experimental warning to help find connections that are not disconnected #288
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
This commit introduces a new configuration option which aims to help Roact users more easily find components which are not properly disconnecting connections when unmounted and are potentially causing memory leaks because of this reason. A full traversal of the component is performed to find all currently active connections but some keys are excluded to reduce false positives. If a connection is referenced in a component but it's closure does not close over self as an upvalue then it is not a concern that the component still has a reference after being unmounted. This commonly occurs through references to component context or the store. Ideally we would be able to inspect the closure associated with each active connection and warn only if the closure uses self as an upvalue. Internal Roact connections are also ignored for this check, so that connections within EventManager do not cause this warning to be emitted.
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
I have read the CLA Document and I hereby sign the CLA |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR is quite speculative, a similar method to this helped track down a memory leak which I was investigating in a Roact project recently but I am not sure if something so specific can be justified as a Roact debugging configuration. I'd like to put up this PR though to see if anyone has any other ideas about how we could catch simple memory leaks like this within Roact as I think this is the most common type of memory leak in many projects and it seems like something we could help Roact consumers catch.
This PR introduces a new configuration option which aims to help Roact users more easily find components which are not properly disconnecting connections when unmounted and are potentially causing memory leaks because of this reason.
A full traversal of the component is performed to find all currently active connections but some keys are excluded to reduce false positives.
If a connection is referenced in a component but it's closure does not close over self as an upvalue then it is not a concern that the component still has a reference after being unmounted.
This commonly occurs through references to component context or the store. Ideally we would be able to inspect the closure associated with each active connection and warn only if the closure uses self as an upvalue.
Internal Roact connections are also ignored for this check, so that connections within EventManager do not cause this warning to be emitted.