Skip to content

fix(frontend): prevent memory leak from uncleaned storage listener in useFeatureFlag - #1295

Merged
github-actions[bot] merged 1 commit into
leojay-net:mainfrom
Mercy017:fix/issue-1220-featureflag-subscription-leak
Jul 27, 2026
Merged

fix(frontend): prevent memory leak from uncleaned storage listener in useFeatureFlag#1295
github-actions[bot] merged 1 commit into
leojay-net:mainfrom
Mercy017:fix/issue-1220-featureflag-subscription-leak

Conversation

@Mercy017

Copy link
Copy Markdown
Contributor

Root cause

useFeatureFlag called window.addEventListener('storage', evaluate) inside useIsomorphicLayoutEffect but never returned a cleanup function. Each time flag or scrollTargetId changed, React re-ran the effect — adding a new listener without removing the previous one. After n re-renders, n orphaned listener references accumulated in window._eventListeners, each retaining the evaluate closure (and with it flag, scrollTargetId, setIsEnabled, etc.) in memory.

Fix (src/hooks/useFeatureFlag.ts)

  1. Extract the body as a named evaluate() function so the same reference is passed to both addEventListener and removeEventListener (required for correct deregistration).
  2. Return () => window.removeEventListener('storage', evaluate) from the effect so React cleans up on unmount and before every re-run triggered by a dep change.
-   const newEnabled = getFeatureFlag(flag);
-   setIsEnabled(newEnabled);
-
+   function evaluate() {
+     const newEnabled = getFeatureFlag(flag);
+     setIsEnabled(newEnabled);
+
+   }
+
+   evaluate();
+
+   if (typeof window !== 'undefined') {
+     window.addEventListener('storage', evaluate);
+     return () => window.removeEventListener('storage', evaluate);
+   }
  }, [flag, scrollTargetId]);

The storage listener also makes flag state reactive: toggling a flag via devtools or another tab now updates the component without a page reload.

Regression tests (src/hooks/__tests__/useFeatureFlag.test.ts)

  • removes storage listener on unmount so it does not leak — spies on window.addEventListener/removeEventListener and asserts the exact handler registered is also removed when the hook unmounts.
  • re-evaluates the flag when a storage event fires — dispatches a StorageEvent after changing the mock return value and asserts the hook state updates.

Closes #1220

…eojay-net#1220)

useFeatureFlag added a window 'storage' listener on each render (when flag
or scrollTargetId changed) but never returned a cleanup function to remove it.
Every re-render that changed those deps leaked an additional listener, causing
the evaluate function closure to be retained in memory indefinitely.

Fix: wrap the window.addEventListener call in a cleanup-returning block so
React removes the listener whenever the effect re-runs or the component unmounts.
Also extracts evaluate() as a named function so the exact same reference is
passed to both addEventListener and removeEventListener (required for correct
deregistration).

The storage listener also adds runtime reactivity: flag state is now refreshed
when another tab or a devtools script modifies localStorage.

Regression tests added in hooks/__tests__/useFeatureFlag.test.ts:
- Verifies the storage listener is removed on unmount
- Verifies re-evaluation fires when a storage event is dispatched

Closes leojay-net#1220
@github-actions
github-actions Bot merged commit fa1769c into leojay-net:main Jul 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(frontend): prevent memory leak from uncleaned subscription in useFeatureFlag.ts

1 participant