feat: Add guidelines for prop drilling prevention and BLoC lifecycle management in architecture principles
This commit is contained in:
@@ -219,3 +219,160 @@ See **`03-data-connect-connectors-pattern.md`** for comprehensive documentation
|
||||
- Each connector follows Clean Architecture (domain interfaces + data implementations)
|
||||
- Features use connector repositories through dependency injection
|
||||
- Results in zero query duplication and single source of truth
|
||||
|
||||
## 8. Prop Drilling Prevention & Direct BLoC Access
|
||||
|
||||
### 8.1 The Problem: Prop Drilling
|
||||
|
||||
Passing data through intermediate widgets creates maintenance headaches:
|
||||
- Every intermediate widget must accept and forward props
|
||||
- Changes to data structure ripple through multiple widget constructors
|
||||
- Reduces code clarity and increases cognitive load
|
||||
|
||||
**Anti-Pattern Example**:
|
||||
```dart
|
||||
// ❌ BAD: Drilling status through 3 levels
|
||||
ProfilePage(status: status)
|
||||
→ ProfileHeader(status: status)
|
||||
→ ProfileLevelBadge(status: status) // Only widget that needs it!
|
||||
```
|
||||
|
||||
### 8.2 The Solution: Direct BLoC Access with BlocBuilder
|
||||
|
||||
Use `BlocBuilder` to access BLoC state directly in leaf widgets:
|
||||
|
||||
**Correct Pattern**:
|
||||
```dart
|
||||
// ✅ GOOD: ProfileLevelBadge accesses ProfileCubit directly
|
||||
class ProfileLevelBadge extends StatelessWidget {
|
||||
const ProfileLevelBadge({super.key});
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
return BlocBuilder<ProfileCubit, ProfileState>(
|
||||
builder: (context, state) {
|
||||
final Staff? profile = state.profile;
|
||||
if (profile == null) return const SizedBox.shrink();
|
||||
|
||||
final level = _mapStatusToLevel(profile.status);
|
||||
return LevelBadgeUI(level: level);
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### 8.3 Guidelines for Avoiding Prop Drilling
|
||||
|
||||
1. **Leaf Widgets Get Data from BLoC**: Widgets that need specific data should access it directly via BlocBuilder
|
||||
2. **Container Widgets Stay Simple**: Parent widgets like `ProfileHeader` only manage layout and positioning
|
||||
3. **No Unnecessary Props**: Don't pass data to intermediate widgets unless they need it for UI construction
|
||||
4. **Single Responsibility**: Each widget should have one reason to exist
|
||||
|
||||
**Decision Tree**:
|
||||
```
|
||||
Does this widget need data?
|
||||
├─ YES, and it's a leaf widget → Use BlocBuilder
|
||||
├─ YES, and it's a container → Use BlocBuilder in child, not parent
|
||||
└─ NO → Don't add prop to constructor
|
||||
```
|
||||
|
||||
## 9. BLoC Lifecycle & State Emission Safety
|
||||
|
||||
### 9.1 The Problem: StateError After Dispose
|
||||
|
||||
When async operations complete after a BLoC is closed, attempting to emit state causes:
|
||||
```
|
||||
StateError: Cannot emit new states after calling close
|
||||
```
|
||||
|
||||
**Root Causes**:
|
||||
1. **Transient BLoCs**: `BlocProvider(create:)` creates new instance on every rebuild → disposed prematurely
|
||||
2. **Singleton Disposal**: Multiple BlocProviders disposing same singleton instance
|
||||
3. **Navigation During Async**: User navigates away while `loadProfile()` is still running
|
||||
|
||||
### 9.2 The Solution: Singleton BLoCs + Error Handler Defensive Wrapping
|
||||
|
||||
#### Step 1: Register as Singleton
|
||||
|
||||
```dart
|
||||
// ✅ GOOD: ProfileCubit as singleton
|
||||
i.addSingleton<ProfileCubit>(
|
||||
() => ProfileCubit(useCase1, useCase2),
|
||||
);
|
||||
|
||||
// ❌ BAD: Creates new instance each time
|
||||
i.add(ProfileCubit.new);
|
||||
```
|
||||
|
||||
#### Step 2: Use BlocProvider.value() for Singletons
|
||||
|
||||
```dart
|
||||
// ✅ GOOD: Use singleton instance
|
||||
ProfileCubit cubit = Modular.get<ProfileCubit>();
|
||||
BlocProvider<ProfileCubit>.value(
|
||||
value: cubit, // Reuse same instance
|
||||
child: MyWidget(),
|
||||
)
|
||||
|
||||
// ❌ BAD: Creates duplicate instance
|
||||
BlocProvider<ProfileCubit>(
|
||||
create: (_) => Modular.get<ProfileCubit>(), // Wrong!
|
||||
child: MyWidget(),
|
||||
)
|
||||
```
|
||||
|
||||
#### Step 3: Defensive Error Handling in BlocErrorHandler Mixin
|
||||
|
||||
The `BlocErrorHandler<S>` mixin provides `_safeEmit()` wrapper:
|
||||
|
||||
**Location**: `apps/mobile/packages/core/lib/src/presentation/mixins/bloc_error_handler.dart`
|
||||
|
||||
```dart
|
||||
void _safeEmit(void Function(S) emit, S state) {
|
||||
try {
|
||||
emit(state);
|
||||
} on StateError catch (e) {
|
||||
// Bloc was closed before emit - log and continue gracefully
|
||||
developer.log(
|
||||
'Could not emit state: ${e.message}. Bloc may have been disposed.',
|
||||
name: runtimeType.toString(),
|
||||
);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Usage in Cubits/Blocs**:
|
||||
```dart
|
||||
Future<void> loadProfile() async {
|
||||
emit(state.copyWith(status: ProfileStatus.loading));
|
||||
|
||||
await handleError(
|
||||
emit: emit,
|
||||
action: () async {
|
||||
final profile = await getProfile();
|
||||
emit(state.copyWith(status: ProfileStatus.loaded, profile: profile));
|
||||
// ✅ If BLoC disposed before emit, _safeEmit catches StateError gracefully
|
||||
},
|
||||
onError: (errorKey) {
|
||||
return state.copyWith(status: ProfileStatus.error);
|
||||
},
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
### 9.3 Pattern Summary
|
||||
|
||||
| Pattern | When to Use | Risk |
|
||||
|---------|------------|------|
|
||||
| Singleton + BlocProvider.value() | Long-lived features (Profile, Shifts, etc.) | Low - instance persists |
|
||||
| Transient + BlocProvider(create:) | Temporary widgets (Dialogs, Overlays) | Medium - requires careful disposal |
|
||||
| Direct BlocBuilder | Leaf widgets needing data | Low - no registration needed |
|
||||
|
||||
**Remember**:
|
||||
- Use **singletons** for feature-level cubits accessed from multiple pages
|
||||
- Use **transient** only for temporary UI states
|
||||
- Always wrap emit() in `_safeEmit()` via `BlocErrorHandler` mixin
|
||||
- Test navigation away during async operations to verify graceful handling
|
||||
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user