From 3ca2887ec3c4189a6e2e8ed4e8baef35f02aead5 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 9 Apr 2026 13:43:27 -0400 Subject: [PATCH 1/6] use readyToTarget to coordinate the first fetch in guide toolbar V2 --- packages/client/src/clients/guide/client.ts | 18 +++++ packages/client/src/clients/guide/helpers.ts | 38 +++++++++ packages/client/src/clients/guide/index.ts | 1 + .../client/test/clients/guide/helpers.test.ts | 58 +++++++++++++- .../guide/context/KnockGuideProvider.tsx | 6 +- .../test/guide/KnockGuideProvider.test.tsx | 1 + .../guide/components/Toolbar/V2/V2.tsx | 17 ++-- .../guide/components/Toolbar/V2/helpers.ts | 42 ---------- .../Toolbar/V2/useInspectGuideClientStore.ts | 3 +- .../guide/providers/KnockGuideProvider.tsx | 8 +- .../test/guide/Toolbar/V2/helpers.test.ts | 78 ------------------- 11 files changed, 141 insertions(+), 129 deletions(-) delete mode 100644 packages/react/test/guide/Toolbar/V2/helpers.test.ts diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 635fd7095..55cbb16e5 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -13,6 +13,7 @@ import { formatFilters, formatGroupStage, formatState, + getToolbarRunConfigFromUrl, mockDefaultGroup, newUrl, predicateUrlPatterns, @@ -598,8 +599,15 @@ export class KnockGuideClient { } } + // TODO: Split setDebug into startDebug vs updateDebug. setDebug(debugOpts?: Omit) { this.knock.log("[Guide] .setDebug()"); + + if (!this.knock.isAuthenticated()) { + this.knock.log("[Guide] Not authenticated, cannot start debugging"); + return; + } + const shouldRefetch = !this.store.state.debug?.debugging; // Clear the stage before updating the store state to let the next render @@ -636,11 +644,17 @@ export class KnockGuideClient { this.store.setState((state) => ({ ...state, debug: undefined })); + if (!this.knock.isAuthenticated()) { + this.knock.log("[Guide] Not authenticated, will not refetch"); + return; + } + if (shouldRefetch) { this.knock.log( `[Guide] Stop debugging, refetching guides and resubscribing to the websocket channel`, ); this.fetch({ force: true }); + // TODO: Manage (re)subscribe/unsubscribe in V2 rather than here. this.subscribe(); } } @@ -1480,4 +1494,8 @@ export class KnockGuideClient { this.replaceStateFn = undefined; } } + + static getToolbarRunConfigFromUrl() { + return getToolbarRunConfigFromUrl(); + } } diff --git a/packages/client/src/clients/guide/helpers.ts b/packages/client/src/clients/guide/helpers.ts index 7e163dcd2..1858b346e 100644 --- a/packages/client/src/clients/guide/helpers.ts +++ b/packages/client/src/clients/guide/helpers.ts @@ -3,6 +3,7 @@ import { GuideActivationUrlRuleData, GuideData, GuideGroupData, + KnockGuide, KnockGuideActivationUrlPattern, SelectFilterParams, StoreState, @@ -225,3 +226,40 @@ export const predicateUrlPatterns = ( } }, predicateDefault); }; + +// Use this param to start Toolbar v2 and enter into a debugging session when +// it is present and set to true. +const TOOLBAR_QUERY_PARAM = "knock_guide_toolbar"; + +// Optional, when present pin/focus on this guide. +const GUIDE_KEY_PARAM = "focused_guide_key"; + +export type ToolbarV2RunConfig = { + isVisible: boolean; + focusedGuideKeys?: Record; +}; + +export const getToolbarRunConfigFromUrl = (): ToolbarV2RunConfig => { + const fallback = { isVisible: false }; + + if (typeof window === "undefined" || !window.location) { + return fallback; + } + + const urlSearchParams = new URLSearchParams(window.location.search); + const toolbarParamValue = urlSearchParams.get(TOOLBAR_QUERY_PARAM); + const guideKeyParamValue = urlSearchParams.get(GUIDE_KEY_PARAM); + + if (toolbarParamValue === null) { + return fallback; + } + + const config: ToolbarV2RunConfig = { + isVisible: toolbarParamValue === "true", + }; + if (guideKeyParamValue) { + config.focusedGuideKeys = { [guideKeyParamValue]: true }; + } + + return config; +}; diff --git a/packages/client/src/clients/guide/index.ts b/packages/client/src/clients/guide/index.ts index effa92f9d..0834dd155 100644 --- a/packages/client/src/clients/guide/index.ts +++ b/packages/client/src/clients/guide/index.ts @@ -4,6 +4,7 @@ export { checkActivatable, } from "./client"; export { checkStateIfThrottled } from "./helpers"; +export type { ToolbarV2RunConfig } from "./helpers"; export type { KnockGuide, KnockGuideStep, diff --git a/packages/client/test/clients/guide/helpers.test.ts b/packages/client/test/clients/guide/helpers.test.ts index ed9a875f6..b7b932eb3 100644 --- a/packages/client/test/clients/guide/helpers.test.ts +++ b/packages/client/test/clients/guide/helpers.test.ts @@ -1,8 +1,9 @@ -import { describe, expect, test } from "vitest"; +import { describe, expect, test, vi, afterEach } from "vitest"; import { URLPattern } from "urlpattern-polyfill"; import { evaluateUrlRule, + getToolbarRunConfigFromUrl, predicateUrlRules, predicateUrlPatterns, } from "../../../src/clients/guide/helpers"; @@ -679,3 +680,58 @@ describe("predicateUrlPatterns", () => { }); }); }); + +describe("getToolbarRunConfigFromUrl", () => { + const stubWindow = (search: string) => { + vi.stubGlobal("window", { location: { search } }); + }; + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + test("returns isVisible true when URL param is 'true'", () => { + stubWindow("?knock_guide_toolbar=true"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: true }); + }); + + test("returns isVisible false when URL param is 'false'", () => { + stubWindow("?knock_guide_toolbar=false"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: false }); + }); + + test("returns default config when URL param is absent", () => { + stubWindow(""); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: false }); + }); + + test("includes focusedGuideKeys when focused_guide_key URL param is present", () => { + stubWindow("?knock_guide_toolbar=true&focused_guide_key=my_guide"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ + isVisible: true, + focusedGuideKeys: { my_guide: true }, + }); + }); + + test("does not include focusedGuideKeys when focused_guide_key param is absent", () => { + stubWindow("?knock_guide_toolbar=true"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: true }); + expect(config).not.toHaveProperty("focusedGuideKeys"); + }); +}); diff --git a/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx b/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx index b69f3ba33..17594e70e 100644 --- a/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx +++ b/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx @@ -75,7 +75,11 @@ export const KnockGuideProvider: React.FC< ]); React.useEffect(() => { - if (readyToTarget) { + // When the toolbar v2 is visible, defer fetch/subscribe to the toolbar + // so it can drive the debugging session lifecycle. + const toolbarRunConfig = KnockGuideClient.getToolbarRunConfigFromUrl(); + + if (readyToTarget && !toolbarRunConfig.isVisible) { knockGuideClient.fetch(); if (listenForUpdates) knockGuideClient.subscribe(); } diff --git a/packages/react-core/test/guide/KnockGuideProvider.test.tsx b/packages/react-core/test/guide/KnockGuideProvider.test.tsx index 9dc45fae0..590037859 100644 --- a/packages/react-core/test/guide/KnockGuideProvider.test.tsx +++ b/packages/react-core/test/guide/KnockGuideProvider.test.tsx @@ -37,6 +37,7 @@ vi.mock("@knocklabs/client", () => { cleanupMock = vi.fn(); MockKnockGuideClient = class { + static getToolbarRunConfigFromUrl = () => ({ isVisible: false }); fetch = fetchMock; subscribe = subscribeMock; cleanup = cleanupMock; diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx index 46d1a0d2a..cc29800a9 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx +++ b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx @@ -1,3 +1,4 @@ +import { KnockGuideClient } from "@knocklabs/client"; import { useGuideContext } from "@knocklabs/react-core"; import { Button } from "@telegraph/button"; import { Icon } from "@telegraph/icon"; @@ -22,7 +23,7 @@ import "../styles.css"; import { FocusChin } from "./FocusChin"; import { GuideContextDetails } from "./GuideContextDetails"; import { GuideRow } from "./GuideRow"; -import { DisplayOption, getRunConfig, sharedTooltipProps } from "./helpers"; +import { DisplayOption, sharedTooltipProps } from "./helpers"; import { useDraggable } from "./useDraggable"; import { InspectionResultOk, @@ -84,12 +85,18 @@ const filterGuides = ( }); }; -export const V2 = () => { +type Props = { + readyToTarget: boolean; +}; + +export const V2 = ({ readyToTarget }: Props) => { const { client } = useGuideContext(); const [displayOption, setDisplayOption] = React.useState("only-active"); - const [runConfig, setRunConfig] = React.useState(() => getRunConfig()); + const [runConfig, setRunConfig] = React.useState(() => + KnockGuideClient.getToolbarRunConfigFromUrl(), + ); const [isCollapsed, setIsCollapsed] = React.useState(false); const [isContextPanelOpen, setIsContextPanelOpen] = React.useState(false); @@ -104,7 +111,7 @@ export const V2 = () => { React.useEffect(() => { const { isVisible = false, focusedGuideKeys = {} } = runConfig || {}; const isDebugging = client.store.state.debug?.debugging; - if (isVisible && !isDebugging) { + if (readyToTarget && isVisible && !isDebugging) { client.setDebug({ focusedGuideKeys }); // If focused, switch to all guides so you can see in the list. @@ -116,7 +123,7 @@ export const V2 = () => { return () => { client.unsetDebug(); }; - }, [runConfig, client, setDisplayOption]); + }, [readyToTarget, runConfig, client, setDisplayOption]); // Toggle collapsed state with Ctrl + . React.useEffect(() => { diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts b/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts index da805edb1..9097b9bef 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts +++ b/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts @@ -1,51 +1,9 @@ -import { KnockGuide } from "@knocklabs/client"; - -import { checkForWindow } from "../../../../../modules/core"; - export const sharedTooltipProps = { delayDuration: 1000, }; export type DisplayOption = "all-guides" | "only-active" | "only-eligible"; -// Use this param to start Toolbar and enter into a debugging session when -// it is present and set to true. -const TOOLBAR_QUERY_PARAM = "knock_guide_toolbar"; - -// Optional, when present pin/focus on this guide. -const GUIDE_KEY_PARAM = "focused_guide_key"; - -export type ToolbarV2RunConfig = { - isVisible: boolean; - focusedGuideKeys?: Record; -}; - -export const getRunConfig = (): ToolbarV2RunConfig => { - const fallback = { isVisible: false }; - - const win = checkForWindow(); - if (!win || !win.location) { - return fallback; - } - - const urlSearchParams = new URLSearchParams(win.location.search); - const toolbarParamValue = urlSearchParams.get(TOOLBAR_QUERY_PARAM); - const guideKeyParamValue = urlSearchParams.get(GUIDE_KEY_PARAM); - - if (toolbarParamValue === null) { - return fallback; - } - - const config: ToolbarV2RunConfig = { - isVisible: toolbarParamValue === "true", - }; - if (guideKeyParamValue) { - config.focusedGuideKeys = { [guideKeyParamValue]: true }; - } - - return config; -}; - export const FOCUS_ERRORS = { focusUnknownGuide: "No such guide exists", focusUncommittedGuide: "This guide has not been committed", diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts b/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts index c9c044ef3..8622dcfdb 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts +++ b/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts @@ -4,12 +4,13 @@ import { KnockGuideClientStoreState, KnockGuideIneligibilityMarker, KnockGuideSelectionResult, + type ToolbarV2RunConfig, checkActivatable, checkStateIfThrottled, } from "@knocklabs/client"; import { useGuideContext, useStore } from "@knocklabs/react-core"; -import { FOCUS_ERRORS, ToolbarV2RunConfig } from "./helpers"; +import { FOCUS_ERRORS } from "./helpers"; const byKey = (items: T[]) => { return items.reduce((acc, item) => ({ ...acc, [item.key]: item }), {}); diff --git a/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx b/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx index 7ef57306a..0d8e26e59 100644 --- a/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx +++ b/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx @@ -16,16 +16,22 @@ type Props = KnockGuideProviderProps & { export const KnockGuideProvider: React.FC> = ({ children, toolbar = "v2", + readyToTarget, ...props }) => { return ( {children} - {toolbar === "v2" ? : } + {toolbar === "v2" ? ( + + ) : ( + + )} ); }; diff --git a/packages/react/test/guide/Toolbar/V2/helpers.test.ts b/packages/react/test/guide/Toolbar/V2/helpers.test.ts deleted file mode 100644 index d8908c831..000000000 --- a/packages/react/test/guide/Toolbar/V2/helpers.test.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { describe, expect, test, vi, afterEach } from "vitest"; - -import { getRunConfig } from "../../../../src/modules/guide/components/Toolbar/V2/helpers"; - -describe("Toolbar V2 helpers", () => { - afterEach(() => { - vi.unstubAllGlobals(); - vi.restoreAllMocks(); - }); - - describe("getRunConfig", () => { - test("returns isVisible true when URL param is 'true'", () => { - Object.defineProperty(window, "location", { - value: { search: "?knock_guide_toolbar=true" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: true }); - }); - - test("returns isVisible false when URL param is 'false'", () => { - Object.defineProperty(window, "location", { - value: { search: "?knock_guide_toolbar=false" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: false }); - }); - - test("returns default config when URL param is absent", () => { - Object.defineProperty(window, "location", { - value: { search: "" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: false }); - }); - - test("includes focusedGuideKeys when focused_guide_key URL param is present", () => { - Object.defineProperty(window, "location", { - value: { - search: "?knock_guide_toolbar=true&focused_guide_key=my_guide", - }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ - isVisible: true, - focusedGuideKeys: { my_guide: true }, - }); - }); - - test("does not include focusedGuideKeys when focused_guide_key param is absent", () => { - Object.defineProperty(window, "location", { - value: { search: "?knock_guide_toolbar=true" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: true }); - expect(config).not.toHaveProperty("focusedGuideKeys"); - }); - }); -}); From a886b8b83679f706d5778ff170c67658c9144ed0 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 9 Apr 2026 13:51:28 -0400 Subject: [PATCH 2/6] add tests --- .../client/test/clients/guide/guide.test.ts | 35 +++++++++++++++++++ .../test/guide/KnockGuideProvider.test.tsx | 31 ++++++++++++++-- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index 770f1c157..78424c164 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -4806,6 +4806,21 @@ describe("KnockGuideClient", () => { expect(client.store.state.debug!.ignoreDisplayInterval).toBe(false); }); + + test("returns early without setting debug when not authenticated", () => { + vi.mocked(mockKnock.isAuthenticated).mockReturnValue(false); + const client = new KnockGuideClient(mockKnock, channelId); + client.store.state.debug = undefined; + + const fetchSpy = vi + .spyOn(client, "fetch") + .mockImplementation(() => Promise.resolve({ status: "ok" })); + + client.setDebug(); + + expect(client.store.state.debug).toBe(undefined); + expect(fetchSpy).not.toHaveBeenCalled(); + }); }); describe("unsetDebug", () => { @@ -4868,6 +4883,26 @@ describe("KnockGuideClient", () => { expect(subscribeSpy).not.toHaveBeenCalled(); }); + test("clears debug state but does not fetch when not authenticated", () => { + const client = new KnockGuideClient(mockKnock, channelId); + client.store.state.debug = { debugging: true }; + + vi.mocked(mockKnock.isAuthenticated).mockReturnValue(false); + + const fetchSpy = vi + .spyOn(client, "fetch") + .mockImplementation(() => Promise.resolve({ status: "ok" })); + const subscribeSpy = vi + .spyOn(client, "subscribe") + .mockImplementation(() => {}); + + client.unsetDebug(); + + expect(client.store.state.debug).toBe(undefined); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(subscribeSpy).not.toHaveBeenCalled(); + }); + test("does not call fetch and subscribe when debug exists but debugging is false", () => { const client = new KnockGuideClient(mockKnock, channelId); client.store.state.debug = { debugging: false }; diff --git a/packages/react-core/test/guide/KnockGuideProvider.test.tsx b/packages/react-core/test/guide/KnockGuideProvider.test.tsx index 590037859..ed60add24 100644 --- a/packages/react-core/test/guide/KnockGuideProvider.test.tsx +++ b/packages/react-core/test/guide/KnockGuideProvider.test.tsx @@ -1,6 +1,6 @@ import { renderHook } from "@testing-library/react"; import React from "react"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { KnockGuideProvider } from "../../src/modules/guide/context/KnockGuideProvider"; import { useGuideContext } from "../../src/modules/guide/hooks/useGuideContext"; @@ -29,6 +29,8 @@ var subscribeMock: ReturnType; // eslint-disable-next-line no-var var cleanupMock: ReturnType; // eslint-disable-next-line no-var +var getToolbarRunConfigFromUrlMock: ReturnType; +// eslint-disable-next-line no-var var MockKnockGuideClient: new () => unknown; vi.mock("@knocklabs/client", () => { @@ -36,8 +38,10 @@ vi.mock("@knocklabs/client", () => { subscribeMock = vi.fn(); cleanupMock = vi.fn(); + getToolbarRunConfigFromUrlMock = vi.fn(() => ({ isVisible: false })); + MockKnockGuideClient = class { - static getToolbarRunConfigFromUrl = () => ({ isVisible: false }); + static getToolbarRunConfigFromUrl = getToolbarRunConfigFromUrlMock; fetch = fetchMock; subscribe = subscribeMock; cleanup = cleanupMock; @@ -51,6 +55,13 @@ describe("KnockGuideProvider", () => { // Deliberately omitting negative-case test that asserts throwing without KnockProvider // as React swallows errors thrown during rendering, making the assertion unreliable + beforeEach(() => { + fetchMock.mockClear(); + subscribeMock.mockClear(); + cleanupMock.mockClear(); + getToolbarRunConfigFromUrlMock.mockReturnValue({ isVisible: false }); + }); + it("provides context and triggers fetch/subscribe when ready", () => { const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => React.createElement( @@ -66,4 +77,20 @@ describe("KnockGuideProvider", () => { expect(fetchMock).toHaveBeenCalled(); expect(subscribeMock).toHaveBeenCalled(); }); + + it("defers fetch/subscribe to toolbar v2 when toolbar is visible", () => { + getToolbarRunConfigFromUrlMock.mockReturnValue({ isVisible: true }); + + const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => + React.createElement( + KnockGuideProvider, + { channelId: "feed", readyToTarget: true }, + children, + ); + + renderHook(() => useGuideContext(), { wrapper }); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(subscribeMock).not.toHaveBeenCalled(); + }); }); From 63453ddf01e3b0b924c1499f8d155c95a1e3d07b Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 9 Apr 2026 15:38:45 -0400 Subject: [PATCH 3/6] changeset --- .changeset/heavy-donkeys-greet.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/heavy-donkeys-greet.md diff --git a/.changeset/heavy-donkeys-greet.md b/.changeset/heavy-donkeys-greet.md new file mode 100644 index 000000000..9d4279ee0 --- /dev/null +++ b/.changeset/heavy-donkeys-greet.md @@ -0,0 +1,7 @@ +--- +"@knocklabs/react-core": patch +"@knocklabs/client": patch +"@knocklabs/react": patch +--- + +[Guides] Use `readyToTarget` to coordinate fetching guides in guide toolbar v2 From 43c0e12b66febb97df61ad176195008355ec14be Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 9 Apr 2026 15:43:39 -0400 Subject: [PATCH 4/6] clean up --- packages/client/src/clients/guide/client.ts | 1 - packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 55cbb16e5..ee5f8c372 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -599,7 +599,6 @@ export class KnockGuideClient { } } - // TODO: Split setDebug into startDebug vs updateDebug. setDebug(debugOpts?: Omit) { this.knock.log("[Guide] .setDebug()"); diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx index cc29800a9..da8159037 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx +++ b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx @@ -111,7 +111,7 @@ export const V2 = ({ readyToTarget }: Props) => { React.useEffect(() => { const { isVisible = false, focusedGuideKeys = {} } = runConfig || {}; const isDebugging = client.store.state.debug?.debugging; - if (readyToTarget && isVisible && !isDebugging) { + if (isVisible && !isDebugging && readyToTarget) { client.setDebug({ focusedGuideKeys }); // If focused, switch to all guides so you can see in the list. From dd33c556f2e69b3938ccbc12aeb7589d9695da2b Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 9 Apr 2026 15:06:50 -0400 Subject: [PATCH 5/6] handle (re)subscribe in guide toolbar V2 when stop debugging --- packages/client/src/clients/guide/client.ts | 3 --- .../guide/components/Toolbar/V2/V2.tsx | 22 ++++++++++++++----- .../guide/providers/KnockGuideProvider.tsx | 7 +++++- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index ee5f8c372..ed107effc 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -629,7 +629,6 @@ export class KnockGuideClient { `[Guide] Start debugging, refetching guides and resubscribing to the websocket channel`, ); this.fetch({ force: true }); - this.subscribe(); } } @@ -653,8 +652,6 @@ export class KnockGuideClient { `[Guide] Stop debugging, refetching guides and resubscribing to the websocket channel`, ); this.fetch({ force: true }); - // TODO: Manage (re)subscribe/unsubscribe in V2 rather than here. - this.subscribe(); } } diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx index da8159037..65b1cd651 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx +++ b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx @@ -87,9 +87,10 @@ const filterGuides = ( type Props = { readyToTarget: boolean; + listenForUpdates: boolean; }; -export const V2 = ({ readyToTarget }: Props) => { +export const V2 = ({ readyToTarget, listenForUpdates }: Props) => { const { client } = useGuideContext(); const [displayOption, setDisplayOption] = @@ -111,8 +112,16 @@ export const V2 = ({ readyToTarget }: Props) => { React.useEffect(() => { const { isVisible = false, focusedGuideKeys = {} } = runConfig || {}; const isDebugging = client.store.state.debug?.debugging; + + const stopDebug = () => { + client.unsetDebug(); + listenForUpdates ? client.subscribe() : client.unsubscribe(); + }; + + // Always subscribe while debugging. if (isVisible && !isDebugging && readyToTarget) { client.setDebug({ focusedGuideKeys }); + client.subscribe(); // If focused, switch to all guides so you can see in the list. if (Object.keys(focusedGuideKeys).length > 0) { @@ -120,10 +129,12 @@ export const V2 = ({ readyToTarget }: Props) => { } } - return () => { - client.unsetDebug(); - }; - }, [readyToTarget, runConfig, client, setDisplayOption]); + if (!isVisible && isDebugging) { + stopDebug(); + } + + return () => stopDebug(); + }, [readyToTarget, listenForUpdates, runConfig, client, setDisplayOption]); // Toggle collapsed state with Ctrl + . React.useEffect(() => { @@ -351,7 +362,6 @@ export const V2 = ({ readyToTarget }: Props) => { leadingIcon={{ icon: LogOut, alt: "Exit" }} onClick={() => { setRunConfig((curr) => ({ ...curr, isVisible: false })); - client.unsetDebug(); }} > Exit diff --git a/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx b/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx index 0d8e26e59..05bccb40d 100644 --- a/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx +++ b/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx @@ -17,18 +17,23 @@ export const KnockGuideProvider: React.FC> = ({ children, toolbar = "v2", readyToTarget, + listenForUpdates = true, ...props }) => { return ( {children} {toolbar === "v2" ? ( - + ) : ( )} From 63df698cebb61139a7bc9dc7610a88afd9872495 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 9 Apr 2026 15:12:26 -0400 Subject: [PATCH 6/6] update tests --- packages/client/test/clients/guide/guide.test.ts | 8 ++++---- .../test/guide/KnockGuideProvider.test.tsx | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index 78424c164..81ef8cdb3 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -4714,9 +4714,9 @@ describe("KnockGuideClient", () => { expect(client.store.state.debug!.debugging!).toBe(true); - // calls fetch and subscribe when not already debugging + // calls fetch when not already debugging; subscribe is managed by V2 toolbar expect(fetchSpy).toHaveBeenCalled(); - expect(subscribeSpy).toHaveBeenCalled(); + expect(subscribeSpy).not.toHaveBeenCalled(); }); test("sets debug state with provided options", () => { @@ -4859,9 +4859,9 @@ describe("KnockGuideClient", () => { expect(client.store.state.debug).toBe(undefined); - // calls fetch and subscribe when was debugging + // calls fetch when was debugging; subscribe is managed by V2 toolbar expect(fetchSpy).toHaveBeenCalled(); - expect(subscribeSpy).toHaveBeenCalled(); + expect(subscribeSpy).not.toHaveBeenCalled(); }); test("does not call fetch and subscribe when was not debugging", () => { diff --git a/packages/react-core/test/guide/KnockGuideProvider.test.tsx b/packages/react-core/test/guide/KnockGuideProvider.test.tsx index ed60add24..899342712 100644 --- a/packages/react-core/test/guide/KnockGuideProvider.test.tsx +++ b/packages/react-core/test/guide/KnockGuideProvider.test.tsx @@ -78,6 +78,20 @@ describe("KnockGuideProvider", () => { expect(subscribeMock).toHaveBeenCalled(); }); + it("does not subscribe when listenForUpdates is false", () => { + const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => + React.createElement( + KnockGuideProvider, + { channelId: "feed", readyToTarget: true, listenForUpdates: false }, + children, + ); + + renderHook(() => useGuideContext(), { wrapper }); + + expect(fetchMock).toHaveBeenCalled(); + expect(subscribeMock).not.toHaveBeenCalled(); + }); + it("defers fetch/subscribe to toolbar v2 when toolbar is visible", () => { getToolbarRunConfigFromUrlMock.mockReturnValue({ isVisible: true });