githubnext/chopinPublic

Let's compose rich plans together

[use-effect] Derive effective access during render instead of an Effect #155

Closedgithub-actions[bot]botclosed 1 commit intomainfromrefs/pull/155/head
  1. ## Effect reviewed

    `RoomWorkspace` (`apps/web/src/room-workspace.tsx`, previously lines 332-338):

    ```tsx useEffect(() => { let editable = canEdit && !archivedAt; latestCanEdit.current = editable; latestCanManage.current = canManage; setEffectiveCanEdit(editable); setEffectiveCanManage(canManage); }, [archivedAt, canEdit, canManage, room]); ```

    ## Classification

    `derive-render`.

    ## Why the Effect was incorrect

    `effectiveCanEdit`/`effectiveCanManage` start out equal to a pure function of the `canEdit`/`archivedAt`/`canManage` props (`useState(canEdit && !archivedAt)` / `useState(canManage)`). This Effect only re-ran that same pure computation whenever those props changed — it never talked to any external system (no subscription, no DOM, no network). Because it ran in an Effect instead of during render, a prop change was visible for one extra paint with the *old* authorization values before the Effect fired and corrected them. That is exactly the case the React docs call out: you don't need an Effect for adjusting state that is a pure function of props.

    The `room` dependency was already vestigial — `RoomWorkspace` is always mounted with `key={channel.id}` from its callers (`document-workspace-host.tsx`), so `room` never changes without remounting the whole component and resetting all state; it did not need to participate in the comparison.

    ## Fix

    Replaced the Effect with the React "adjust state during render when a prop changes" pattern: track the previous `{ archivedAt, canEdit, canManage }` in a `ref`, and if any of them differ from the current render's props, update the ref and call the state setters synchronously in the render body (before any JSX is produced). This keeps the exact same values and the same `latestCanEdit`/`latestCanManage` ref side effects that later `session:*` socket handlers rely on (those still update the same refs/state directly, unaffected by this change), but removes the extra render pass.

    ## Verification

    - `bun run types` — all workspaces (`@chopin/web` included) type-check cleanly. - `bun test apps/web` — 304 pass, 0 fail (no regressions). - `bunx oxlint apps/web/src/room-workspace.tsx` — 0 warnings, 0 errors. - `bun run ci` could not complete in this sandbox: `dprint check` fails to download its TypeScript plugin binary because outbound network access to GitHub Releases is blocked here. `oxlint` (run standalone above) and the token checker were not otherwise affected by this change.

    Only this one Effect/file was touched, per the one-violation-per-PR constraint.

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

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

  2. MaggieAppletonclosedlast month on GitHub