Skip to content

Conversation

@thomaswhyyou
Copy link
Contributor

@thomaswhyyou thomaswhyyou commented Jan 13, 2026

Description

Updates the KnockGuideStep type as reported here: https://knocklabs.slack.com/archives/C079G6X3DD4/p1768330901136269?thread_ts=1767880973.264099&cid=C079G6X3DD4

The mark* methods are async so should wrap its return value in a promise.

@linear
Copy link

linear bot commented Jan 13, 2026

@changeset-bot
Copy link

changeset-bot bot commented Jan 13, 2026

🦋 Changeset detected

Latest commit: 37363d1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@knocklabs/client Patch
client-example Patch
guide-example Patch
@knocklabs/expo Patch
@knocklabs/react-core Patch
@knocklabs/react-native Patch
@knocklabs/react Patch
@knocklabs/expo-example Patch
ms-teams-connect-example Patch
nextjs-app-dir-example Patch
nextjs-example Patch
slack-connect-example Patch
slack-kit-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Jan 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
javascript-ms-teams-connect-example Ready Ready Preview, Comment Jan 13, 2026 10:10pm
javascript-nextjs-example Ready Ready Preview, Comment Jan 13, 2026 10:10pm
javascript-slack-connect-example Ready Ready Preview, Comment Jan 13, 2026 10:10pm
javascript-slack-kit-example Ready Ready Preview, Comment Jan 13, 2026 10:10pm

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@@ -1,2 +1,2 @@
nodejs 22.17.0
nodejs 24.12.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 but upgrading the nodejs to v24, similar to what we did in dashboard: https://github.com/knocklabs/control/pull/6801

message: { ...message },
markAsSeen() {
// Send a seen event if it has not been previously seen.
if (this.message.seen_at) return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

markAsSeen and markAsArchived class methods already check the same thing and noop if already marked as such. This allows the return value types to be cleaner. i.e.

// This
markAsSeen: () => Promise<KnockGuideStep<TContent> | undefined>;
// Instead of
markAsSeen: () => Promise<KnockGuideStep<TContent> | undefined> | undefined;

@thomaswhyyou thomaswhyyou marked this pull request as ready for review January 13, 2026 21:17
@thomaswhyyou thomaswhyyou requested a review from a team as a code owner January 13, 2026 21:17
@thomaswhyyou thomaswhyyou requested review from MikeCarbone, connorlindsey and mattmikolay and removed request for a team January 13, 2026 21:17
Copy link
Contributor

@MikeCarbone MikeCarbone left a comment

Choose a reason for hiding this comment

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

Looks good. Can't remember release process off the dome, make sure this doesn't require a version number bump / changeset / something else before merging 👍

@thomaswhyyou
Copy link
Contributor Author

Looks good. Can't remember release process off the dome, make sure this doesn't require a version number bump / changeset / something else before merging 👍

@MikeCarbone Yep just need a changeset to trigger the release pipeline.

@thomaswhyyou thomaswhyyou changed the title fix: KnockGuideStep type definition to return promise from async methodsd fix: KnockGuideStep type definition to return a promise from async methods Jan 13, 2026
@codecov
Copy link

codecov bot commented Jan 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.10%. Comparing base (bdcf188) to head (37363d1).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #821      +/-   ##
==========================================
+ Coverage   66.85%   67.10%   +0.24%     
==========================================
  Files         189      189              
  Lines        7911     7962      +51     
  Branches     1005     1024      +19     
==========================================
+ Hits         5289     5343      +54     
- Misses       2597     2599       +2     
+ Partials       25       20       -5     
Files with missing lines Coverage Δ
packages/client/src/clients/guide/client.ts 88.73% <ø> (+0.21%) ⬆️
packages/client/src/clients/guide/types.ts 100.00% <ø> (ø)

... and 12 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants