fix(ui): avoid spurious StreamTypingIndicator rebuilds on unchanged typing users#2826
fix(ui): avoid spurious StreamTypingIndicator rebuilds on unchanged typing users#2826VelikovPetar wants to merge 1 commit into
Conversation
…yping users Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesTyping indicator optimization
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart (1)
53-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFilter the initial typing state using the same
parentIdpredicate as stream emissions. Otherwise, a thread indicator can initially display a main-channel user until another event arrives.
packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart#L53-L58: filterchannelState.typingEvents.entriesbyparentIdbefore mapping the initial users.packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart#L172-L194: seed initial typing data with a nonmatching main-channel event and verify it is never displayed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart` around lines 53 - 58, Filter the initial typing users in BetterStreamBuilder using channelState.typingEvents.entries and the same parentId predicate applied to stream emissions before mapping entries to users. In packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart lines 172-194, seed initial data with a nonmatching main-channel event and verify it is never displayed.
🧹 Nitpick comments (1)
packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart (1)
147-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the comparator’s unchanged-ID and reordered-ID branches.
These assertions only emit different user sets, so they pass even without
comparator. Emit replacementUserobjects with the same ordered IDs and verify no visible update, then reverse their order and verify the displayed first user changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart` around lines 147 - 158, Update the typing indicator test around emit and the existing text assertions to cover comparator behavior: emit replacement User objects with the same ordered IDs and assert the displayed text remains unchanged, then emit the same users in reversed order and assert the displayed first user changes accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart`:
- Around line 172-194: Extend the test around StreamTypingIndicator to seed
channelState.typingEvents with a main-channel typing event before mounting the
widget, then assert that User(id: 'user-a') is not rendered immediately after
the initial pump. Keep the existing stream-emission assertion to verify
filtering continues to work for subsequent events.
---
Outside diff comments:
In `@packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart`:
- Around line 53-58: Filter the initial typing users in BetterStreamBuilder
using channelState.typingEvents.entries and the same parentId predicate applied
to stream emissions before mapping entries to users. In
packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart
lines 172-194, seed initial data with a nonmatching main-channel event and
verify it is never displayed.
---
Nitpick comments:
In `@packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart`:
- Around line 147-158: Update the typing indicator test around emit and the
existing text assertions to cover comparator behavior: emit replacement User
objects with the same ordered IDs and assert the displayed text remains
unchanged, then emit the same users in reversed order and assert the displayed
first user changes accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10a89c91-ee80-4cb4-8bfc-7b6f83f9f4cb
📒 Files selected for processing (3)
packages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/indicators/typing_indicator.dartpackages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart
| when(() => channelState.typingEvents).thenReturn(const {}); | ||
| when(() => channelState.typingEventsStream).thenAnswer( | ||
| (_) => typingController.stream, | ||
| ); | ||
|
|
||
| await tester.pumpWidget( | ||
| MaterialApp( | ||
| home: Scaffold( | ||
| body: StreamTypingIndicator( | ||
| channel: channel, | ||
| child: const Scaffold( | ||
| body: StreamTypingIndicator( | ||
| key: typingKey, | ||
| ), | ||
| ), | ||
| parentId: 'thread-1', | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| await tester.pump(); | ||
|
|
||
| // wait for the initial state to be rendered. | ||
| await tester.pump(Duration.zero); | ||
| // Typing in the main channel (no parentId) is ignored by a thread | ||
| // indicator. | ||
| await emit(tester, typingController, { | ||
| User(id: 'user-a', name: 'Alice'): Event(type: EventType.typingStart), | ||
| }); | ||
| expect(find.byType(Flexible), findsNothing); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Cover filtering of the initial typing state.
Initializing typingEvents to {} only tests filtering after stream emission. Seed it with a main-channel typing event before mounting the thread indicator and assert that user is never rendered; this exposes the unfiltered initialData at typing_indicator.dart Line 54.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart`
around lines 172 - 194, Extend the test around StreamTypingIndicator to seed
channelState.typingEvents with a main-channel typing event before mounting the
widget, then assert that User(id: 'user-a') is not rendered immediately after
the initial pump. Keep the existing stream-emission assertion to verify
filtering continues to work for subsequent events.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2826 +/- ##
=======================================
Coverage 71.28% 71.28%
=======================================
Files 430 430
Lines 26934 26939 +5
=======================================
+ Hits 19199 19204 +5
Misses 7735 7735 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Submit a pull request
Linear: FLU-600
Github Issue: #
CLA
Description of the pull request
Problem.
StreamTypingIndicatorfeeds aBetterStreamBuilder<Iterable<User>>a lazyMappedIterable(from.where().map()) with no comparator, soBetterStreamBuilderfalls back to identity equality (event == _lastEvent). Every typing-stream emission produces a freshIterableidentity — including repeatedtyping.startre-sends while a user keeps typing, the 1s stale-typing cleanup timer, and out-of-scope thread/main-channel events — forcing a rebuild even when the set of typing users hasn't changed.Fix. Add a comparator to the
BetterStreamBuilderthat compares the typing users by user id (order-sensitiveListEquality<String>). Order-sensitivity is deliberate: the label rendersusers.first, so a reorder that changes who is first must still rebuild; a repeatedtyping.startfor an already-typing user preserves insertion order and is therefore correctly suppressed. Comparing by id (rather than fullUserequality) also ignores unrelated churn such as presence updates on the same typing user.Impact. Benchmarked against the real widget, before vs after, over an identical 43-event typing churn (one user typing → repeated re-emissions → a second user joins → more re-emissions → first stops):
~14× fewer builds — but the more important change is the shape: build count goes from linear in the number of typing events to constant (= the number of distinct typing-user sets), independent of how much churn the backend re-sends or how often the stale-cleanup timer ticks.
Tests. The component previously had a single render smoke test. Added widget tests covering
empty → one → many → emptyupdates (driven through a real stream) andparentId/thread filtering, closing pre-existing coverage gaps.Breaking change. None. Not source-breaking (no signature changes). Behavioral-only and benign — the indicator no longer re-renders for unrelated
User-object churn (e.g. presence updates) while the same users keep typing.Screenshots / Videos
No UI changes. (Rendered output is identical; only the rebuild frequency changes.)
Summary by CodeRabbit
Bug Fixes
Tests