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

Stabilize WeakMap in NodeEventPlugin #3780

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

abelsj60
Copy link
Contributor

@abelsj60 abelsj60 commented Jan 25, 2023

Today, while using the supremely useful NodeEventPlugin, I watched as the number of "click" event listeners on my code lines exponentially increased on every click.

After some hunting and pecking, I discovered that the WeakMap that tracks elements in this plugin was being reset on every render. In other words, it reset to {} on each click. I therefore moved the WeakMap up a line, placing it outside the scope of the registerMutationListener function that makes use of it.

Hark! The exponential increase ceased, the WeakMap stabilized, and a whole bunch of my code went from "bad" to "good."

@trueadm I see you worked heavily on this plugin, is this OK?

I have run npm run test-unit and observed Greenland. I am rolling the dice on the e2es, as I'm still not confident in them.

This LinedCodeNode-related pull request is sibling to:

#3770
#3769
#3695
#3731
#3695
#3693

@vercel
Copy link

vercel bot commented Jan 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
lexical ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 25, 2023 at 7:38PM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 25, 2023 at 7:38PM (UTC)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 25, 2023
Copy link
Collaborator

@trueadm trueadm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@trueadm trueadm merged commit b3b7ae1 into facebook:main Jan 25, 2023
@trueadm trueadm mentioned this pull request Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants