Combine Code Review
Quick Reference
| Issue Type | Reference |
|---|---|
| Publishers, Subjects, AnyPublisher | references/publishers.md |
| map, flatMap, combineLatest, switchToLatest | references/operators.md |
| AnyCancellable, retain cycles, [weak self] | references/memory.md |
| tryMap, catch, replaceError, Never | references/error-handling.md |
Review Checklist
- All
sinkclosures use[weak self]when self owns cancellable - No
assign(to:on:self)usage (useassign(to: &$property)or sink) - All AnyCancellables stored in Set or property (not discarded)
- Subjects exposed as
AnyPublisherviaeraseToAnyPublisher() -
flatMapused correctly (not whenmap + switchToLatestneeded) - Error handling inside
flatMapto keep main chain alive -
tryMapfollowed bymapErrorto restore error types -
receive(on: DispatchQueue.main)before UI updates - PassthroughSubject for events, CurrentValueSubject for state
- Future wrapped in Deferred when used with retry
When to Load References
- Reviewing Subjects or publisher selection → publishers.md
- Reviewing operator chains or combining publishers → operators.md
- Reviewing subscriptions or memory issues → memory.md
- Reviewing error handling or try* operators → error-handling.md
Hard gates (before you report findings)
Complete in order. Do not skip ahead while a prior gate is open.
- Scope — Pass: You name at least one file or type under review that imports Combine or uses APIs from the Quick Reference (e.g.
AnyPublisher,@Published,PassthroughSubject). If none apply, stop with “out of scope.” - Subscription retention — Pass: For each
sink,assign, andstore(in:)in scope, you state where theAnyCancellableis retained (property,Set, task lifetime) or mark ephemeral with a one-line reason (e.g. synchronous one-shot that cannot outlive caller). If you cannot tell from the snippet, say unknown and ask for surrounding storage, do not assume safe. - Retain-cycle claim — Pass: Confirmed leak findings state the capture chain (e.g. self → stored cancellable → closure strongly capturing self). Label suspected cases risk / verify, not confirmed leaks. When arguing safety, cite
[weak self],[unowned self], or non-capturing patterns you relied on. - UI / main thread — Pass: For updates to UIKit/SwiftUI from a chain, you either point to
receive(on: DispatchQueue.main),@MainActor, or equivalent before the UI work, or flag missing scheduling withfile:line. - Severity and checklist — Pass: Every high or critical item includes
file:line(or exact pasted lines) and names which Review Checklist row it breaks. Lower-severity notes may omit line numbers but must still be reproducible from named files.
Review Questions
- Are all subscriptions being retained? (Check for discarded AnyCancellables)
- Could any sink or assign create a retain cycle with self?
- Does flatMap need to be switchToLatest for search/autocomplete?
- What happens when this publisher fails? (Will it kill the main chain?)
- Are error types preserved or properly mapped after try* operators?