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 diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index e458b3d8c..523336b7b 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; } @@ -416,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 = { @@ -455,6 +462,21 @@ 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). + // 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; + return; + } + this.knock.log("[Guide] Socket reconnected, refetching guides"); + this.fetch({ force: true }); + }); } private handleChannelJoinError() { @@ -481,6 +503,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..1d80f3757 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); @@ -371,6 +396,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 +446,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 +472,71 @@ describe("KnockGuideClient", () => { expect(mockChannel.join).toHaveBeenCalled(); }); + 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"; + }), + }); + + 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(); + + // 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(); + }); + }); + + test("cleans up socket onOpen listener when unsubscribing", () => { + const { mockSocket } = createSubscribableMocks(); + + 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(["listener_1"]); + }); + test("handles successful channel join", () => { let okCallback: () => void; const mockChannel = { @@ -440,6 +560,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 +617,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 +694,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;