fix: plug visibilitychange listener memory leak, add cleanup tests

The visibilitychange handler was registered with an anonymous arrow
function, making it impossible to remove via removeEventListener.
Extracted it to a named variable (onVisibilityChange) and added proper
cleanup, matching the pattern already used for onError / onRejection /
onUnload.

Added two new tests in the cleanup suite:
- removes visibilitychange event listener
- no longer flushes on visibilitychange after cleanup

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Prateek 2026-02-19 18:59:21 +05:30
parent 080bd1d5fd
commit 9e78ec6c2b
2 changed files with 47 additions and 4 deletions

View File

@ -562,6 +562,47 @@ describe("cleanup", () => {
expect(pageRemoves.length).toBe(1);
});
it("removes visibilitychange event listener", () => {
const removeSpy = vi.spyOn(window, "removeEventListener");
const cleanup = initClientLogger();
cleanup();
const visRemoves = removeSpy.mock.calls.filter(
([type]) => type === "visibilitychange",
);
expect(visRemoves.length).toBe(1);
});
it("no longer flushes on visibilitychange after cleanup", () => {
const cleanup = initClientLogger();
cleanup();
// Reset spies after cleanup flush
fetchSpy.mockClear();
sendBeaconSpy.mockClear();
// Add an entry directly via a new error (won't be captured since error
// listener was removed, but set up document state for the visibility test)
Object.defineProperty(document, "visibilityState", {
value: "hidden",
writable: true,
configurable: true,
});
// Fire visibilitychange -- listener should have been removed
window.dispatchEvent(new Event("visibilitychange"));
expect(sendBeaconSpy).not.toHaveBeenCalled();
expect(fetchSpy).not.toHaveBeenCalled();
// Restore
Object.defineProperty(document, "visibilityState", {
value: "visible",
writable: true,
configurable: true,
});
});
it("clears the flush interval", () => {
const clearIntervalSpy = vi.spyOn(globalThis, "clearInterval");
const cleanup = initClientLogger();

View File

@ -117,11 +117,12 @@ export function initClientLogger(): () => void {
// Flush periodically
flushTimer = setInterval(flush, 10_000);
// Flush on page unload
const onUnload = (): void => flush();
window.addEventListener("visibilitychange", () => {
// Flush on page hide / visibility change
const onVisibilityChange = (): void => {
if (document.visibilityState === "hidden") flush();
});
};
const onUnload = (): void => flush();
window.addEventListener("visibilitychange", onVisibilityChange);
window.addEventListener("pagehide", onUnload);
// Return cleanup function
@ -129,6 +130,7 @@ export function initClientLogger(): () => void {
flush();
window.removeEventListener("error", onError);
window.removeEventListener("unhandledrejection", onRejection);
window.removeEventListener("visibilitychange", onVisibilityChange);
window.removeEventListener("pagehide", onUnload);
if (flushTimer) clearInterval(flushTimer);
if (perfObserver) perfObserver.disconnect();