perf: stop leaking React trees on repeated injection mutations - #12436
Merged
Conversation
Follow-up to #12432, which fixed the same class of leak for injectAvatar and the tips buttons on x.com. The remaining injection points that create a React tree inside an onNodeMutation/onTargetChanged handler had the same problem: a mutation of the watched element (lazy image src swaps, hover states, feed virtualization reusing nodes) re-runs the handler, which attaches a brand-new shadow-root sibling + React tree without tearing down the previous one. On a long scroll session this accumulates orphaned DOM nodes / shadow roots / React roots (each with its own data-fetching hooks) that are never cleaned up until navigation. - facebook/minds/instagram injectAvatar: call remove() before re-creating the DOMProxy, mirroring the twitter.com fix from #12432. - injectMaskIconToPostTwitter: post.author refines several times as post parsing completes and re-fires on every onNodeMutation of the tweet, so each add() stacked another DOMProxy shadow sibling + <Icon> React root (visible as duplicated Mask icons on the post). Reuse a single proxy, tear down the previous tree first, and release the post.author subscription on signal abort instead of leaking it. - injectMaskUserBadgeAtTwitter (MaskIcon `_` helper): check() re-ran on every mutation and re-called attachReactTreeWithContainer with the same shadow key, which the mounter rejects with a console.error on every floating-bio-card hover while also clobbering `remover` to a no-op. Attach once; remove()/onRemove reset the guard so a genuinely re-added element still re-attaches. - site-adaptor-infra startPostListener: the per-post AbortController wired a listener onto the page-lifetime signal on every post `set` and never removed it, so every post scrolled past left a permanent listener (and its retained controller). Pass { signal: abort.signal } so it drops when the post unmounts. Output values are unchanged; only the redundant/leaked work is removed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013SLkuieqJXCMg1xoL98jij
This file contains hidden or 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
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.
Summary
Follow-up to #12432, which fixed this class of React-tree leak for
injectAvatarand the tips buttons on x.com. The remaining injection points that build a React tree inside anonNodeMutation/onTargetChangedhandler had the same bug: a mutation of the watched element (lazy imagesrcswaps, hover states, feed virtualization reusing nodes) re-runs the handler, which attaches a fresh shadow-root sibling + React tree without tearing down the previous one. Over a long scroll session this accumulates orphaned DOM nodes / shadow roots / React roots — each with its own data-fetching hooks — until navigation.Changes
facebook/minds/instagraminjection/Avatar/index.tsxremove()before re-creating theDOMProxy, mirroring thetwitter.comfix from #12432.twitter.com/injection/MaskIcon.tsx—injectMaskIconToPostTwitterpost.authorrefines several times as parsing completes and re-fires on everyonNodeMutationof the tweet, so eachadd()stacked anotherDOMProxyshadow sibling +<Icon>React root (visible as duplicated Mask icons on posts). Reuse a single proxy, tear down the previous tree first, and release thepost.authorsubscription onsignalabort.twitter.com/injection/MaskIcon.tsx—injectMaskUserBadgeAtTwitter(_helper)check()re-ran on every mutation and re-calledattachReactTreeWithContainerwith the same shadow key, which the mounter rejects with aconsole.erroron every floating-bio-card hover while also clobberingremoverto a no-op. Attach once;remove()/onRemovereset the guard so a genuinely re-added element still re-attaches.site-adaptor-infra/ui.ts—startPostListenerAbortControllerwired a listener onto the page-lifetimesignalon every postsetand never removed it, so every post scrolled past left a permanent listener (and its retained controller). Pass{ signal: abort.signal }so it drops when the post unmounts.Behavior
Output values are unchanged; only the redundant / leaked work is removed.
Test plan
console.errorfromattachReactTreeToMountedRoot, no duplicate Mask icons on posts whose author re-parses, DOM node / listener count stays flat.🤖 Generated with Claude Code
https://claude.ai/code/session_013SLkuieqJXCMg1xoL98jij