feat: use network and WS connection observer services from LLC - #3281
Merged
Merged
Conversation
…ng i18n init The persistent "Waiting for network…" notification was removed moments after being published, whenever the socket dropped before `Streami18n.init()` resolved — an offline app launch, a captive portal, an expired token. That is precisely when the banner is wanted, and there was none. Dismissal was part of the subscription effect's cleanup, so any change to that effect's dependencies destroyed the notification, and `t` changes identity when init completes (the constructed placeholder gives way to i18next's real one). Re-subscribing does not republish, so the banner was gone for good. Dismissal is now scoped to the mount, which is what it was always for. `t` stays in the subscription's dependencies, where the lint rule wants it; re-subscribing on a change is idempotent and leaves the notification alone. The existing test passed only because `waitFor` polls and caught the transient window between the notification being published and being removed. The new test drops the socket without awaiting anything first, so it lands inside the init window, and fails if dismissal is moved back into the subscription's cleanup.
`connection.changed` and `connection.recovered` carry `connection: 'network' | 'ws'` in stream-chat v10, and both variants reach every existing handler. A handler that ignores the field therefore reacts to the device's network as well as to our WebSocket, which would report a lost network every time the socket drops on a working one. Every subscription keeps its event name and its `online` field; each now narrows to `'ws'`, preserving today's behaviour exactly: - `useReportLostConnectionSystemNotification` publishes its banner for the socket only. The notification type says `network`, but the fact behind it has always been the socket. - `Channel` reloads its loaded message window on the socket's recovery only. The compiler cannot help here — both variants have the same payload shape, so an un-guarded handler keeps compiling and keeps behaving as before. Two tests fail if either guard is removed. `Channel` also drops its `connection.changed` subscription entirely. The branch that read it went with the dead `online` ref, and `handleEvent` has no catch-all, so the event reached the component and did nothing. The mock builders take an optional `ConnectionType`, defaulting to `'ws'` so existing call sites keep the meaning they had when this event could only be about the socket.
`ConnectionRecoveryManager` reloads every active channel and then dispatches `connection.recovered`. `Channel` handled that event by calling `channel.reload()` again, and since it marks its channel active while mounted, every open channel was reloaded twice per reconnect — two full `watch()` requests each. `Channel.reload()`'s `_reloading` flag is a re-entrancy guard and has already reset by the time the event is dispatched, so it did not collapse the pair. Measured at two reloads per reconnect before this change and one after. The handler goes, and with it the `connection.recovered` subscription: nothing else in `handleEvent` reacted to that event. The reconciliation React's reload existed for — a hard delete that happened offline arrives via no event, so only a re-query surfaces it — still happens, because the client reloads active channels itself. The old test dispatched `connection.recovered` directly, which is exactly why it never saw the duplicate; the replacement drives a whole reconnect from the socket coming back. `keeps rendering when the reload fails` moves with the behaviour it covered: the client reloads with `Promise.allSettled`, so a socket flapping mid-reload cannot throw into the component at all. Note the `stream-chat` pin is deliberately left at 10.0.0-rc.7. It has to move to the release that carries the connection work — rc.9 exports none of `ConnectionType`, `NetworkConnectionState` or `client.networkConnection`, which this branch already imports — so bumping it is a release-time step, not part of this change.
The persistent banner read "Waiting for network…" whenever `connection.changed` reported `online: false`. That event is the WebSocket, so the message told users their network was down when the server had closed the socket, the token had expired, or a health check had timed out on working Wi-Fi. It now reads both facts and picks the wording: - the device reports no network → "Waiting for network…" - the network is up, or unknown, and the socket is down → "Reconnecting…" One banner throughout, replaced rather than stacked when the reason changes. Choosing that grouping is a copy decision rather than a fact about connectivity, which is why the client publishes no combined status and why the decision is made here, in the component that renders the copy. The existing translation key keeps the network case, where its wording was always true, so its translations stay valid and only one new string was needed. It also reads the current state at mount. It reacted only to transitions before, so a client that was already offline showed nothing until something changed. Two things kept deliberately: - One notification type for both messages. Consumers filter banners on `system:network:connection:lost` — the SDK's own cookbook recipe does — so splitting it would silently stop those filters seeing the socket case. The `network` in the name is historical; the message is what was wrong. - The socket half stays on `connection.changed` rather than `client.wsConnection.state`. The event is held `WS_OFFLINE_ANNOUNCE_DELAY_MS` (5s) on the way down and dropped entirely if the socket returns inside that window, which is what stops a brief flap strobing the banner; the store publishes the raw edge. The socket's last announced value therefore lives in a ref seeded from the store only once — re-reading the store on every effect re-run discarded what the event had said and dismissed the banner. `ConnectionType` is imported from the client rather than redeclared, so a connection type added there breaks `yarn build` here until someone decides what the banner should say about it. Both signals are subscribed imperatively rather than through `useNetworkConnectionState` / `useWSConnectionState`: `Chat` calls this hook, and re-rendering the whole tree on every network flap is what those hooks exist to let consumers avoid.
Driving the two connection facts apart by hand is awkward, and the obvious tool is the wrong one: DevTools' "Offline" checkbox takes down `navigator.onLine` *and* the socket, which is the one combination that always worked. What needs exercising is a dead socket on a live network — a server close, an expired token, a health-check timeout — because that is the case the offline banner used to describe as "Waiting for network…". Two toggles, showing both facts and flipping each independently. Hidden behind a sidebar button that mirrors the theme and RTL toggles, backed by a `devTools.connectionPanel` setting that defaults to off. Both toggles simulate rather than sever, and the socket one writes the store *and* dispatches `connection.changed`, because that is what the real socket does: the store carries the raw state, the event is the announcement the banner listens to. Closing the real socket is no use for a toggle — it reconnects on its own within a second or two, so it would flip back by itself — and the one thing that does hold, `client.closeConnection()`, deliberately dispatches no event, so no banner appears. The component's doc gives the console one-liner for the genuine path, including the five-second announce delay the toggles cannot show. The German and Italian dictionaries here are complete by type assertion, so the new `chat.reportLostConnection.reconnecting.text` key needed translating in both — the assertion fails the build naming any key left out. Earned its keep immediately: it surfaced a regression where every watched request went out without a `connection_id`.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…r here `stream-chat` no longer dispatches `connection.changed`: connectivity is published only as `client.wsConnection.state` and `client.networkConnection.state`. This catches the SDK up. The lost-connection banner subscribes to the socket's store and holds a drop itself. The client used to hold it for five seconds before publishing, so a brief flap did not strobe the banner; it now publishes every transition as it happens, and how long to wait before telling a person is a decision about copy rather than about connectivity. The hook waits `WS_OFFLINE_ANNOUNCE_DELAY_MS`, exported by the client so the UI SDKs do not each pick their own number, and cancels the wait if the socket returns inside it. Coming back is not held: there is no reason to sit on good news. Three details the previous shape already needed and this keeps. The status seeded on mount is shown immediately rather than held, so an application that starts up with no connection says so. The held drop lives in a ref, because the effect re-runs when `Streami18n.init()` replaces `t` and restarting the timer would extend the wait. And the device's network is not held back at all: a browser reports that accurately and it does not flap the way a socket does. `dispatchConnectionChangedEvent` is replaced by `setWSConnectionStatus` and `setNetworkStatus`, which write the stores. The tests that asserted the banner now advance timers past the window, and one that asserted the raw store must *not* show a banner is replaced by one asserting that a flap inside the window shows nothing at all. Also renames registrar to reporter throughout, following the client, and drops the dev panel's event dispatch so its socket toggle writes the store alone. BREAKING CHANGE: `dispatchConnectionChangedEvent` is removed from `stream-chat-react/mock-builders`. Use `setWSConnectionStatus(client, isOnline)` or `setNetworkStatus(client, isOnline)`; a test asserting on the lost-connection banner must also advance timers by `WS_OFFLINE_ANNOUNCE_DELAY_MS`, which the client used to absorb. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The wait before a socket drop reaches the banner was a constant exported by the client, so changing it meant replacing this hook. It is now `client.wsConnection.config.offlineNotificationDisplayDelayMs`, read when the drop happens rather than captured, so a change reaches the next drop without the subscription being re-established. Setting it to zero reports a drop immediately. The tests derive the window from the same place instead of hard-coding five seconds, and one new test pins that a configured value is honoured. BREAKING CHANGE: `WS_OFFLINE_ANNOUNCE_DELAY_MS` is gone from `stream-chat`. A test that advances timers past the banner's hold window should read `client.wsConnection.config.offlineNotificationDisplayDelayMs`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The client moved two things and this catches up with both. The connection id left `client.wsConnection.state` for `client.connectionIdManager`, which the request layer awaits for anything that watches or subscribes to presence. Nothing in this SDK reads it any more, so the WebSocket state hook stops exposing it and the fixtures publish one through the manager instead — without that, every mocked watch waits forever. The socket's status is `isHealthy` again, with `lastHealthyAt` and `lastUnhealthyAt` alongside it. The device's network keeps `isOnline`. That ends the collision the two identical names created: the test that demonstrated the hazard of a blind destructure now demonstrates that there is nothing left to alias. BREAKING CHANGE: `useWSConnectionState()` returns `isHealthy`, `lastHealthyAt` and `lastUnhealthyAt`, and no longer returns `connectionId`. `setWSConnectionStatus` from `stream-chat-react/mock-builders` publishes the connection id through `client.connectionIdManager` as well as writing the store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # src/components/Channel/Channel.tsx
# Conflicts: # examples/tutorial/package.json # examples/vite/package.json # package.json # yarn.lock
|
Size Change: +4.3 kB (+0.52%) Total Size: 831 kB 📦 View Changed
ℹ️ View Unchanged
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-v15 #3281 +/- ##
==============================================
Coverage ? 85.21%
==============================================
Files ? 526
Lines ? 15335
Branches ? 4866
==============================================
Hits ? 13068
Misses ? 2267
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on GetStream/stream-chat-js#1859
🎯 Goal
The offline banner said "you're offline" when the user's internet was fine.
It rendered off WebSocket health, and a socket dies for reasons that have nothing to do with the
user's network — the server closes it, an auth token expires, a keep-alive times out. On good
Wi-Fi, the app still blamed the network.
The SDK had no way to know better: it couldn't tell whether the device had a network, because
nothing told it. GetStream/stream-chat-js#1859
adds that missing signal — the device's network as a fact separate from the socket. This is the
React half: use it, and fix the things that were wrong once the two facts are told apart.
🛠 Implementation details
The banner now picks its wording from both facts. Network definitely down → "Waiting for
network…". Network up or unknown and socket down → "Reconnecting…". One banner throughout,
replaced rather than stacked when the reason changes. Deciding which of those to show is a copy
decision, not a fact about connectivity, which is why the client publishes no combined status and
the choice is made here, in the component that renders the copy.
It also reads the current state on mount. It only reacted to transitions before, so a client that
was already offline showed nothing until something changed.
Two hooks, one per fact, thin wrappers over
useStateStore, exported from the package:The field names differ because the guarantees differ.
isHealthyis always a boolean — a socketalways has a state.
isOnlineisboolean | undefined, whereundefinedmeans no platformreporter has reported yet, which is the normal state on React Native until one is installed. Test
it with
=== false;!isOnlinealso fires when the answer is unknown, which would show apermanent offline banner.
There is no third hook combining them, for the reason above.
useNetworkConnectionStateSelectoris there for components that only want one field and shouldn'tre-render on the others.
Connectivity is read from stores, not events.
connection.changedno longer exists in the LLC:the socket's status lives in
client.wsConnection.stateand the device's inclient.networkConnection.state, each written on every transition. Every consumer in this packagesubscribes to whichever store answers its question, so there is no longer a single event that two
different facts have to share.
The banner's hold is now a client setting. The LLC used to delay its connectivity event by five
seconds; that hold moved here, and its length comes from
client.wsConnection.config.offlineNotificationDisplayDelayMs(5s default) so every UI SDK sharesone value. A drop that resolves inside the window publishes nothing at all.
Fixed: open channels were reloaded twice per reconnect.
ConnectionRecoveryManagerreloadsevery active channel and then dispatches
connection.recovered;Channelhandled that event byreloading again, and it marks its channel active while mounted. Two full
watch()requests peropen channel, measured. The reload now happens once, in the client —
Channel's own handler andits subscription are gone.
Fixed: the banner vanished on a cold offline start.
Streami18n.init()is async andtchanges identity when it resolves. Dismissal was part of the subscription effect's cleanup, so a
socket dropping inside that window published the notification and then had it removed — no banner
on an offline app launch or behind a captive portal. Dismissal is now scoped to the mount, where
it belongs.
Removed a dead
onlineref inChannel. Written on every connectivity event, read by nothingsince the v15 state migration.
Added a dev panel to the vite example, behind a sidebar toggle next to theme and RTL. Two
toggles that drive the network and the socket independently, because DevTools' "Offline" checkbox
takes down both at once — which is the one combination that always worked. The socket toggle writes
the status store and parks the live connection id while it is "down", handing the same id back on
the way up, so the client's recovery replay after the simulated reconnect uses a real id rather
than a fabricated one. It found a real regression in the LLC during review.
🎨 UI Changes
Screenshots needed — I can't produce them. What to capture:
socket toggle, or
client.wsConnection.connection.ws.close()in the console). Previously thissaid "Waiting for network…".
Offline.
Behaviour changes for integrators
No API is removed from this package, so nothing here is a
BREAKING CHANGE:forstream-chat-reactitself. Five things are worth knowing, and the first two arrive from the LLC.stream-chat@^10.0.0-rc.12. That release carries its own breakingchanges — see GetStream/stream-chat-js#1859
for the full list.
connection.changedno longer exists. If you listened for it yourself — to render your ownbanner, to trigger a refetch — subscribe to
client.wsConnection.stateorclient.networkConnection.stateinstead, or use the two hooks above. Nothing in this packagedispatches or forwards the old event.
chat.reportLostConnection.reconnecting.text(default "Reconnecting…").An untranslated language falls back to that English text, so nothing renders a raw
chat.reportLostConnection…string. Add it to your dictionaries when you want it localized. Theexisting
waitingNetworkkey is unchanged and still used for the no-network case, so itstranslations stay valid.
message, or on the
system:network:connection:lostnotification meaning "no network", should bere-checked. The notification type is unchanged so existing filters keep working; it has always
tracked the socket despite
networkin its name.Channelno longer reloads onconnection.recovered. If you relied on that component torefresh a channel after a reconnect, the client does it now.