Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[react-interactions] Fix memory leak in event responder system #17421

Merged
merged 2 commits into from
Nov 21, 2019

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Nov 21, 2019

This should fix some issues with leaking fibers from root events in the event responder system. Specifically:

  • Unsure we unmount listeners on Scope Nodes
  • Unsure we memoize the responders map back on fibers

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 21, 2019

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 473a138:

Sandbox Source
gifted-rosalind-1d62w Configuration

@gaearon
Copy link
Collaborator

gaearon commented Nov 21, 2019

Can you make a small fixture to reproduce the leak?

@sizebot
Copy link

sizebot commented Nov 21, 2019

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 473a138

@sizebot
Copy link

sizebot commented Nov 21, 2019

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 473a138

@trueadm
Copy link
Contributor Author

trueadm commented Nov 21, 2019

@gaearon I'll add a regression test for the scopes as that's the main cause of this leak.

Remove debugger
@trueadm trueadm merged commit 007a276 into facebook:master Nov 21, 2019
@trueadm trueadm deleted the flare-leak branch November 21, 2019 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants