Skip to content
This repository was archived by the owner on Jan 24, 2024. It is now read-only.

fix(feeds): ensure websocket connections aren't closed in the feed #15

Merged
merged 1 commit into from
Jun 3, 2022
Merged

fix(feeds): ensure websocket connections aren't closed in the feed #15

merged 1 commit into from
Jun 3, 2022

Conversation

@cjbell
Copy link
Contributor

@cjbell cjbell commented Jun 3, 2022

We have been seeing issues with the recent changes in React 18, where in development React double renders. This had the effect of causing our websocket connection to terminate because of the call to knock.teardown() in the useEffect hook.

This change:

  1. Connects the feed client to the real-time stream when we're creating the feedClient
  2. Moves to using mutable refs to keep track of state for teardowns
juanazam
juanazam reviewed Jun 3, 2022
View reviewed changes
src/components/KnockFeedProvider/KnockFeedProvider.tsx
colorMode = "light",
rootless,
}) => {
const knockInstance = React.useRef<Knock | null>(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to add comments on what's going on? (React 18 strict mode on dev)
juanazam
juanazam approved these changes Jun 3, 2022
View reviewed changes
Copy link
Contributor

@juanazam juanazam left a comment

Choose a reason for hiding this comment

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

Awesome!
@cjbell cjbell merged commit e6f08db into main Jun 3, 2022
@cjbell cjbell deleted the cjbell-kno-1465-in-app-feed-issue-with-websocket-closing branch June 3, 2022 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

4 participants

@cjbell @juanazam @bceskavich