Skip to content

Conversation

@thomaswhyyou
Copy link
Contributor

@thomaswhyyou thomaswhyyou commented Oct 24, 2025

Description

Adds a couple Next js specific helper components for detecting location changes for the guide client, rather than auto detecting from the window object, so activation evaluations can run properly.

We've observed the latter (default) not working in our dashboard in prod currently, even though it works fine when running locally. It appears that Next js is doing something differently in production builds, where it's potentially caching history methods early in production (for optimizations?) and bypassing the monkey patching (pushState/replaceState) we are doing on the window object during a guide client initialization.

These specific helpers are meant to be used inside the KnockGuideProvider, which calls setLocation on the provided client using the nextjs router object. For example:

<KnockProvider>
  <KnockGuideProvider
    trackLocationFromWindow={false}
  >
    {children}
    <KnockGuideLocationSensorNextPagesRouter />
  </KnockGuideProvider>
</KnockProvider>

Note

Adds Next.js App/Pages Router LocationSensor components and updates the guide client to explicitly remove window-based listeners, with exports and peer deps adjusted.

  • Guides (React):
    • New: LocationSensorNextAppRouter and LocationSensorNextPagesRouter components to track route changes via Next.js routers and call client.setLocation.
    • Exports: Re-exported as KnockGuideLocationSensorNextAppRouter and KnockGuideLocationSensorNextPagesRouter from packages/react/src/index.ts and guide module indexes.
  • Guide Client (Core):
    • Renames cleanup method to removeLocationChangeEventListeners() and uses it in cleanup().
    • Adds log in handleLocationChange; maintains and restores history patches when removing listeners.
  • Dependencies:
    • Adds optional next as a peer dependency and includes it in devDependencies for @knocklabs/react.

Written by Cursor Bugbot for commit d795cc6. This will update automatically on new commits. Configure here.

@linear
Copy link

linear bot commented Oct 24, 2025

@vercel
Copy link

vercel bot commented Oct 24, 2025

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

Project Deployment Preview Comments Updated (UTC)
javascript-ms-teams-connect-example Ready Ready Preview Comment Oct 28, 2025 9:42pm
javascript-nextjs-example Ready Ready Preview Comment Oct 28, 2025 9:42pm
javascript-slack-connect-example Ready Ready Preview Comment Oct 28, 2025 9:42pm
javascript-slack-kit-example Ready Ready Preview Comment Oct 28, 2025 9:42pm

@changeset-bot
Copy link

changeset-bot bot commented Oct 24, 2025

🦋 Changeset detected

Latest commit: d795cc6

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
@knocklabs/react Patch
client-example Patch
guide-example Patch
@knocklabs/expo Patch
@knocklabs/react-core Patch
@knocklabs/react-native Patch
ms-teams-connect-example Patch
nextjs-app-dir-example Patch
nextjs-example Patch
slack-connect-example Patch
slack-kit-example Patch
@knocklabs/expo-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

Copy link
Contributor Author

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

@thomaswhyyou thomaswhyyou changed the title add LocationSensor for guide feat: add LocationSensor helper component for next AppRouter and PagesRouter Oct 24, 2025
@thomaswhyyou thomaswhyyou marked this pull request as ready for review October 24, 2025 22:23
@thomaswhyyou thomaswhyyou requested a review from a team as a code owner October 24, 2025 22:23
@thomaswhyyou thomaswhyyou requested review from connorlindsey and meryldakin and removed request for a team October 24, 2025 22:23
@thomaswhyyou thomaswhyyou requested a review from cjbell October 24, 2025 22:46
@thomaswhyyou thomaswhyyou force-pushed the thomas-kno-10209-sdk-activation-rules-are-not-being-re-evaluated-on-location branch from aaa9042 to 9640193 Compare October 27, 2025 15:29
cursor[bot]

This comment was marked as outdated.

@thomaswhyyou thomaswhyyou force-pushed the thomas-kno-10209-sdk-activation-rules-are-not-being-re-evaluated-on-location branch from d9253b5 to d795cc6 Compare October 28, 2025 21:40
@thomaswhyyou thomaswhyyou merged commit 4b888c4 into main Oct 28, 2025
11 of 13 checks passed
@thomaswhyyou thomaswhyyou deleted the thomas-kno-10209-sdk-activation-rules-are-not-being-re-evaluated-on-location branch October 28, 2025 22:41
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 32.65306% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.84%. Comparing base (be65601) to head (d795cc6).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...uide/components/LocationSensor/NextPagesRouter.tsx 20.83% 19 Missing ⚠️
.../guide/components/LocationSensor/NextAppRouter.tsx 26.31% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #789      +/-   ##
==========================================
- Coverage   66.06%   65.84%   -0.22%     
==========================================
  Files         185      188       +3     
  Lines        7706     7753      +47     
  Branches      950      950              
==========================================
+ Hits         5091     5105      +14     
- Misses       2591     2624      +33     
  Partials       24       24              
Files with missing lines Coverage Δ
packages/client/src/clients/guide/client.ts 87.23% <100.00%> (+0.01%) ⬆️
packages/react/src/index.ts 100.00% <ø> (ø)
...c/modules/guide/components/LocationSensor/index.ts 100.00% <100.00%> (ø)
...ckages/react/src/modules/guide/components/index.ts 100.00% <100.00%> (ø)
packages/react/src/modules/guide/index.ts 100.00% <ø> (ø)
.../guide/components/LocationSensor/NextAppRouter.tsx 26.31% <26.31%> (ø)
...uide/components/LocationSensor/NextPagesRouter.tsx 20.83% <20.83%> (ø)

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