Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .tool-versions
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
nodejs 24.12.0
nodejs 24.13.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated, upgrading to the same node version we are using in control.

yarn 1.22.22
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@
"vitest": "^3.1.1"
},
"engines": {
"node": ">=24.10.0"
"node": ">=24.13.0"
}
}
45 changes: 9 additions & 36 deletions packages/client/src/clients/guide/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,29 +157,11 @@ const select = (state: StoreState, filters: SelectFilterParams = {}) => {
const defaultGroup = findDefaultGroup(state.guideGroups);
if (!defaultGroup) return result;

const displaySequence = [...defaultGroup.display_sequence];
const displaySequence = defaultGroup.display_sequence;
const location = state.location;

// If in debug mode, put the forced guide at the beginning of the display sequence.
if (state.debug.forcedGuideKey) {
const forcedKeyIndex = displaySequence.indexOf(state.debug.forcedGuideKey);
Copy link

Choose a reason for hiding this comment

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

Preview guides not in display sequence cannot be shown

Low Severity

The removal of the displaySequence.unshift(state.debug.forcedGuideKey) logic means guides that exist only in previewGuides (but aren't yet in the display_sequence) can no longer be force-shown. Previously, the forced guide key was always added to the front of the display sequence copy, allowing iteration over keys not originally in the sequence. Now, only keys already in display_sequence are iterated, so a newly-created guide being previewed before publication to a group won't appear.

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is true, and I think we should move toward the behavior where only published guides can be rendered.

if (forcedKeyIndex > -1) {
displaySequence.splice(forcedKeyIndex, 1);
}
displaySequence.unshift(state.debug.forcedGuideKey);
}

for (const [index, guideKey] of displaySequence.entries()) {
let guide = state.guides[guideKey];

// Use preview guide if it exists and matches the forced guide key
if (
state.debug.forcedGuideKey === guideKey &&
state.previewGuides[guideKey]
) {
guide = state.previewGuides[guideKey];
}

const guide = state.previewGuides[guideKey] || state.guides[guideKey];
if (!guide) continue;

const affirmed = predicate(guide, {
Expand Down Expand Up @@ -215,11 +197,11 @@ const predicate = (
return false;
}

// Bypass filtering if the debugged guide matches the given filters.
// This should always run AFTER checking the filters but BEFORE
// checking archived status and location rules.
if (debug.forcedGuideKey === guide.key) {
return true;
// If in debug mode with a forced guide key, bypass other filtering and always
// return true for that guide only. This should always run AFTER checking the
// filters but BEFORE checking archived status and location rules.
if (debug.forcedGuideKey) {
return debug.forcedGuideKey === guide.key;
}

if (!guide.active) {
Expand Down Expand Up @@ -736,17 +718,8 @@ export class KnockGuideClient {
// callback to a setTimeout, but just to be safe.
this.ensureClearTimeout();

// If in debug mode, try to resolve the forced guide, otherwise return the first non-undefined guide.
let resolved = undefined;
if (this.store.state.debug.forcedGuideKey) {
resolved = this.stage.ordered.find(
(x) => x === this.store.state.debug.forcedGuideKey,
);
}

if (!resolved) {
resolved = this.stage.ordered.find((x) => x !== undefined);
}
// Resolve to the first non-undefined guide in the stage.
const resolved = this.stage.ordered.find((x) => x !== undefined);

this.knock.log(
`[Guide] Closing the current group stage: resolved=${resolved}`,
Expand Down
48 changes: 38 additions & 10 deletions packages/client/test/clients/guide/guide.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,7 @@ describe("KnockGuideClient", () => {
expect(result!.type).not.toBe("regular-type");
});

test("doesn't return the preview guide when filtered by a different key", () => {
test("doesn't return any guide when filtered by a different key than forced guide", () => {
const previewGuide = {
...mockGuideTwo,
type: "preview-type",
Expand Down Expand Up @@ -1791,10 +1791,12 @@ describe("KnockGuideClient", () => {
key: "onboarding",
});

expect(result!.key).toBe("onboarding");
// When forcedGuideKey is set, only that guide is considered, so filtering
// by a different key returns undefined
expect(result).toBeUndefined();
});

test("doesn't return the preview guide when filtered by a different type", () => {
test("doesn't return any guide when filtered by a different type than forced guide", () => {
const previewGuide = {
...mockGuideTwo,
type: "preview-type",
Expand Down Expand Up @@ -1827,7 +1829,9 @@ describe("KnockGuideClient", () => {
type: "banner",
});

expect(result!.key).toBe("system_status");
// When forcedGuideKey is set, only that guide is considered, so filtering
// by a different type returns undefined
expect(result).toBeUndefined();
});

test("does not return a guide inside a throttle window ", () => {
Expand Down Expand Up @@ -2131,7 +2135,7 @@ describe("KnockGuideClient", () => {
expect(result[0]!.key).toBe(mockGuideTwo.key);
});

test("does not return an inactive guide when forced guide key is set", () => {
test("returns the target guide even if inactive when forced guide key is set", () => {
const stateWithGuides = {
guideGroups: [mockDefaultGroup],
guideGroupDisplayLogs: {},
Expand All @@ -2147,17 +2151,41 @@ describe("KnockGuideClient", () => {
location: undefined,
counter: 0,
debug: {
forcedGuideKey: mockGuideThree.key,
forcedGuideKey: mockGuideTwo.key,
previewSessionId: "test-session-id",
},
};

const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuides"](stateWithGuides);
const result = client["_selectGuides"](stateWithGuides, {
key: mockGuideTwo.key
});

expect(result).toHaveLength(2);
expect(result[0]!.key).toBe(mockGuideThree.key);
expect(result[1]!.key).toBe(mockGuideOne.key);
expect(result[0]!.key).toBe(mockGuideTwo.key);
});

test("returns only the forced guide when forced guide key is set", () => {
const stateWithGuides = {
guideGroups: [mockDefaultGroup],
guideGroupDisplayLogs: {},
guides: mockGuides,
previewGuides: {},
queries: {},
location: undefined,
counter: 0,
debug: {
forcedGuideKey: mockGuideOne.key,
previewSessionId: "test-session-id",
},
};

const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuides"](stateWithGuides, {
type: "card"
});

expect(result).toHaveLength(1);
expect(result[0]!.key).toBe(mockGuideOne.key);
});

test("returns empty array when inside throttle window by default", () => {
Expand Down
Loading