Skip to content

Commit ee35de2

Browse files
authored
chore: handle (re)subscribe in guide toolbar V2 when entering/exiting (#943)
1 parent ecd6454 commit ee35de2

6 files changed

Lines changed: 46 additions & 13 deletions

File tree

.changeset/tangy-tools-cross.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@knocklabs/react-core": patch
3+
"@knocklabs/client": patch
4+
"@knocklabs/react": patch
5+
---
6+
7+
[Guides] Only re-subscribe per listenForUpdates when exiting debugging in guide toolbar v2

packages/client/src/clients/guide/client.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -629,11 +629,12 @@ export class KnockGuideClient {
629629
`[Guide] Start debugging, refetching guides and resubscribing to the websocket channel`,
630630
);
631631
this.fetch({ force: true });
632+
// Always subscribe while debugging.
632633
this.subscribe();
633634
}
634635
}
635636

636-
unsetDebug() {
637+
unsetDebug(opts?: { listenForUpdates?: boolean }) {
637638
this.knock.log("[Guide] .unsetDebug()");
638639
const shouldRefetch = this.store.state.debug?.debugging;
639640

@@ -653,8 +654,11 @@ export class KnockGuideClient {
653654
`[Guide] Stop debugging, refetching guides and resubscribing to the websocket channel`,
654655
);
655656
this.fetch({ force: true });
656-
// TODO: Manage (re)subscribe/unsubscribe in V2 rather than here.
657-
this.subscribe();
657+
if (opts?.listenForUpdates) {
658+
this.subscribe();
659+
} else {
660+
this.unsubscribe();
661+
}
658662
}
659663
}
660664

packages/client/test/clients/guide/guide.test.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4851,17 +4851,13 @@ describe("KnockGuideClient", () => {
48514851
const fetchSpy = vi
48524852
.spyOn(client, "fetch")
48534853
.mockImplementation(() => Promise.resolve({ status: "ok" }));
4854-
const subscribeSpy = vi
4855-
.spyOn(client, "subscribe")
4856-
.mockImplementation(() => {});
48574854

48584855
client.unsetDebug();
48594856

48604857
expect(client.store.state.debug).toBe(undefined);
48614858

4862-
// calls fetch and subscribe when was debugging
4859+
// calls fetch when exiting debugging
48634860
expect(fetchSpy).toHaveBeenCalled();
4864-
expect(subscribeSpy).toHaveBeenCalled();
48654861
});
48664862

48674863
test("does not call fetch and subscribe when was not debugging", () => {

packages/react-core/test/guide/KnockGuideProvider.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,20 @@ describe("KnockGuideProvider", () => {
7878
expect(subscribeMock).toHaveBeenCalled();
7979
});
8080

81+
it("does not subscribe when listenForUpdates is false", () => {
82+
const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) =>
83+
React.createElement(
84+
KnockGuideProvider,
85+
{ channelId: "feed", readyToTarget: true, listenForUpdates: false },
86+
children,
87+
);
88+
89+
renderHook(() => useGuideContext(), { wrapper });
90+
91+
expect(fetchMock).toHaveBeenCalled();
92+
expect(subscribeMock).not.toHaveBeenCalled();
93+
});
94+
8195
it("defers fetch/subscribe to toolbar v2 when toolbar is visible", () => {
8296
getToolbarRunConfigFromUrlMock.mockReturnValue({ isVisible: true });
8397

packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,10 @@ const filterGuides = (
8787

8888
type Props = {
8989
readyToTarget: boolean;
90+
listenForUpdates: boolean;
9091
};
9192

92-
export const V2 = ({ readyToTarget }: Props) => {
93+
export const V2 = ({ readyToTarget, listenForUpdates }: Props) => {
9394
const { client } = useGuideContext();
9495

9596
const [displayOption, setDisplayOption] =
@@ -111,6 +112,7 @@ export const V2 = ({ readyToTarget }: Props) => {
111112
React.useEffect(() => {
112113
const { isVisible = false, focusedGuideKeys = {} } = runConfig || {};
113114
const isDebugging = client.store.state.debug?.debugging;
115+
114116
if (isVisible && !isDebugging && readyToTarget) {
115117
client.setDebug({ focusedGuideKeys });
116118

@@ -120,10 +122,16 @@ export const V2 = ({ readyToTarget }: Props) => {
120122
}
121123
}
122124

125+
if (!isVisible && isDebugging) {
126+
client.unsetDebug({ listenForUpdates });
127+
}
128+
123129
return () => {
124-
client.unsetDebug();
130+
if (client.store.state.debug?.debugging) {
131+
client.unsetDebug({ listenForUpdates });
132+
}
125133
};
126-
}, [readyToTarget, runConfig, client, setDisplayOption]);
134+
}, [readyToTarget, listenForUpdates, runConfig, client, setDisplayOption]);
127135

128136
// Toggle collapsed state with Ctrl + .
129137
React.useEffect(() => {
@@ -351,7 +359,6 @@ export const V2 = ({ readyToTarget }: Props) => {
351359
leadingIcon={{ icon: LogOut, alt: "Exit" }}
352360
onClick={() => {
353361
setRunConfig((curr) => ({ ...curr, isVisible: false }));
354-
client.unsetDebug();
355362
}}
356363
>
357364
Exit

packages/react/src/modules/guide/providers/KnockGuideProvider.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,23 @@ export const KnockGuideProvider: React.FC<React.PropsWithChildren<Props>> = ({
1717
children,
1818
toolbar = "v2",
1919
readyToTarget,
20+
listenForUpdates = true,
2021
...props
2122
}) => {
2223
return (
2324
<KnockGuideProviderCore
2425
{...props}
2526
readyToTarget={readyToTarget}
27+
listenForUpdates={listenForUpdates}
2628
// For backward compatibility with toolbar v1. Remove once v2 ships.
2729
trackDebugParams={toolbar === "v1"}
2830
>
2931
{children}
3032
{toolbar === "v2" ? (
31-
<ToolbarV2 readyToTarget={readyToTarget} />
33+
<ToolbarV2
34+
readyToTarget={readyToTarget}
35+
listenForUpdates={listenForUpdates}
36+
/>
3237
) : (
3338
<ToolbarV1 />
3439
)}

0 commit comments

Comments
 (0)