Conversation
WalkthroughAdds explicit names to several Signals/Computed in the todos example and extends the devtools extension with an "active duplicates" filter, hot-restart handling to clear signals, new UI controls (checkbox, Clear All), and search layout updates. Changes
Sequence Diagram(s)mermaid Note over UI,State: User toggles controls or searches Note over UI,State: Clear action Note over VM,State: Hot-restart handling rect rgba(46,125,50,0.06) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #169 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 18 18
Lines 931 931
=========================================
Hits 931 931 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/solidart_devtools_extension/lib/main.dart (1)
346-353: Sorting insideitemBuildercauses redundant work.The
filteredSignals.valuelist is sorted on every call toitemBuilder, meaning the entire list is sorted once per visible grid item. This should be moved outside the builder or computed once.♻️ Proposed fix
Sort once before iterating:
child: SignalBuilder( builder: (context, _) { + final sortedSignals = filteredSignals.value + ..sort( + (a, b) => b.value.lastUpdate.compareTo( + a.value.lastUpdate, + ), + ) + ..sort((a, b) => a.value.disposed ? 1 : -1); return GridView.builder( gridDelegate: SliverGridDelegateWithMaxCrossAxisExtent( maxCrossAxisExtent: 500, childAspectRatio: 5, mainAxisSpacing: 8, crossAxisSpacing: 8, ), - itemCount: filteredSignals.value.length, + itemCount: sortedSignals.length, padding: EdgeInsets.symmetric(horizontal: 4), itemBuilder: (BuildContext context, int index) { - final sortedSignals = filteredSignals.value - ..sort( - (a, b) => b.value.lastUpdate.compareTo( - a.value.lastUpdate, - ), - ) - ..sort((a, b) => a.value.disposed ? 1 : -1); - final entry = sortedSignals.elementAt(index);
🤖 Fix all issues with AI agents
In @packages/solidart_devtools_extension/lib/main.dart:
- Around line 171-177: Store the StreamSubscription returned by
vmService.onIsolateEvent.listen so you can cancel it when the widget is
disposed: assign the result of vmService.onIsolateEvent.listen(...) to a field
(e.g., _isolateEventSub), keep the existing listener body that calls
signals.clear() on 'IsolateReload'/'IsolateStart', and override dispose() to
call await _isolateEventSub.cancel() (or _isolateEventSub.cancel();) before
calling super.dispose();.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/todos/lib/controllers/todos.dartexamples/todos/lib/widgets/todos_body.dartexamples/todos/lib/widgets/toolbar.dartpackages/solidart_devtools_extension/lib/main.dart
🧰 Additional context used
📓 Path-based instructions (1)
examples/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Build and run example applications (counter, todos, github_search, toggle_theme) to validate real user scenarios
Files:
examples/todos/lib/widgets/toolbar.dartexamples/todos/lib/widgets/todos_body.dartexamples/todos/lib/controllers/todos.dart
🧠 Learnings (7)
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/{lib,test}/**/*.dart : For Flutter Solidart changes, ensure widget tests cover `SignalBuilder`, `Provider`, and `Show` behaviors
Applied to files:
examples/todos/lib/widgets/toolbar.dartexamples/todos/lib/widgets/todos_body.dartpackages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Use Signals for state, Effects for side effects, and Computed for derived values in library code
Applied to files:
examples/todos/lib/widgets/toolbar.dartpackages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-12-21T03:28:51.442Z
Learnt from: medz
Repo: nank1ro/solidart PR: 167
File: packages/solidart/lib/src/solidart.dart:1396-1396
Timestamp: 2025-12-21T03:28:51.442Z
Learning: In Dart 3.10+, dot shorthand syntax such as identifier = ._(name) or Type x = .constructor() is valid when the type can be inferred from context (e.g., field type, variable declaration, parameter type). Do not flag these as syntax errors when the SDK version is >= 3.10.0. This applies to enum values, static members, and constructors; apply this guidance broadly to Dart files across the repository.
Applied to files:
examples/todos/lib/widgets/toolbar.dartexamples/todos/lib/widgets/todos_body.dartexamples/todos/lib/controllers/todos.dartpackages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/solidart_test.dart : After Signal changes, always run `packages/solidart/test/solidart_test.dart`
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/**/*.dart : Write unit tests covering Signal operations and state transitions
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Ensure proper memory management: Signals auto-dispose in Flutter contexts; manually dispose in pure Dart code
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-10-08T11:10:13.808Z
Learnt from: nank1ro
Repo: nank1ro/solidart PR: 151
File: packages/solidart_hooks/CHANGELOG.md:12-16
Timestamp: 2025-10-08T11:10:13.808Z
Learning: In solidart/flutter_solidart, SignalBuilder supports automatic signal tracking where signals are detected when accessed inside the builder. The builder signature for automatic tracking is `(BuildContext, Widget? child)`, not requiring an explicit signal parameter or value parameter. Example: `SignalBuilder(builder: (context, child) => Text('${counter.value}'))`.
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
🔇 Additional comments (5)
examples/todos/lib/widgets/toolbar.dart (1)
20-31: LGTM! Explicit signal names added for improved dev tools debugging.The naming convention is consistent—each Computed signal's name matches its variable name, which will make debugging and tracking signals in the dev tools extension straightforward.
examples/todos/lib/controllers/todos.dart (1)
21-36: LGTM! Signal naming improves debuggability.The explicit names for
todos,completedTodos, andincompleteTodossignals are consistent with the variable names and will help identify these signals in the dev tools extension.examples/todos/lib/widgets/todos_body.dart (1)
9-11: LGTM! Named signal for the filter provider.Adding the explicit name to the
todosFiltersignal aligns with the PR's goal of improving signal identification in dev tools.packages/solidart_devtools_extension/lib/main.dart (2)
184-195: Good: Comprehensive disposal of all signals.All signals including the new
showOnlyActiveDuplicatesandfilteredSignalsare properly disposed. This ensures no memory leaks from the signal subscriptions.
302-317: LGTM! UI controls implement the PR objectives.The "Active duplicates" checkbox and "Clear All" button address the requirements from issue #161. The layout is clean and the controls are appropriately placed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/solidart_devtools_extension/lib/main.dart (1)
127-134: Consider pre-computing duplicate counts for better performance.The current duplicate filter iterates through all signals for each entry, resulting in O(n²) complexity. While acceptable for typical devtools usage, this could be optimized by pre-computing a name-to-count map.
⚡ Suggested optimization
late final filteredSignals = Computed(() { final lowercasedSearch = searchText.value.toLowerCase(); final type = filterType.value; final viewDisposed = showDisposed.value; final onlyActiveDuplicates = showOnlyActiveDuplicates.value; + + // Pre-compute duplicate counts for active signals + final nameCounts = onlyActiveDuplicates + ? signals.value.values + .where((s) => !s.disposed) + .groupFoldBy<String, int>( + (s) => s.name, + (count, _) => (count ?? 0) + 1, + ) + : <String, int>{}; + return signals.value.entries .where( (entry) => entry.value.name.toString().toLowerCase().contains( lowercasedSearch, ) || entry.value.matchesSearch(lowercasedSearch), ) .where((entry) => type == null || entry.value.type == type) .where((entry) => viewDisposed || !entry.value.disposed) .where((entry) { if (!onlyActiveDuplicates) return true; - // Count signals with same name - final sameName = signals.value.values - .where((s) => s.name == entry.value.name); - // Show only if: has duplicates AND is active - return sameName.length > 1 && !entry.value.disposed; + // Show only if: has duplicates AND is active + final count = nameCounts[entry.value.name] ?? 0; + return count > 1 && !entry.value.disposed; }) .toList(); });Note:
groupFoldByis already available from thecollectionpackage imported at line 4.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/solidart_devtools_extension/lib/main.dart
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/{lib,test}/**/*.dart : For Flutter Solidart changes, ensure widget tests cover `SignalBuilder`, `Provider`, and `Show` behaviors
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Use Signals for state, Effects for side effects, and Computed for derived values in library code
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/**/*.dart : Write unit tests covering Signal operations and state transitions
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/solidart_test.dart : After Signal changes, always run `packages/solidart/test/solidart_test.dart`
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Ensure proper memory management: Signals auto-dispose in Flutter contexts; manually dispose in pure Dart code
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-10-08T11:10:13.808Z
Learnt from: nank1ro
Repo: nank1ro/solidart PR: 151
File: packages/solidart_hooks/CHANGELOG.md:12-16
Timestamp: 2025-10-08T11:10:13.808Z
Learning: In solidart/flutter_solidart, SignalBuilder supports automatic signal tracking where signals are detected when accessed inside the builder. The builder signature for automatic tracking is `(BuildContext, Widget? child)`, not requiring an explicit signal parameter or value parameter. Example: `SignalBuilder(builder: (context, child) => Text('${counter.value}'))`.
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-12-21T03:28:51.442Z
Learnt from: medz
Repo: nank1ro/solidart PR: 167
File: packages/solidart/lib/src/solidart.dart:1396-1396
Timestamp: 2025-12-21T03:28:51.442Z
Learning: In Dart 3.10+, dot shorthand syntax such as identifier = ._(name) or Type x = .constructor() is valid when the type can be inferred from context (e.g., field type, variable declaration, parameter type). Do not flag these as syntax errors when the SDK version is >= 3.10.0. This applies to enum values, static members, and constructors; apply this guidance broadly to Dart files across the repository.
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
🔇 Additional comments (3)
packages/solidart_devtools_extension/lib/main.dart (3)
172-178: LGTM! Hot restart handling correctly implemented.The subscription to
onIsolateEventis now properly stored inisolateEventSuband cancelled indispose(), addressing the subscription leak identified in the previous review. Clearing signals onIsolateReloadorIsolateStartcorrectly handles hot restart scenarios.Also applies to: 188-188
236-320: LGTM! UI changes correctly implement PR objectives.The layout successfully addresses all three requirements:
- Search input and type selector now share the same row (lines 236-288)
- "Active duplicates" checkbox added (lines 304-314)
- "Clear All" button clears the devtools signal cache (line 316)
The use of
Expandedwidgets with appropriate flex ratios provides good space allocation.
190-196: LGTM! Proper resource cleanup.All signals (including the new
showOnlyActiveDuplicates) and subscriptions are correctly disposed, ensuring no memory leaks.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/solidart_devtools_extension/lib/main.dart (3)
102-103: Consider simplifying the subscription field declarations.Both subscription fields use
late finalwith nullable types. This pattern works but could be simplified to just nullable fields (removinglate final) for clarity, since they're conditionally initialized in an async method and already handled safely with?.cancel()in dispose.♻️ Optional simplification
- late final StreamSubscription<Object>? sub; - late final StreamSubscription<Object>? isolateEventSub; + StreamSubscription<Object>? sub; + StreamSubscription<Object>? isolateEventSub;
127-134: Duplicate filtering logic is correct.The logic correctly identifies signals with duplicate names and filters to show only active (not disposed) ones. The nested iteration for counting duplicates is O(n²), but acceptable for a dev tools extension with a limited number of signals.
💡 Optional performance optimization
If the number of signals grows large, consider pre-computing name counts outside the filter chain:
final filtered = signals.value.entries .where(...) .where((entry) => type == null || entry.value.type == type) .where((entry) => viewDisposed || !entry.value.disposed); // Pre-compute name counts if active duplicates filter is enabled Map<String, int>? nameCounts; if (onlyActiveDuplicates) { nameCounts = <String, int>{}; for (final signal in signals.value.values) { if (!signal.disposed) { nameCounts[signal.name] = (nameCounts[signal.name] ?? 0) + 1; } } } final result = onlyActiveDuplicates ? filtered.where((entry) => (nameCounts![entry.value.name] ?? 0) > 1 && !entry.value.disposed ).toList() : filtered.toList();However, this adds complexity and may not be necessary for typical usage.
340-340: Consider making the filter description more precise.The text appends "(duplicates only)" when the active duplicates filter is enabled, but the filter specifically shows only active duplicates (not disposed). Consider "(active duplicates only)" for clarity.
📝 Suggested text improvement
-'${filteredSignals.value.length} visible of ${signals.value.length}${showOnlyActiveDuplicates.value ? ' (duplicates only)' : ''}', +'${filteredSignals.value.length} visible of ${signals.value.length}${showOnlyActiveDuplicates.value ? ' (active duplicates only)' : ''}',
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/solidart_devtools_extension/lib/main.dart
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/{lib,test}/**/*.dart : For Flutter Solidart changes, ensure widget tests cover `SignalBuilder`, `Provider`, and `Show` behaviors
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Use Signals for state, Effects for side effects, and Computed for derived values in library code
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/solidart_test.dart : After Signal changes, always run `packages/solidart/test/solidart_test.dart`
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/**/*.dart : Write unit tests covering Signal operations and state transitions
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
Repo: nank1ro/solidart PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Ensure proper memory management: Signals auto-dispose in Flutter contexts; manually dispose in pure Dart code
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-10-08T11:10:13.808Z
Learnt from: nank1ro
Repo: nank1ro/solidart PR: 151
File: packages/solidart_hooks/CHANGELOG.md:12-16
Timestamp: 2025-10-08T11:10:13.808Z
Learning: In solidart/flutter_solidart, SignalBuilder supports automatic signal tracking where signals are detected when accessed inside the builder. The builder signature for automatic tracking is `(BuildContext, Widget? child)`, not requiring an explicit signal parameter or value parameter. Example: `SignalBuilder(builder: (context, child) => Text('${counter.value}'))`.
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
📚 Learning: 2025-12-21T03:28:51.442Z
Learnt from: medz
Repo: nank1ro/solidart PR: 167
File: packages/solidart/lib/src/solidart.dart:1396-1396
Timestamp: 2025-12-21T03:28:51.442Z
Learning: In Dart 3.10+, dot shorthand syntax such as identifier = ._(name) or Type x = .constructor() is valid when the type can be inferred from context (e.g., field type, variable declaration, parameter type). Do not flag these as syntax errors when the SDK version is >= 3.10.0. This applies to enum values, static members, and constructors; apply this guidance broadly to Dart files across the repository.
Applied to files:
packages/solidart_devtools_extension/lib/main.dart
🔇 Additional comments (6)
packages/solidart_devtools_extension/lib/main.dart (6)
109-109: LGTM!The new signal for the "Active duplicates" filter is correctly declared and initialized, following the existing pattern for
showDisposed.
137-145: LGTM!The sorting logic correctly prioritizes active (not disposed) signals and orders by newest update first. The implementation is clear and efficient.
184-190: Hot restart handling looks good!The isolate event listener correctly clears signals on hot restart events, and the subscription is properly stored for lifecycle management.
200-208: LGTM!All resources are properly disposed, including the new
isolateEventSubsubscription andshowOnlyActiveDuplicatessignal. The memory management is thorough and follows best practices.Based on learnings, proper memory management is essential in Flutter contexts.
248-332: LGTM!The UI reorganization improves the layout with a clear two-column structure for the search and type filter. The new "Active duplicates" checkbox and "Clear All" button are well-integrated and follow existing patterns.
361-361: LGTM!Direct indexed access is correct and efficient since
filteredSignalsalready contains the sorted results from the computed signal.
1 participant
closes #161
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.