Skip to content

fix: close WebSocket on pagehide to support bfcache #162

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.

Sign up for GitHub

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

Open
FeironoX5 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: close WebSocket on pagehide to support bfcache #162

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c31547
fix: close WebSocket on pagehide to support bfcache
FeironoX5 Feb 13, 2026
715f016
fix: lint
FeironoX5 Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion packages/javascript/src/modules/socket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ export default class Socket implements Transport {
*/
private reconnectionAttempts: number;

/**
* Page hide event handler reference (for removal)
*/
private pageHideHandler: () => void;

/**
* Creates new Socket instance. Setup initial socket params.
*
Expand All @@ -77,6 +82,11 @@ export default class Socket implements Transport {
this.reconnectionTimeout = reconnectionTimeout;
this.reconnectionAttempts = reconnectionAttempts;

this.pageHideHandler = () => {
log('Page entering bfcache, closing WebSocket', 'info');
Copy link
Member

@neSpecc neSpecc Feb 13, 2026

Choose a reason for hiding this comment

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

I don't think users will be happy to see this log in their application. Let's remove it after testing.

this.close();
};
Comment on lines +85 to +88
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

pageHideHandler always logs "Page entering bfcache" and closes the socket, but it doesn’t receive the pagehide event so it can’t check event.persisted. This makes the log message inaccurate and deviates from the MDN pattern (close only when the page is being persisted to bfcache). Consider typing the handler as (event: PageTransitionEvent) => void, checking event.persisted, and updating the log message accordingly (or only logging when persisted is true).

Copilot uses AI. Check for mistakes.

this.eventsQueue = [];
this.ws = null;

Expand Down Expand Up @@ -120,7 +130,21 @@ export default class Socket implements Transport {
}

/**
* Create new WebSocket connection and setup event listeners
* Remove window event listeners
*/
public destroyListeners(): void {
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

destroyListeners() is declared public, but it appears to be an internal helper (only used inside this class). Exposing it expands the public API surface of Socket without being part of the Transport interface; consider making it private (or introducing a single public destroy() method that both removes listeners and closes the socket if external cleanup is intended).

Suggested change
public destroyListeners(): void {
private destroyListeners(): void {

Copilot uses AI. Check for mistakes.
window.removeEventListener('pagehide', this.pageHideHandler, { capture: true });
}

/**
* Setup window event listeners
*/
private setupListeners(): void {
window.addEventListener('pagehide', this.pageHideHandler, { capture: true });
}
Comment on lines +142 to +144
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

The new bfcache behavior (listening to pagehide and closing the WebSocket) isn’t covered by tests. Since this package already uses Vitest + jsdom, it should be possible to add a unit test that mocks WebSocket, dispatches a pagehide event (with persisted: true if you adopt that check), and asserts that close() is called and that a later send() recreates the connection.

Copilot uses AI. Check for mistakes.

/**
* Create new WebSocket connection and setup socket event listeners
*/
private init(): Promise<void> {
return new Promise((resolve, reject) => {
Expand All @@ -139,6 +163,8 @@ export default class Socket implements Transport {
* @param event - websocket event on closing
*/
this.ws.onclose = (event: CloseEvent): void => {
this.destroyListeners();

if (typeof this.onClose === 'function') {
this.onClose(event);
}
Expand All @@ -154,6 +180,8 @@ export default class Socket implements Transport {
};

this.ws.onopen = (event: Event): void => {
this.setupListeners();

if (typeof this.onOpen === 'function') {
this.onOpen(event);
}
Expand All @@ -163,6 +191,16 @@ export default class Socket implements Transport {
});
}

/**
* Closes socket, it can be restored with init() later
*/
private close(): void {
if (this.ws) {
this.ws.close();
this.ws = null;
}
}

/**
* Tries to reconnect to the server for specified number of times with the interval
*
Expand Down