Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow falsey userIds for auth and gracefully handle (#343) #344

Merged
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
6 changes: 6 additions & 0 deletions .changeset/healthy-ladybugs-drum.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"nextjs-example": patch
"@knocklabs/client": patch
---

fix: ensure feed can render with empty/missing userId values
2 changes: 1 addition & 1 deletion examples/nextjs-example/next-env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
/// <reference types="next/image-types/global" />

// NOTE: This file should not be edited
// see https://nextjs.org/docs/basic-features/typescript for more information.
// see https://nextjs.org/docs/pages/building-your-application/configuring/typescript for more information.
15 changes: 10 additions & 5 deletions examples/nextjs-example/pages/api/identify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@ export default async function handler(
name: name || faker.person.fullName(),
});

const userToken = await Knock.signUserToken(userId, {
expiresInSeconds: process.env.KNOCK_TOKEN_EXPIRES_IN_SECONDS
? Number(process.env.KNOCK_TOKEN_EXPIRES_IN_SECONDS)
: 3600,
});
let userToken = undefined;

if (process.env.KNOCK_SIGNING_KEY) {
userToken = await Knock.signUserToken(userId, {
expiresInSeconds: process.env.KNOCK_TOKEN_EXPIRES_IN_SECONDS
? Number(process.env.KNOCK_TOKEN_EXPIRES_IN_SECONDS)
: 3600,
});
}

return res.status(200).json({ error: null, user: knockUser, userToken });
} catch (error) {
console.error(error);
return res.status(500).json({
error: (error as Error).message || (error as Error).toString(),
user: null,
Expand Down
4 changes: 2 additions & 2 deletions examples/nextjs-example/pages/api/notify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default async function handler(
const { message, showToast, userId, tenant, templateType } = req.body;

try {
await knockClient.notify(KNOCK_WORKFLOW, {
const response = await knockClient.workflows.trigger(KNOCK_WORKFLOW, {
recipients: [userId],
actor: userId,
tenant,
Expand All @@ -32,7 +32,7 @@ export default async function handler(
},
});

return res.status(200).json({ error: null });
return res.status(200).json({ error: null, response });
} catch (error) {
return res.status(500).json({
error: (error as Error).message || (error as Error).toString(),
Expand Down
26 changes: 2 additions & 24 deletions examples/nextjs-example/pages/index.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,4 @@
import {
Box,
Flex,
Heading,
Icon,
Link,
Select,
Spinner,
Text,
} from "@chakra-ui/react";
import { Box, Flex, Heading, Icon, Link, Select, Text } from "@chakra-ui/react";
import {
KnockFeedProvider,
KnockProvider,
Expand All @@ -32,7 +23,7 @@ const TenantLabels = {
};

export default function Home() {
const { userId, isLoading, userToken } = useIdentify();
const { userId, userToken } = useIdentify();
const [tenant, setTenant] = useState(Tenants.TeamA);

const tokenRefreshHandler = useCallback(async () => {
Expand All @@ -43,19 +34,6 @@ export default function Home() {
return json.userToken;
}, [userId]);

if (isLoading) {
return (
<Flex
alignItems="center"
justifyContent="center"
width="100vw"
height="100vh"
>
<Spinner />
</Flex>
);
}

return (
<KnockProvider
userId={userId}
Expand Down
35 changes: 25 additions & 10 deletions packages/client/src/clients/feed/feed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ class Feed {

this.hasSubscribedToRealTimeUpdates = true;

// If the user is not authenticated, then do nothing
if (!this.knock.isAuthenticated()) {
this.knock.log(
"[Feed] User is not authenticated, skipping listening for updates",
);
return;
}

const maybeSocket = this.knock.client().socket;

// Connect the socket only if we don't already have a connection
Expand Down Expand Up @@ -318,11 +326,9 @@ class Feed {
const shouldOptimisticallyRemoveItems =
this.defaultOptions.archived === "exclude";

const normalizedItems = Array.isArray(itemOrItems)
? itemOrItems
: [itemOrItems];
const items = Array.isArray(itemOrItems) ? itemOrItems : [itemOrItems];

const itemIds: string[] = normalizedItems.map((item) => item.id);
const itemIds: string[] = items.map((item) => item.id);

/*
In the code here we want to optimistically update counts and items
Expand Down Expand Up @@ -354,15 +360,17 @@ class Feed {
if (shouldOptimisticallyRemoveItems) {
// If any of the items are unseen or unread, then capture as we'll want to decrement
// the counts for these in the metadata we have
const unseenCount = normalizedItems.filter((i) => !i.seen_at).length;
const unreadCount = normalizedItems.filter((i) => !i.read_at).length;
const unseenCount = items.filter((i) => !i.seen_at).length;
const unreadCount = items.filter((i) => !i.read_at).length;

// Build the new metadata
const updatedMetadata = {
...state.metadata,
total_count: state.metadata.total_count - normalizedItems.length,
unseen_count: state.metadata.unseen_count - unseenCount,
unread_count: state.metadata.unread_count - unreadCount,
// Ensure that the counts don't ever go below 0 on archiving where the client state
// gets out of sync with the server state
total_count: Math.max(0, state.metadata.total_count - items.length),
unseen_count: Math.max(0, state.metadata.unseen_count - unseenCount),
unread_count: Math.max(0, state.metadata.unread_count - unreadCount),
};

// Remove the archiving entries
Expand Down Expand Up @@ -464,8 +472,15 @@ class Feed {
async fetch(options: FetchFeedOptions = {}) {
const { networkStatus, ...state } = this.store.getState();

// If the user is not authenticated, then do nothing
if (!this.knock.isAuthenticated()) {
this.knock.log("[Feed] User is not authenticated, skipping fetch");
return;
}

// If there's an existing request in flight, then do nothing
if (isRequestInFlight(networkStatus)) {
this.knock.log("[Feed] Request is in flight, skipping fetch");
return;
}

Expand Down Expand Up @@ -750,7 +765,7 @@ class Feed {

// If we're initializing but they have previously opted to listen to real-time updates
// then we will automatically reconnect on their behalf
if (this.hasSubscribedToRealTimeUpdates) {
if (this.hasSubscribedToRealTimeUpdates && this.knock.isAuthenticated()) {
if (!maybeSocket.isConnected()) maybeSocket.connect();
this.channel.join();
}
Expand Down
10 changes: 0 additions & 10 deletions packages/client/src/knock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ class Knock {
}

client() {
if (!this.userId) {
console.warn(
`[Knock] You must call authenticate(userId, userToken) first before trying to make a request.
Typically you'll see this message when you're creating a feed instance before having called
authenticate with a user Id and token. That means we won't know who to issue the request
to Knock on behalf of.
`,
);
}

// Initiate a new API client if we don't have one yet
if (!this.apiClient) {
this.apiClient = this.createApiClient();
Expand Down
Loading