A Code Review Checklist for Flutter Teams (Copy, Paste, Adapt)

A Flutter pull request can pass CI, look great in the screenshot and still leak a stream subscription, crash when the user taps back mid-request, and rebuild the whole screen every time a timer ticks.

None of that shows up in a diff unless someone is looking for it. That’s what review is for. Linters catch formatting and a decent chunk of obvious mistakes. A reviewer catches the things that need context.

This is what I look for, roughly in the order I look for it, followed by a checklist you can paste into your PR template.

Start with the size of the PR

Before reading a single line, I check how big the change is. A focused PR that does one thing gets a careful review. A huge PR that mixes a new feature, a refactor and a dependency bump gets a skim, no matter how good the reviewer’s intentions are.

What I ask for:

One purpose per PR. Refactors and renames go in their own PR, ahead of the feature that needs them.
A description that says why, not just what. “Adds retry to order loading because the API times out on slow connections” beats “Order changes.”
Screenshots or a short recording for any UI change, ideally on both a small phone and a large one.
Generated files called out, so reviewers know to skip *.g.dart and focus on the source.

If a PR is too big to review properly, saying so is the review.

Rebuild scope

Flutter is fast, but it’s not magic. The most common performance issue I find in review isn’t an expensive algorithm. It’s a setState or a watch that rebuilds far more of the tree than it needs to.

Things I look for:

State that lives too high. A timer or text field at the top of a screen means the whole screen rebuilds on every tick or keystroke. Push that state down into a smaller widget.
Watching more than you use. If a widget only needs the cart’s item count, it shouldn’t rebuild when the cart’s notes change. Use ref.watch(cartProvider.select((c) => c.items.length)) in Riverpod or context.select with Provider and BLoC.
Missing const. Constructors that could be const should be. The prefer_const_constructors lint helps, but reviewers should notice when a big static subtree isn’t const.
Helper methods returning widgets. A _buildHeader() method rebuilds with its parent every time. A small Header widget class can be const and gets its own element. Not every helper needs converting, but large ones do.
Expensive work in build. Sorting, filtering, parsing dates or formatting currency on every build belongs in the state layer.

When a screen feels slow, I go deeper with the tools in fixing jank in Flutter apps. In review, I’m mostly catching the obvious version before it ships.

Async gaps and `mounted` checks

This is the bug I flag most often, by a wide margin. Code awaits something, and then uses a BuildContext that might no longer be valid because the user left the screen.

Flagged in review:

Future<void> _save() async {
  await repository.save(form);
  Navigator.of(context).pop();
  ScaffoldMessenger.of(context).showSnackBar(
    const SnackBar(content: Text('Saved')),
  );
}

What I ask for:

Future<void> _save() async {
  final messenger = ScaffoldMessenger.of(context);
  await repository.save(form);
  if (!context.mounted) return;
  Navigator.of(context).pop();
  messenger.showSnackBar(const SnackBar(content: Text('Saved')));
}

Inside a State class, if (!mounted) return; works the same way. The use_build_context_synchronously lint catches many of these, so make sure it’s turned on, but it can’t catch every case, especially when the context is passed into another function.

Related things I check:

Calling setState after an await without a mounted check.
Fire-and-forget futures with no error handling. If a Future isn’t awaited, I want to see why. The unawaited() function from dart:async makes intent explicit.
Buttons that can be tapped twice while a request is in flight.

Error, empty and loading states

Most PRs are written for the happy path, because that’s what the developer tests on their own device. I read the state handling and ask three questions:

What does the user see while this loads?
What does the user see if it fails? And can they retry without restarting the app?
What does the user see if it succeeds with no data?

If any answer is “a blank screen” or “an infinite spinner,” that’s a change request. With sealed state classes and exhaustive switches, the compiler forces the first two to be handled. The third, the empty list, still slips through constantly.

I also look at error messages. Raw exception text like SocketException: Failed host lookup should never reach the UI. Map errors to something a person can act on, and log the details for yourself.

Disposal

Anything you create in a State, you dispose in that State. I scan initState and field declarations, then check dispose for the matching cleanup:

TextEditingController, ScrollController, PageController, TabController
AnimationController
FocusNode
StreamSubscription (call cancel())
Timer (call cancel())
Listeners added with addListener, removed with removeListener

@override
void dispose() {
  _subscription.cancel();
  _searchController.dispose();
  _focusNode.dispose();
  super.dispose();
}

Leaks rarely crash anything right away. They show up weeks later as a screen that gets slower every time you open it, or a callback firing on a screen that no longer exists.

Naming and structure

I’m not picky about style. The formatter and linter settle that. I am picky about names that hide meaning:

data, info, item, temp and handleTap tell me nothing. pendingInvoices and onRetryPressed tell me a lot.
Booleans should read as questions: isLoading, hasMore, canSubmit.
Files and classes should match: order_card.dart contains OrderCard.
Business logic in widgets gets flagged. Logic in a state class or service can be tested; logic in an onPressed closure can’t.

Tests

I don’t demand a specific coverage number. I ask whether the change came with tests where they matter:

New business logic and state classes should have unit tests.
Bug fixes should have a test that would have caught the bug.
Important custom widgets and form validation deserve widget tests.

I also read the tests themselves. A test that only checks “it doesn’t throw” isn’t testing much. My broader approach is in my Flutter testing strategy post.

Accessibility

Accessibility issues are cheap to fix in review and expensive to fix after a complaint. I check:

Icon-only buttons have a tooltip or semantic label.
Tap targets are at least 48x48 logical pixels.
Text isn’t in fixed-height containers that clip when the user increases font size.
Color isn’t the only way information is shown, like a red border with no error text.
Decorative images are excluded from semantics; meaningful ones have labels.

The quickest test: turn on large text and the screen reader and try the new screen for a minute. More in accessibility in Flutter apps.

Platform differences

One codebase doesn’t mean one platform. I look for:

Back navigation. Android’s system back button and iOS’s swipe gesture both need to behave. If a screen intercepts back with PopScope, I check it’s intentional.
Safe areas for notches, home indicators and Android’s edge-to-edge display.
Permissions handled on both platforms, including “denied forever.”
Keyboard behavior when fields sit near the bottom of the screen.
Platform-only APIs guarded properly, especially if the app also targets web.

If the PR only shows screenshots from one platform, I ask for the other.

The checklist

Paste this into .github/pull_request_template.md or your Azure DevOps PR template and adjust it to your team:

## What and why
<!-- One purpose. Explain why this change is needed. -->

## Screenshots / recording
<!-- Small phone + large phone. Both platforms for UI changes. -->

## Checklist
- [ ] PR does one thing; refactors are split out
- [ ] Rebuilds are scoped (state pushed down, `select` used, `const` where possible)
- [ ] No heavy work in `build`
- [ ] Every `await` followed by context use has a `mounted` check
- [ ] Un-awaited futures are intentional and handle errors
- [ ] Loading, error (with retry) and empty states are handled
- [ ] User-facing errors are readable; details are logged
- [ ] Controllers, focus nodes, subscriptions, timers and listeners are disposed
- [ ] Names describe intent; no business logic in widgets
- [ ] Tests added or updated for logic and bug fixes
- [ ] Icon buttons labeled, tap targets >= 48x48, large text doesn't clip
- [ ] Tested on Android and iOS (back nav, safe areas, permissions, keyboard)
- [ ] No secrets, debug flags or stray print statements

Takeaways

Small, single-purpose PRs get real reviews. Big ones get skimmed.
Rebuild scope and async gaps cause more Flutter bugs than complex logic does.
Always ask: what does the user see while loading, on failure and with no data?
Everything created in a State must be disposed in it.
Accessibility and platform differences are review items, not post-launch fixes.
Turn on the lints first, so humans can spend their attention on what lints can’t see.

A checklist won’t make a team careful, but it makes being careful the default, and that’s most of the job.