Code review example: body-only edits don't reliably autosave: body() is read outside effect's tracking in Angular

Retired from the DiffDojo practice pool in October 2026; the review below is the canonical answer. An AI wrote this pull request for the task underneath. Read the diff and decide what you would say before you scroll to the review.

Angular · State & data flow · intermediate

The task

Autosave the draft editor

1. Autosave the draft whenever its title or body changes.
2. Debounce saves by 500ms so fast typing doesn't hammer the endpoint.
3. Use signals and effect() per the codebase conventions.

Adds debounced autosave to the draft editor: an effect() watches the title and body signals and persists the draft 500ms after a change. Saves fire while the user types.

title and body are separate signals bound to two inputs; DraftService.save persists one draft record.

The diff

The review

Issue 1: body-only edits don't reliably autosave: body() is read outside effect's tracking

src/app/editor/autosave.ts, lines 12, 13, 14, 15, 16

1. effect() tracks only synchronous reads. During the effect's run it reads title(), tracked, then schedules a setTimeout. body() is read *inside that callback*, which fires 500ms later, after dependency collection has closed. So body is not a dependency and body edits don't re-run the effect.
2. Autosave becomes one-sided and timing-dependent. Editing the title re-runs the effect and saves. A body-only edit schedules nothing; it only reaches the server if it happens before an already-pending timer fires (that callback reads body() live). Stop typing the title, edit the body after the last timer has fired, and that change is never persisted, breaking the first requirement.
3. setTimeout is not a debounce. Nothing cancels the previous timer, so several title changes queue several saves 500ms apart instead of collapsing into one. Requirement 2 is unmet too.

The fix: read both signals synchronously so both are tracked, and cancel the pending timer each run to actually debounce.

The fix

constructor() {
  let timer: ReturnType<typeof setTimeout>;
  effect(() => {
    const draft = { title: this.title(), body: this.body() }; // both tracked
    clearTimeout(timer);
    timer = setTimeout(() => this.drafts.save(draft), 500);
  });
}

Why it matters

A signal read inside a callback, setTimeout, then, or subscribe inside an effect is NOT a dependency. Only synchronous reads register. If an effect must react to a signal, read it in the effect body.

Real-world stakes: a user tweaks the body after they've stopped touching the title, no save fires, and a refresh loses the edit, with no error anywhere. Reactive-tracking mistakes are the signature state bug in signal/computed/effect code, and models reproduce them because the read "looks" present; only its timing matters.

Read more: angular effect signal not tracked async, signal read inside setTimeout dependency, angular effect debounce signal

The verdict

Request changes. body-only edits don't reliably autosave: body() is read outside effect's tracking

How to review Angular code · All code review examples