From 0b667b18fcf631a25a64242a208705af12857e4c Mon Sep 17 00:00:00 2001 From: Jared Dellitt Date: Wed, 18 Feb 2026 18:08:06 -0600 Subject: [PATCH 1/6] chore(PLA-2224): re-fetch guides when the existing socket is disconnected and reconnected --- packages/client/src/clients/guide/client.ts | 27 +++- .../client/test/clients/guide/guide.test.ts | 123 ++++++++++++++++++ 2 files changed, 146 insertions(+), 4 deletions(-) diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index e458b3d8c..0768caaee 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -1,6 +1,6 @@ import { GenericData } from "@knocklabs/types"; import { Store } from "@tanstack/store"; -import { Channel, Socket } from "phoenix"; +import { Channel, type MessageRef, Socket } from "phoenix"; import { URLPattern } from "urlpattern-polyfill"; import Knock from "../../knock"; @@ -257,6 +257,7 @@ export class KnockGuideClient { "guide.live_preview_updated", ]; private subscribeRetryCount = 0; + private socketOpenListenerId: MessageRef | undefined; // Original history methods to monkey patch, or restore in cleanups. private pushStateFn: History["pushState"] | undefined; @@ -345,16 +346,16 @@ export class KnockGuideClient { this.clearCounterInterval(); } - async fetch(opts?: { filters?: QueryFilterParams }) { + async fetch(opts?: { filters?: QueryFilterParams; force?: boolean }) { this.knock.log("[Guide] .fetch"); this.knock.failIfNotAuthenticated(); const queryParams = this.buildQueryParams(opts?.filters); const queryKey = this.formatQueryKey(queryParams); - // If already fetched before, then noop. + // If already fetched before, then noop (unless force refetch). const maybeQueryStatus = this.store.state.queries[queryKey]; - if (maybeQueryStatus) { + if (maybeQueryStatus && !opts?.force) { return maybeQueryStatus; } @@ -455,6 +456,19 @@ export class KnockGuideClient { // Track the joined channel. this.socketChannel = newChannel; + + // Refetch guides on socket reconnect to pick up any changes that + // occurred while disconnected (e.g. tab was hidden for a long time). + // Skip the first onOpen since that's the initial connection, not a reconnect. + let isFirstOpen = true; + this.socketOpenListenerId = this.socket.onOpen(() => { + if (isFirstOpen) { + isFirstOpen = false; + return; + } + this.knock.log("[Guide] Socket reconnected, refetching guides"); + this.fetch({ force: true }); + }); } private handleChannelJoinError() { @@ -481,6 +495,11 @@ export class KnockGuideClient { } this.socketChannel.leave(); + if (this.socketOpenListenerId && this.socket) { + this.socket.off([this.socketOpenListenerId]); + this.socketOpenListenerId = undefined; + } + // Unset the channel. this.socketChannel = undefined; } diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index e68a710fd..b0da55123 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -371,6 +371,34 @@ describe("KnockGuideClient", () => { }); } }); + + test("skips fetch when query was already fetched", async () => { + vi.mocked(mockKnock.user.getGuides).mockResolvedValueOnce({ + entries: [], + }); + + const client = new KnockGuideClient(mockKnock, channelId); + await client.fetch(); + + vi.mocked(mockKnock.user.getGuides).mockClear(); + await client.fetch(); + + expect(mockKnock.user.getGuides).not.toHaveBeenCalled(); + }); + + test("re-fetches when force is true even if query was already fetched", async () => { + vi.mocked(mockKnock.user.getGuides).mockResolvedValue({ + entries: [], + }); + + const client = new KnockGuideClient(mockKnock, channelId); + await client.fetch(); + + vi.mocked(mockKnock.user.getGuides).mockClear(); + await client.fetch({ force: true }); + + expect(mockKnock.user.getGuides).toHaveBeenCalledOnce(); + }); }); describe("subscribe/unsubscribe", () => { @@ -393,6 +421,8 @@ describe("KnockGuideClient", () => { channel: vi.fn().mockReturnValue(mockChannel), isConnected: vi.fn().mockReturnValue(true), connect: vi.fn(), + onOpen: vi.fn().mockReturnValue("ref_1"), + off: vi.fn(), }; mockApiClient.socket = mockSocket as unknown as Socket; @@ -417,6 +447,93 @@ describe("KnockGuideClient", () => { expect(mockChannel.join).toHaveBeenCalled(); }); + test("refetches guides when socket reconnects", async () => { + let onOpenCallback: () => void; + const mockChannel = { + join: vi.fn().mockReturnValue({ + receive: vi.fn().mockReturnValue({ + receive: vi.fn().mockReturnValue({ + receive: vi.fn(), + }), + }), + }), + on: vi.fn(), + off: vi.fn(), + leave: vi.fn(), + state: "closed", + }; + + const mockSocket = { + channel: vi.fn().mockReturnValue(mockChannel), + isConnected: vi.fn().mockReturnValue(true), + connect: vi.fn(), + onOpen: vi.fn((cb) => { + onOpenCallback = cb; + return "ref_1"; + }), + off: vi.fn(), + }; + + mockApiClient.socket = mockSocket as unknown as Socket; + vi.mocked(mockKnock.client).mockReturnValue(mockApiClient as ApiClient); + vi.mocked(mockKnock.user.getGuides).mockResolvedValue({ entries: [] }); + + const client = new KnockGuideClient( + mockKnock, + channelId, + defaultTargetParams, + ); + + // Initial fetch to populate the query cache + await client.fetch(); + vi.mocked(mockKnock.user.getGuides).mockClear(); + + client.subscribe(); + + // First onOpen is the initial connection, should not refetch + onOpenCallback!(); + expect(mockKnock.user.getGuides).not.toHaveBeenCalled(); + + // Second onOpen is a reconnect, should refetch + onOpenCallback!(); + await vi.waitFor(() => { + expect(mockKnock.user.getGuides).toHaveBeenCalledOnce(); + }); + }); + + test("cleans up socket onOpen listener when unsubscribing", () => { + const mockChannel = { + join: vi.fn().mockReturnValue({ + receive: vi.fn().mockReturnValue({ + receive: vi.fn().mockReturnValue({ + receive: vi.fn(), + }), + }), + }), + on: vi.fn(), + off: vi.fn(), + leave: vi.fn(), + state: "closed", + }; + + const mockSocket = { + channel: vi.fn().mockReturnValue(mockChannel), + isConnected: vi.fn().mockReturnValue(true), + connect: vi.fn(), + onOpen: vi.fn().mockReturnValue("ref_1"), + off: vi.fn(), + }; + + mockApiClient.socket = mockSocket as unknown as Socket; + vi.mocked(mockKnock.client).mockReturnValue(mockApiClient as ApiClient); + + const client = new KnockGuideClient(mockKnock, channelId); + client.subscribe(); + client.unsubscribe(); + + expect(mockSocket.off).toHaveBeenCalledWith(["ref_1"]); + }); + test("handles successful channel join", () => { let okCallback: () => void; const mockChannel = { @@ -440,6 +557,8 @@ describe("KnockGuideClient", () => { channel: vi.fn().mockReturnValue(mockChannel), isConnected: vi.fn().mockReturnValue(true), connect: vi.fn(), + onOpen: vi.fn().mockReturnValue("ref_1"), + off: vi.fn(), }; mockApiClient.socket = mockSocket as unknown as Socket; @@ -495,6 +614,8 @@ describe("KnockGuideClient", () => { channel: vi.fn().mockReturnValue(mockChannel), isConnected: vi.fn().mockReturnValue(true), connect: vi.fn(), + onOpen: vi.fn().mockReturnValue("ref_1"), + off: vi.fn(), }; mockApiClient.socket = mockSocket as unknown as Socket; @@ -570,6 +691,8 @@ describe("KnockGuideClient", () => { channel: vi.fn().mockReturnValue(mockChannel), isConnected: vi.fn().mockReturnValue(true), connect: vi.fn(), + onOpen: vi.fn().mockReturnValue("ref_1"), + off: vi.fn(), }; mockApiClient.socket = mockSocket as unknown as Socket; From 327226ce681d27cf3b8765e06db5b120b46fa293 Mon Sep 17 00:00:00 2001 From: Jared Dellitt Date: Wed, 18 Feb 2026 18:11:27 -0600 Subject: [PATCH 2/6] chore(PLA-2224): simplify tests --- .../client/test/clients/guide/guide.test.ts | 88 +++++++------------ 1 file changed, 32 insertions(+), 56 deletions(-) diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index b0da55123..979a45e2a 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -150,6 +150,31 @@ describe("KnockGuideClient", () => { tenant: "tenant_123", }; + function createSubscribableMocks(socketOverrides: Record = {}) { + const mockChannel = { + join: vi.fn().mockReturnValue({ + receive: vi.fn().mockReturnValue({ + receive: vi.fn().mockReturnValue({ receive: vi.fn() }), + }), + }), + on: vi.fn(), + off: vi.fn(), + leave: vi.fn(), + state: "closed", + }; + + const mockSocket = { + channel: vi.fn().mockReturnValue(mockChannel), + isConnected: vi.fn().mockReturnValue(true), + connect: vi.fn(), + onOpen: vi.fn().mockReturnValue("listener_1"), + off: vi.fn(), + ...socketOverrides, + }; + + return { mockChannel, mockSocket }; + } + describe("constructor", () => { test("initializes with default options", () => { const client = new KnockGuideClient(mockKnock, channelId); @@ -447,54 +472,25 @@ describe("KnockGuideClient", () => { expect(mockChannel.join).toHaveBeenCalled(); }); - test("refetches guides when socket reconnects", async () => { + test("refetches guides on socket reconnect but not on initial connection", async () => { let onOpenCallback: () => void; - const mockChannel = { - join: vi.fn().mockReturnValue({ - receive: vi.fn().mockReturnValue({ - receive: vi.fn().mockReturnValue({ - receive: vi.fn(), - }), - }), - }), - on: vi.fn(), - off: vi.fn(), - leave: vi.fn(), - state: "closed", - }; - - const mockSocket = { - channel: vi.fn().mockReturnValue(mockChannel), - isConnected: vi.fn().mockReturnValue(true), - connect: vi.fn(), + const { mockSocket } = createSubscribableMocks({ onOpen: vi.fn((cb) => { onOpenCallback = cb; - return "ref_1"; + return "listener_1"; }), - off: vi.fn(), - }; + }); mockApiClient.socket = mockSocket as unknown as Socket; vi.mocked(mockKnock.client).mockReturnValue(mockApiClient as ApiClient); vi.mocked(mockKnock.user.getGuides).mockResolvedValue({ entries: [] }); - const client = new KnockGuideClient( - mockKnock, - channelId, - defaultTargetParams, - ); - - // Initial fetch to populate the query cache - await client.fetch(); - vi.mocked(mockKnock.user.getGuides).mockClear(); - + const client = new KnockGuideClient(mockKnock, channelId); client.subscribe(); - // First onOpen is the initial connection, should not refetch onOpenCallback!(); expect(mockKnock.user.getGuides).not.toHaveBeenCalled(); - // Second onOpen is a reconnect, should refetch onOpenCallback!(); await vi.waitFor(() => { expect(mockKnock.user.getGuides).toHaveBeenCalledOnce(); @@ -502,27 +498,7 @@ describe("KnockGuideClient", () => { }); test("cleans up socket onOpen listener when unsubscribing", () => { - const mockChannel = { - join: vi.fn().mockReturnValue({ - receive: vi.fn().mockReturnValue({ - receive: vi.fn().mockReturnValue({ - receive: vi.fn(), - }), - }), - }), - on: vi.fn(), - off: vi.fn(), - leave: vi.fn(), - state: "closed", - }; - - const mockSocket = { - channel: vi.fn().mockReturnValue(mockChannel), - isConnected: vi.fn().mockReturnValue(true), - connect: vi.fn(), - onOpen: vi.fn().mockReturnValue("ref_1"), - off: vi.fn(), - }; + const { mockSocket } = createSubscribableMocks(); mockApiClient.socket = mockSocket as unknown as Socket; vi.mocked(mockKnock.client).mockReturnValue(mockApiClient as ApiClient); @@ -531,7 +507,7 @@ describe("KnockGuideClient", () => { client.subscribe(); client.unsubscribe(); - expect(mockSocket.off).toHaveBeenCalledWith(["ref_1"]); + expect(mockSocket.off).toHaveBeenCalledWith(["listener_1"]); }); test("handles successful channel join", () => { From a2000c7c6dc47ffcac4c44b7d1bc9a0b41cffbc8 Mon Sep 17 00:00:00 2001 From: Jared Dellitt Date: Wed, 18 Feb 2026 18:19:19 -0600 Subject: [PATCH 3/6] chore(PLA-2224): remove an existing listener --- packages/client/src/clients/guide/client.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 0768caaee..9684b7475 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -417,6 +417,12 @@ export class KnockGuideClient { this.unsubscribe(); } + // If there's an existing open listener, then remove it. + if (this.socketOpenListenerId) { + this.socket.off([this.socketOpenListenerId]); + this.socketOpenListenerId = undefined; + } + // Join the channel topic and subscribe to supported events. const debugState = this.store.state.debug; const params = { @@ -1113,9 +1119,9 @@ export class KnockGuideClient { const guideGroupDisplayLogs = attrs.archived_at && !guide.bypass_global_group_limit ? { - ...state.guideGroupDisplayLogs, - [DEFAULT_GROUP_KEY]: attrs.archived_at, - } + ...state.guideGroupDisplayLogs, + [DEFAULT_GROUP_KEY]: attrs.archived_at, + } : state.guideGroupDisplayLogs; return { ...state, guides, guideGroupDisplayLogs }; From ad56c8c0f59aa214cd29fe85ad9f282ff0ca44d4 Mon Sep 17 00:00:00 2001 From: Jared Dellitt Date: Wed, 18 Feb 2026 18:25:14 -0600 Subject: [PATCH 4/6] chore(PLA-2224): add changeset entry --- .changeset/mighty-phones-run.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/mighty-phones-run.md diff --git a/.changeset/mighty-phones-run.md b/.changeset/mighty-phones-run.md new file mode 100644 index 000000000..b0ef18c12 --- /dev/null +++ b/.changeset/mighty-phones-run.md @@ -0,0 +1,5 @@ +--- +"@knocklabs/client": patch +--- + +Proactively re-fetches guides when the socket is reconnected From 13e8c3b7006a84c4e3e6dec27bdc371b3f466a5e Mon Sep 17 00:00:00 2001 From: Jared Dellitt Date: Wed, 18 Feb 2026 18:28:00 -0600 Subject: [PATCH 5/6] chore(PLA-2224): format --- packages/client/src/clients/guide/client.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 9684b7475..14bf3ac99 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -1119,9 +1119,9 @@ export class KnockGuideClient { const guideGroupDisplayLogs = attrs.archived_at && !guide.bypass_global_group_limit ? { - ...state.guideGroupDisplayLogs, - [DEFAULT_GROUP_KEY]: attrs.archived_at, - } + ...state.guideGroupDisplayLogs, + [DEFAULT_GROUP_KEY]: attrs.archived_at, + } : state.guideGroupDisplayLogs; return { ...state, guides, guideGroupDisplayLogs }; From 58d3def4536578d9f17ecbe0f111d75b6a24d3fc Mon Sep 17 00:00:00 2001 From: Jared Dellitt Date: Wed, 18 Feb 2026 18:39:02 -0600 Subject: [PATCH 6/6] chore(PLA-2224): correct isFirstOpen --- packages/client/src/clients/guide/client.ts | 6 ++-- .../client/test/clients/guide/guide.test.ts | 29 ++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 14bf3ac99..523336b7b 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -465,8 +465,10 @@ export class KnockGuideClient { // Refetch guides on socket reconnect to pick up any changes that // occurred while disconnected (e.g. tab was hidden for a long time). - // Skip the first onOpen since that's the initial connection, not a reconnect. - let isFirstOpen = true; + // If the socket is already connected, there's no initial open event coming, + // so we don't need to skip anything. If it's not yet connected, skip the + // first onOpen since that's the initial connection, not a reconnect. + let isFirstOpen = !this.socket.isConnected(); this.socketOpenListenerId = this.socket.onOpen(() => { if (isFirstOpen) { isFirstOpen = false; diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index 979a45e2a..1d80f3757 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -472,9 +472,10 @@ describe("KnockGuideClient", () => { expect(mockChannel.join).toHaveBeenCalled(); }); - test("refetches guides on socket reconnect but not on initial connection", async () => { + test("refetches guides on socket reconnect but not on initial connection when socket starts disconnected", async () => { let onOpenCallback: () => void; const { mockSocket } = createSubscribableMocks({ + isConnected: vi.fn().mockReturnValue(false), onOpen: vi.fn((cb) => { onOpenCallback = cb; return "listener_1"; @@ -488,9 +489,35 @@ describe("KnockGuideClient", () => { const client = new KnockGuideClient(mockKnock, channelId); client.subscribe(); + // First onOpen is the initial connection — should be skipped. onOpenCallback!(); expect(mockKnock.user.getGuides).not.toHaveBeenCalled(); + // Second onOpen is a reconnect — should trigger a refetch. + onOpenCallback!(); + await vi.waitFor(() => { + expect(mockKnock.user.getGuides).toHaveBeenCalledOnce(); + }); + }); + + test("refetches guides on first onOpen when socket is already connected at subscribe time", async () => { + let onOpenCallback: () => void; + const { mockSocket } = createSubscribableMocks({ + isConnected: vi.fn().mockReturnValue(true), + onOpen: vi.fn((cb) => { + onOpenCallback = cb; + return "listener_1"; + }), + }); + + mockApiClient.socket = mockSocket as unknown as Socket; + vi.mocked(mockKnock.client).mockReturnValue(mockApiClient as ApiClient); + vi.mocked(mockKnock.user.getGuides).mockResolvedValue({ entries: [] }); + + const client = new KnockGuideClient(mockKnock, channelId); + client.subscribe(); + + // Socket was already connected, so no initial open to skip — first onOpen is a reconnect. onOpenCallback!(); await vi.waitFor(() => { expect(mockKnock.user.getGuides).toHaveBeenCalledOnce();