fix: null check operator used on a null value#3447
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully addresses the issue of calling setState on a disposed widget by adding mounted checks. The refactoring of _applyFilter to use a single setState call is a good improvement. However, this has led to a situation in initState where two setState calls are made in succession, which is inefficient. My review includes suggestions to consolidate these into a single call to improve performance and fully align with the goal of using a single setState.
| _applyFilter(_currentFilter); | ||
|
|
||
| // Mark initial load as complete | ||
| if (mounted) { | ||
| setState(() { | ||
| _isInitialLoad = false; | ||
| }); | ||
| } | ||
| setState(() { | ||
| _isInitialLoad = false; | ||
| }); |
There was a problem hiding this comment.
There are two separate setState calls here, one inside _applyFilter and another one here to set _isInitialLoad. This is inefficient as it causes two rebuilds. After applying the suggested change to _applyFilter to accept a callback, you can combine these into a single state update.
_applyFilter(_currentFilter, andThen: () {
_isInitialLoad = false;
});| void _applyFilter(FilterOption option) { | ||
| setState(() { | ||
| _currentFilter = option; | ||
| if (!mounted) return; | ||
|
|
||
| switch (option) { | ||
| case FilterOption.interesting: | ||
| _filterByCategory(MemoryCategory.interesting); | ||
| MixpanelManager().memoriesFiltered('interesting'); | ||
| break; | ||
| case FilterOption.system: | ||
| _filterByCategory(MemoryCategory.system); | ||
| MixpanelManager().memoriesFiltered('system'); | ||
| break; | ||
| case FilterOption.all: | ||
| _filterByCategory(null); // null means no category filter | ||
| MixpanelManager().memoriesFiltered('all'); | ||
| break; | ||
| } | ||
| }); | ||
| } | ||
| MemoryCategory? category; | ||
| switch (option) { | ||
| case FilterOption.interesting: | ||
| category = MemoryCategory.interesting; | ||
| MixpanelManager().memoriesFiltered('interesting'); | ||
| break; | ||
| case FilterOption.system: | ||
| category = MemoryCategory.system; | ||
| MixpanelManager().memoriesFiltered('system'); | ||
| break; | ||
| case FilterOption.all: | ||
| category = null; // null means no category filter | ||
| MixpanelManager().memoriesFiltered('all'); | ||
| break; | ||
| } | ||
|
|
||
| void _filterByCategory(MemoryCategory? category) { | ||
| if (!mounted) return; | ||
|
|
||
| setState(() { | ||
| _currentFilter = option; | ||
| _selectedCategory = category; | ||
| }); | ||
|
|
There was a problem hiding this comment.
To allow combining setState calls from different places (like in initState), you can modify this method to accept an optional callback. This callback can be executed inside the setState block, consolidating multiple state updates into one. This improves performance by reducing unnecessary rebuilds.
Additionally, the mounted check on line 211 is redundant since there's already a check on line 193 and no await operations are performed in between. The suggested change removes it.
void _applyFilter(FilterOption option, {void Function()? andThen}) {
if (!mounted) return;
MemoryCategory? category;
switch (option) {
case FilterOption.interesting:
category = MemoryCategory.interesting;
MixpanelManager().memoriesFiltered('interesting');
break;
case FilterOption.system:
category = MemoryCategory.system;
MixpanelManager().memoriesFiltered('system');
break;
case FilterOption.all:
category = null; // null means no category filter
MixpanelManager().memoriesFiltered('all');
break;
}
setState(() {
_currentFilter = option;
_selectedCategory = category;
andThen?.call();
});
}
|
@krushnarout conflicts |
|
@krushnarout ready? if yes pls tag thinh |
Cherry-picked crash fixes from BasedHardware/omi: - 51351be: null check operator fix in app_detail.dart (BasedHardware#3305) - cfba83e: RangeError fix in capture_provider.dart (BasedHardware#3306) - e875812: async device null crash fix (BasedHardware#3476) - a491646: audio file handling fix in Swift SDK (BasedHardware#3501) - ec86179: mounted checks in memories page (BasedHardware#3447) These fixes prevent: - Null pointer crashes on app detail page - RangeError when processing transcripts - Async device connection crashes - Audio file accumulation in Swift SDK - Widget state errors after unmount 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
`setState` was called after the widget was already removed fix: Added mounted checks and moved provider updates outside of setState closes BasedHardware#3442
3 participants
setStatewas called after the widget was already removedfix: Added mounted checks and moved provider updates outside of setState
closes #3442