Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughSummary by CodeRabbit
WalkthroughExplicit casts were added after Sequence Diagram(s)(omitted) Assessment against linked issues
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/realtime_client/test/channel_test.dart`:
- Around line 224-254: The test "maintains type safety after off() - reproduces
web hot restart issue" has several lines exceeding the 80-character limit
(notably the test declaration and the long channel.onEvents calls and comments);
wrap those long lines to ≤80 chars by breaking long string comments and argument
lists across lines (e.g., split the test description, each channel.onEvents
call's arguments, the comment block above expect(() => channel.off(...)), and
the channel.trigger/broadcastCalled lines) while preserving semantics and
keeping identifiers intact (test name, channel.onEvents, ChannelFilter,
channel.off, channel.trigger, broadcastCalled, defaultRef).
|
Great if this fixes the issue! I tried to fix this last year for some hours and tried to find the reason. I cannot believe something like this is the issue. This definitely smells like a Dart bug, since the type system should be sound and adhere to what the analysis says, but here we are. |
|
@Vinzent03 I had Claude give it a try, but not sure if it actually fixes it also, I still need to manually test it. |
Fixes TypeError when using RealtimeChannel.off() on Flutter web during hot restart. JavaScript interop causes type inference to fail on .where().toList() chains, returning List<dynamic> instead of List<Binding>, which fails type checks. Root Cause: - RealtimeChannel.off() at line 475-478 used .where().toList() without explicit casting - RealtimeClient.remove() at line 332 had similar pattern - During web hot restart, JS interop loses type information - Assignment back to typed collections (Map<String, List<Binding>>, List<RealtimeChannel>) fails Solution: - Added .cast<Binding>() to RealtimeChannel.off() after .toList() - Added .cast<RealtimeChannel>() to RealtimeClient.remove() after .toList() - Follows existing pattern in realtime_presence.dart (lines 172, 177, 240, 303) - Consistent with type safety improvements from commit 102595d Acceptance Criteria: - [x] The off() method successfully removes event bindings on Flutter web without type errors - [x] Hot restart on Flutter web no longer throws TypeError - [x] All existing tests pass (98 tests) - [x] No breaking changes to public API - [x] Type safety maintained across all platforms - [x] Similar pattern in realtime_client.dart reviewed and fixed Testing: - Added test case documenting the web hot restart issue - Verified all 98 tests pass in realtime_client package - No functional changes, only type safety improvements Linear: SDK-640 GitHub: #1307 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
fcad177 to
50c9fc8
Compare
Pull Request Test Coverage Report for Build 22070800208Details
💛 - Coveralls |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary
Fixes TypeError when using
RealtimeChannel.off()on Flutter web during hot restart. JavaScript interop causes type inference to fail on.where().toList()chains, returningList<dynamic>instead of the expected typed lists.Changes
.cast<Binding>()tooff()method (line 480).cast<RealtimeChannel>()toremove()method (line 335)Root Cause
File:
packages/realtime_client/lib/src/realtime_channel.dart:475-478The issue occurs when:
.where()method's return type inference fails.toList()producesList<dynamic>instead ofList<Binding>_bindings[typeLower](declared asMap<String, List<Binding>>) fails the type checkSimilar issue existed in
RealtimeClient.remove()with thechannelslist.Error Message:
Solution
Added explicit
.cast<T>()calls after.toList()to ensure type safety:This follows existing patterns in
realtime_presence.dart(lines 172, 177, 240, 303) and is consistent with type safety improvements from commit 102595d.Testing
Test Coverage
off()functionality maintainedManual Testing
Risk Assessment
.cast()is a lightweight operation)Acceptance Criteria
off()method successfully removes event bindings on Flutter web without type errorsLinear Issue
Closes: SDK-640
Related
GitHub Issue: #1307
🤖 Generated with Claude Code
/take