## Effect reviewed

`PlanEditor` in `packages/editor/src/plan-editor.tsx`, the `useEffect` at (formerly) lines 269–276:

```tsx useEffect(() => { if (previousWire.current !== wire) { previousWire.current = wire; setState({ synced: false }); setGeneration(value => value + 1); } if (!wire) setState({ synced: false }); }, [wire]); ```

## Classification

`reset-or-adjust-state`. This Effect does not synchronize with any external system — it compares the current `wire` prop against a ref of the previous value and, when it has changed, resets local `state`/`generation` so the editor remounts (via the `key={generation}` on `MDXEditor`) against the new connection.

## Why it was incorrect

This is exactly the "adjusting state when a prop changes" pattern React's docs call out as an Effect anti-pattern: the component renders once with stale `state`/`generation`, then the Effect fires and triggers a second render with the corrected values. Since the condition is comparable during render (`previousWire.current !== wire`), the reset can happen directly in the render body, avoiding the wasted render and being robust to Strict Mode's double-effect invocation (double-invoking the old Effect was already idempotent here, but doing it in render removes the extra render pass entirely).

## Fix

Moved the comparison and `setState`/`setGeneration` calls out of `useEffect` into the render body (the documented "adjust state during render" pattern), guarded by the same ref comparison, executed before the early-return for `!wire`. The `if (!wire) setState({ synced: false })` fallback was subsumed into the general reset since `wire` becoming falsy is itself a change from any previous truthy value (and stays consistent on repeated `undefined → undefined` renders, which are no-ops). The `useEffect` import remains, since three other Effects in the same file (viewport/scroll sync, scrollTop restoration, `changes.dispose()` cleanup) are genuine external-sync Effects and are unchanged.

## Verification

- `tsgo --noEmit --skipLibCheck` in `packages/editor` — passes with no errors. - `oxlint packages/editor/src/plan-editor.tsx` — 0 warnings, 0 errors. - `dprint check` could not run in this sandbox (plugin download blocked by network policy); the diff follows existing file formatting by inspection (tabs, indentation, quote style unchanged from surrounding code). - No unit tests cover this render path directly (it requires a live `MDXEditor` mount), so `bun test` was not run; `bun run e2e` was not run since this change doesn't alter effect-observable browser behavior beyond removing a render, which existing e2e coverage of plan reconnect flows already exercises indirectly.

> [!WARNING] > <details> > <summary>Firewall blocked 1 domain</summary> > > The following domain was blocked by the firewall during workflow execution: > > - `releaseassets.githubusercontent.com` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "releaseassets.githubusercontent.com" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details>

> Generated by [Weekly React Effect review](https://github.com/githubnext/chopin/actions/runs/32244799677) · auto · 88 AIC · ⌖ 4.54 AIC · ⊞ 9.2K · [◷](https://github.com/search?q=repo%3Agithubnext%2Fchopin+%22gh-aw-workflow-id%3A+weekly-use-effect-review%22&type=pullrequests)

<!-- gh-aw-agentic-workflow: Weekly React Effect review, engine: copilot, model: auto, id: 32244799677, workflow_id: weekly-use-effect-review, run: https://github.com/githubnext/chopin/actions/runs/32244799677 -->

<!-- gh-aw-workflow-id: weekly-use-effect-review --> <!-- gh-aw-workflow-call-id: githubnext/chopin/weekly-use-effect-review -->

The explanatory comment is a bit misleading around Strict Mode: Strict Mode doesn’t “skip” Effects; in dev it intentionally re-runs mount Effects (and double-invokes render), so wording like “cannot be skipped by Strict Mode’s double-invoked Effects” reads backwards/unclear. Consider rephrasing to focus on the concrete behavior change (avoiding a post-commit Effect-driven reset render) without implying Effects are skipped.

## Pull request overview

This PR updates `PlanEditor`’s connection-reset behavior by moving the “reset local state when `wire` changes” logic from a `useEffect` into a render-time adjustment, aligning it with React’s “adjust state during render” guidance for prop-derived state and avoiding an extra post-commit render.

**Changes:** - Replaced the `useEffect([wire])` reset logic with a render-time `previousWire` comparison that resets `state.synced` and bumps `generation`. - Removed the now-redundant `if (!wire) setState({ synced: false })` Effect branch (subsumed by the `wire` change check).

<details open> <summary>Show a summary per file</summary>

| File | Description | | ---- | ----------- | | packages/editor/src/plan-editor.tsx | Moves `wire`-change reset from an Effect to render-time state adjustment and keeps the “Not connected” early-return behavior. | </details>

## Review details

💡 <a href="/githubnext/chopin/new/main?filename=.github/skills/code-review/SKILL.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add a `code-review` agent skill</a> or configure MCP servers for context-aware, tailored reviews. <a href="https://docs.github.com/en/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review#mcp-servers-and-agent-skills" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn more in the docs.</a>

- **Files reviewed:** 1/1 changed files - **Comments generated:** 1 - **Review effort level:** Lite

Removed the comment entirely in ddf1981.