githubnext/chopinPublic

Let's compose rich plans together

[use-effect] Replace reset-on-prop Effect with render-time state adjustment in Chip #126

Mergedgithub-actions[bot]botmerged 1 commit intomainfromrefs/pull/126/head
  1. ## Effect location and classification

    `Chip` in `packages/editor/src/changes-chip.tsx` (the `PlanChanges` per-edge indicator) had:

    ```ts useEffect(() => { if (waiting === 0) setOpen(false); }, [waiting]); ```

    This is a `reset-or-adjust-state` Effect: it only reacts to the `waiting` prop crossing to `0` and forces `open` back to `false`. It does not synchronize with any external system (no DOM API, network, subscription, or store) — it exists purely to keep one piece of state consistent with another.

    ## Why it was incorrect

    Per the "you don't need an Effect" guidance, adjusting state in response to a prop change inside a Effect causes an extra render: React first commits with the stale `open` value (the list still open for a `waiting === 0` frame), the Effect then runs, calls `setOpen(false)`, and a second render/commit happens. This produces a visible flash of an open, now-empty change list before it closes, and adds an unnecessary render pass.

    ## Refactor

    Replaced the Effect with React's documented "adjusting state during render" pattern: track the `waiting` value the `open` state was last computed for (`openedForWaiting`), and when `waiting` changes, call `setOpen(false)` synchronously in the render body (before returning JSX) rather than in a Effect. React detects the state update during render and re-renders immediately with the corrected value in the same commit, avoiding the stale intermediate frame. The existing outside-click-to-close Effect (a legitimate `external-sync` Effect for a `document` event listener) was left untouched.

    ## Verification

    `bun`, `dprint`, and `oxlint` were not available in this sandboxed environment (no network access to fetch the Bun binary or install the workspace's pinned toolchain), so `bun test` / `bun run types` / `bun run ci` could not be executed. I verified the change by: - Manual review against React's documented state-adjustment-during-render pattern. - Confirming the file's existing tab/quote/semicolon formatting conventions are preserved. - Tracing all call sites of `open`/`setOpen` in the component to confirm no other logic depends on the Effect timing.

    Given the tooling limitation, please run `bun run types` (or the editor package's narrower check) and `bun run ci` locally before merging to confirm formatting and type-correctness.

    > [!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/32948086081) · auto · 77.1 AIC · ⌖ 3.76 AIC · ⊞ 8.8K · [◷](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: 32948086081, workflow_id: weekly-use-effect-review, run: https://github.com/githubnext/chopin/actions/runs/32948086081 -->

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

  2. MaggieAppletonready-for-review2 months ago on GitHub

  3. MaggieAppletonmerged 7d3364e535f92 months ago on GitHub

  4. MaggieAppletonclosed2 months ago on GitHub

  5. MaggieAppletonhead-ref-deleted2 months ago on GitHub