Skip to content

fix: preserve WebSocket connections on SPA route changes (#25)#47

Open
emavitta wants to merge 2 commits into
law-chain-hot:mainfrom
emavitta:fix/spa-navigation-clears-connections
Open

fix: preserve WebSocket connections on SPA route changes (#25)#47
emavitta wants to merge 2 commits into
law-chain-hot:mainfrom
emavitta:fix/spa-navigation-clears-connections

Conversation

@emavitta
Copy link
Copy Markdown

@emavitta emavitta commented May 14, 2026

Problem

In a single-page app, changing route makes the connection disappear from the DevTools panel even though the WebSocket is still open — and subsequent messages on that socket are no longer shown. This matches issue #25.

Root cause

background.js detected "real navigation" with a chrome.tabs.onUpdated listener that compared URL pathnames:

const isRealNavigation = !previousData || currentBaseUrl !== previousBaseUrl;

That heuristic is wrong for SPAs. A client-side route change via history.pushState/replaceState changes the pathname without reloading the document. The injected proxy keeps the ProxiedWebSocket alive (correct), but the background treated the pathname change as a full reload, cleared websocketData.connections for the tab, and sent page-refresh to the panel — which called handleClearConnections().

After that, incoming message events can't restore the connection either: the panel only updates connectionsMap entries that already exist, so the wiped connection never reappears.

Fix

Detect real document loads from the content script instead of from URL diffing. A content script is re-injected on every full page load/reload but never on an SPA route change — so it's the reliable signal.

  • content.js sends a page-loaded message on injection (top frame only, so an iframe reload can't wipe the parent's connections).
  • background.js clears that tab's connections + notifies the panel in response to page-loaded, and the old chrome.tabs.onUpdated / tabUrls logic is removed.

No new manifest permissions. Real reloads still clear correctly; SPA navigations now preserve the live connection and its message stream.

Testing

Built the extension and loaded it unpacked. Verified on a path-routed SPA: navigating between routes keeps the connection and its messages in the panel, while a full page reload still resets state as expected.


Summary by cubic

Preserves WebSocket connections during SPA route changes and keeps the message stream alive. Connections now clear only on real page loads or when navigating to pages where the content script can’t run.

  • Bug Fixes
    • Detect real document loads via content.js: sends page-loaded from the top frame on injection.
    • In background.js, use clearTabConnections() to reset on page-loaded and on non-http(s) navigations via a scoped chrome.tabs.onUpdated fallback; notify the panel with page-refresh.
    • Removed the old URL-diffing heuristic; no new permissions.

Written for commit 01caba4. Summary will update on new commits.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/background/background.js">

<violation number="1" location="src/background/background.js:291">
P2: Stale-connection regression: connection clearing now depends exclusively on content-script `page-loaded` messages, which never arrive on non-HTTP/HTTPS navigations (chrome://, file://, etc.) since the content script only matches `http://*/*` and `https://*/*`. Unlike the removed `chrome.tabs.onUpdated` listener, there is no fallback cleanup for tabs that navigate to pages outside the content-script match patterns.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/background/background.js
@emavitta
Copy link
Copy Markdown
Author

Thanks for the review. Valid point — addressed in 535a7f9.

After removing the chrome.tabs.onUpdated pathname heuristic, connection cleanup relied solely on the content script's page-loaded message, which never fires for tabs navigating to URLs the content script can't run on (chrome://, file://, extension pages).

535a7f9 re-adds a chrome.tabs.onUpdated listener, but scoped strictly to non-http(s) destinations:

chrome.tabs.onUpdated.addListener((tabId, changeInfo) => {
  if (!changeInfo.url) return;
  if (/^https?:\/\//i.test(changeInfo.url)) return; // handled by content script
  clearTabConnections(tabId, changeInfo.url);
});

This covers the gap the content script can't, and it can't reintroduce #25: an SPA route change always stays on http/https, so it never reaches clearTabConnections. The clearing logic is now shared via a clearTabConnections() helper used by both paths.

@cubic-dev-ai

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented May 14, 2026

Thanks for the review. Valid point — addressed in 535a7f9.

After removing the chrome.tabs.onUpdated pathname heuristic, connection cleanup relied solely on the content script's page-loaded message, which never fires for tabs navigating to URLs the content script can't run on (chrome://, file://, extension pages).

535a7f9 re-adds a chrome.tabs.onUpdated listener, but scoped strictly to non-http(s) destinations:
...

@emavitta Looks good — this addresses the gap I flagged. The two-path approach is clean:

  1. Content script → page-loaded — fires on any real document load where the content script runs (i.e., http/https pages). SPA route changes never re-inject the script, so they're correctly ignored here.

  2. Scoped tabs.onUpdated — catches the edge cases the content script can't reach (chrome://, file://, extension pages). Since SPA routes always stay on http/https, the early return (/^https?:\/\//i.test(url)) prevents it from ever re-triggering 页面跳转后,websocket 连接被清空,无法看到后续的消息 #25.

The shared clearTabConnections() helper keeps both paths consistent — same cleanup, same page-refresh message shape. No new manifest permissions, no regression risk for the original SPA bug.

Commit 535a7f9 resolves the concern from my previous review.

emavitta added 2 commits May 14, 2026 17:08
…ot#25)

The background script detected "real navigation" via a chrome.tabs.onUpdated
listener that compared URL pathnames. That heuristic is wrong for SPAs: a
client-side route change (history.pushState/replaceState) changes the
pathname without reloading the document, so the listener wiped still-alive
connections from the panel and stopped showing their subsequent messages.

Real document loads are now detected by the content script itself, which is
re-injected on every full page load/reload but never on an SPA route change.
On load it sends a "page-loaded" message (top frame only) and the background
clears that tab's connections in response. No new manifest permissions.
Addresses review feedback (identified by cubic): after removing the
chrome.tabs.onUpdated pathname heuristic, cleanup relied solely on the
content script's page-loaded message, which never arrives when a tab
navigates to a URL the content script cannot run on (chrome://, file://,
extension pages), leaving stale connections for that tab.

Re-add a chrome.tabs.onUpdated listener scoped strictly to non-http(s)
destinations. It cannot fire for SPA route changes (those always stay on
http/https), so it does not reintroduce issue law-chain-hot#25. Clearing logic is
factored into a shared clearTabConnections() helper.
@emavitta emavitta force-pushed the fix/spa-navigation-clears-connections branch from 535a7f9 to 01caba4 Compare May 14, 2026 15:08
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.

1 participant