/flutter-dart-code-review

name: flutter-dart-code-review
description: Library-agnostic Flutter/Dart code review checklist covering widget best practices, state management patterns (BLoC, Riverpod, Provider, GetX, MobX, Signals), Dart idioms, performance, accessibility, security, and clean architecture.
origin: ECC

Flutter/Dart Code Review Best Practices

Comprehensive, library-agnostic checklist for reviewing Flutter/Dart applications. These principles apply regardless of which state management solution, routing library, or DI framework is used.

---

1. General Project Health

• [ ] Project follows consistent folder structure (feature-first or layer-first)
• [ ] Proper separation of concerns: UI, business logic, data layers
• [ ] No business logic in widgets; widgets are purely presentational
• [ ] `pubspec.yaml` is clean — no unused dependencies, versions pinned appropriately
• [ ] `analysis_options.yaml` includes a strict lint set with strict analyzer settings enabled
• [ ] No `print()` statements in production code — use `dart:developer` `log()` or a logging package
• [ ] Generated files (`.g.dart`, `.freezed.dart`, `.gr.dart`) are up-to-date or in `.gitignore`
• [ ] Platform-specific code isolated behind abstractions

---

2. Dart Language Pitfalls

• [ ] **Implicit dynamic**: Missing type annotations leading to `dynamic` — enable `strict-casts`, `strict-inference`, `strict-raw-types`
• [ ] **Null safety misuse**: Excessive `!` (bang operator) instead of proper null checks or Dart 3 pattern matching (`if (value case var v?)`)
• [ ] **Type promotion failures**: Using `this.field` where local variable promotion would work
• [ ] **Catching too broadly**: `catch (e)` without `on` clause; always specify exception types
• [ ] **Catching `Error`**: `Error` subtypes indicate bugs and should not be caught
• [ ] **Unused `async`**: Functions marked `async` that never `await` — unnecessary overhead
• [ ] **`late` overuse**: `late` used where nullable or constructor initialization would be safer; defers errors to runtime
• [ ] **String concatenation in loops**: Use `StringBuffer` instead of `+` for iterative string building
• [ ] **Mutable state in `const` contexts**: Fields in `const` constructor classes should not be mutable
• [ ] **Ignoring `Future` return values**: Use `await` or explicitly call `unawaited()` to signal intent
• [ ] **`var` where `final` works**: Prefer `final` for locals and `const` for compile-time constants
• [ ] **Relative imports**: Use `package:` imports for consistency
• [ ] **Mutable collections exposed**: Public APIs should return unmodifiable views, not raw `List`/`Map`
• [ ] **Missing Dart 3 pattern matching**: Prefer switch expressions and `if-case` over verbose `is` checks and manual casting
• [ ] **Throwaway classes for multiple returns**: Use Dart 3 records `(String, int)` instead of single-use DTOs
• [ ] **`print()` in production code**: Use `dart:developer` `log()` or the project's logging package; `print()` has no log levels and cannot be filtered

---

3. Widget Best Practices

Widget decomposition:

• [ ] No single widget with a `build()` method exceeding ~80-100 lines
• [ ] Widgets split by encapsulation AND by how they change (rebuild boundaries)
• [ ] Private `_build*()` helper methods that return widgets are extracted to separate widget classes (enables element reuse, const propagation, and framework optimizations)
• [ ] Stateless widgets preferred over Stateful where no mutable local state is needed
• [ ] Extracted widgets are in separate files when reusable

Const usage:

• [ ] `const` constructors used wherever possible — prevents unnecessary rebuilds
• [ ] `const` literals for collections that don't change (`const []`, `const {}`)
• [ ] Constructor is declared `const` when all fields are final

Key usage:

• [ ] `ValueKey` used in lists/grids to preserve state across reorders
• [ ] `GlobalKey` used sparingly — only when accessing state across the tree is truly needed
• [ ] `UniqueKey` avoided in `build()` — it forces rebuild every frame
• [ ] `ObjectKey` used when identity is based on a data object rather than a single value

Theming & design system:

• [ ] Colors come from `Theme.of(context).colorScheme` — no hardcoded `Colors.red` or hex values
• [ ] Text styles come from `Theme.of(context).textTheme` — no inline `TextStyle` with raw font sizes
• [ ] Dark mode compatibility verified — no assumptions about light background
• [ ] Spacing and sizing use consistent design tokens or constants, not magic numbers

Build method complexity:

• [ ] No network calls, file I/O, or heavy computation in `build()`
• [ ] No `Future.then()` or `async` work in `build()`
• [ ] No subscription creation (`.listen()`) in `build()`
• [ ] `setState()` localized to smallest possible subtree

---

4. State Management (Library-Agnostic)

These principles apply to all Flutter state management solutions (BLoC, Riverpod, Provider, GetX, MobX, Signals, ValueNotifier, etc.).

Architecture:

• [ ] Business logic lives outside the widget layer — in a state management component (BLoC, Notifier, Controller, Store, ViewModel, etc.)
• [ ] State managers receive dependencies via injection, not by constructing them internally
• [ ] A service or repository layer abstracts data sources — widgets and state managers should not call APIs or databases directly
• [ ] State managers have a single responsibility — no "god" managers handling unrelated concerns
• [ ] Cross-component dependencies follow the solution's conventions:
• In **Riverpod**: providers depending on providers via `ref.watch` is expected — flag only circular or overly tangled chains
• In **BLoC**: blocs should not directly depend on other blocs — prefer shared repositories or presentation-layer coordination
• In other solutions: follow the documented conventions for inter-component communication

Immutability & value equality (for immutable-state solutions: BLoC, Riverpod, Redux):

• [ ] State objects