fix(state): isolate activeTask/activeTemplate seeding (OUT-3714)#1218
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummarySplits
Confidence Score: 4/5Safe to merge for the task-detail path; the template-editor path retains a concurrent-rendering race that can leave the editor blank on fast navigation. The task-seeding race is robustly fixed by the two-effect reconcile pattern. However, SeedActiveTemplate uses the old single-effect cleanup design: a stale concurrent cleanup can still overwrite a fresh setActiveTemplate dispatch with null, and there is no self-heal mechanism to recover. src/hoc/state-seeders.tsx — specifically the SeedActiveTemplate implementation, which needs the same two-effect reconcile pattern applied to SeedActiveTask. Important Files Changed
Sequence DiagramsequenceDiagram
participant React as React 18 Renderer
participant CSU as ClientSideStateUpdate
participant SAT as SeedActiveTask (new)
participant SAT2 as SeedActiveTask (prev)
participant Store as Redux Store
Note over React,Store: Normal mount
React->>CSU: mount with workflowStates/token
CSU->>Store: dispatch workflowStates/token/...
React->>SAT: mount with task prop
SAT->>Store: reconcile effect sets activeTask(task)
Note over React,Store: Concurrent race - stale cleanup wins
React->>SAT: mount NEW instance, sets activeTask(task)
SAT2-->>Store: stale cleanup fires setActiveTask(undefined)
SAT->>Store: reconcile detects drift, re-dispatches activeTask(task)
Note over SAT,Store: Self-healed - sidebar never stuck
Note over React,Store: SeedActiveTemplate - no reconcile guard
React->>Store: new mount sets activeTemplate(template)
SAT2-->>Store: stale cleanup fires setActiveTemplate(null)
Note over Store: activeTemplate stays null - no recovery
Reviews (2): Last reviewed commit: "fix(state): isolate activeTask/activeTem..." | Re-trigger Greptile |
|
Deployment failed with the following error: Learn More: https://vercel.link/multiple-function-regions |
Sidebar got stuck on its loading skeleton because `activeTask` could be
left `undefined` while the SSR `task` prop was defined. Two paths:
- Fast Esc → click → click navigation: a stale unmount cleanup from a
previous mount of ClientSideStateUpdate could land AFTER the new
mount's setActiveTask(task) dispatch under React 18 concurrent
rendering, clearing it.
- First load on slow workspaces: similar race during hydration.
The race is latent on main too — feature/c1-optimization exposed it by
removing the server-side awaits on AssigneeFetcher/WorkspaceFetcher
(moved to client-side SWR in the layout), which shortens the time
between successive page commits and shifts the navigation into the race
window.
Root cause is that ClientSideStateUpdate's mega-effect coupled ~14
unrelated state concerns into one effect with a unified cleanup that
cleared activeTask + activeTemplate. Any single dep changing re-ran
every dispatch AND the cleanup.
Surgical fix: extract only the two concerns with cleanups
(activeTask + activeTemplate) into dedicated components. Everything
else stays in CSU exactly as today.
- `src/hoc/state-seeders.tsx` (new): SeedActiveTask + SeedActiveTemplate.
SeedActiveTask has a reconcile-on-render effect (idempotent self-heal
against stale-cleanup races) and a separate empty-dep unmount-clear
so the cleanup fires only on true unmount.
- `src/hoc/ClientSideStateUpdate.tsx`: dropped `task` + `template`
props, the corresponding branches in the mega-effect, and both lines
in the cleanup (cleanup is now empty so it's removed). Dep array
no longer includes `task`. Everything else unchanged.
- `DetailStateUpdate.tsx`: mounts <SeedActiveTask task={task} /> inside
CSU instead of passing task to CSU.
- `manage-templates/[template_id]/page.tsx`: mounts <SeedActiveTemplate>
inside CSU instead of passing template.
- `WorkflowStateFetcher.tsx`: mounts <SeedActiveTask task={task} />
inside CSU. Preserves the probabilistic safety of the redundant
seeding the detail page already had.
Home, client, configure-tasks-app, AllTasksFetcher, TemplatesFetcher all
keep using ClientSideStateUpdate exactly as before — no API changes for
them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
91b431c to
4f23993
Compare
|
@greptileai Review again and resolve your existing comments if they are no longer valid. |
Greptile flagged that SeedActiveTemplate kept the old single-effect shape where the cleanup is tied to the same effect that re-runs on prop changes. A stale cleanup from a previous SeedActiveTemplate instance could fire after the new mount's setActiveTemplate(template) dispatch under React 18 concurrent rendering, leaving activeTemplate as null with no recovery — template editor would silently render empty until the user navigates away and back. Mirror SeedActiveTask: reconcile-on-render effect with [template, activeTemplate] deps for self-healing drift, and a separate empty-deps effect whose cleanup fires only on true unmount. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
With SeedActiveTask's reconcile-on-render pattern, a single seeder per page is sufficient — drift always self-heals. The double-seeding via WorkflowStateFetcher was kept as probabilistic safety in case the race re-emerged, but it provides no additional protection beyond what the reconcile already guarantees. Drop the SeedActiveTask call in WorkflowStateFetcher and the now-unused task prop. DetailStateUpdate's SeedActiveTask is the sole owner of activeTask. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The deferred cleanup in the realtime task-delete handler captured tasks and accessibleTasks at event time. By the time setTimeout(0) fired, the user could already have been redirected to the board and CSU may have seeded a fresh list from SSR — the captured-snapshot dispatch would then clobber the freshly seeded data (and lose any new task that landed via a racing create event). Read latest state from the store inside the setTimeout callback instead. Keeps the original deferral (a race patch for update-before-create ordering) while avoiding the clobber on the redirect path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Fixes OUT-3714: Sidebar can get stuck on its loading skeleton because Redux
activeTaskisundefinedwhile the SSRtaskprop is defined.Root cause
ClientSideStateUpdate's mega-useEffectcoupled ~14 unrelated state concerns into one body + one cleanup that clearedactiveTask+activeTemplate. Any single dep changing re-ran every dispatch and cleanup. Under React 18 concurrent rendering, a stale unmount cleanup from a previous mount could land AFTER the next mount'ssetActiveTask(task)dispatch, leavingactiveTaskundefined.The race is latent on main too —
feature/c1-optimizationexposed it. MovingAssigneeFetcher/WorkspaceFetcherfrom server-side awaits to client-side SWR (insrc/app/layout.tsx) shortened the time between successive commits and shifted rapid Esc → click navigation into the race window. Workspaces with fast Copilot responses on main never hit it.Surgical fix
Isolate only the two state concerns that actually have cleanups (
activeTask+activeTemplate) into dedicated components. Everything else inClientSideStateUpdatestays exactly as today.src/hoc/state-seeders.tsx:SeedActiveTask+SeedActiveTemplate.SeedActiveTaskhas two effects:activeTaskdrifts from the SSRtaskprop. Rescues a stale-cleanup race because the next render with drift re-dispatches.ClientSideStateUpdate.tsx: dropped thetask+templateprops, thesetActiveTask/setActiveTemplatedispatches in the effect body, and both lines in the cleanup (the cleanup is now empty and removed).taskis out of the dep array. Everything else unchanged.DetailStateUpdate.tsx: mounts<SeedActiveTask task={task} />inside CSU.manage-templates/[template_id]/page.tsx: mounts<SeedActiveTemplate template={template} />inside CSU.WorkflowStateFetcher.tsx: mounts<SeedActiveTask task={task} />inside CSU. Preserves the redundant double-seeding that the detail page already had (probabilistic safety: two mount-time dispatches means a stale cleanup has to land after BOTH to break things).Home, client, configure-tasks-app, AllTasksFetcher, TemplatesFetcher all continue using
ClientSideStateUpdateexactly as on main — no API changes.Why this is safer than the previous design
SeedActiveTaskcleans up activeTaskTest plan
activeTemplate.Files
5 files changed. ClientSideStateUpdate stays the same for 5 of its 7 callers.
🤖 Generated with Claude Code