[stipsan]

Description

With beta.reactActivityMode, returning to the Graph tool showed a blank canvas, then the graph laid itself out from scratch for two seconds. The Studio keeps recently used tools mounted behind <Activity>, which cleans up their effects on hide and runs them again on reveal. Two things happened on reveal: the fetch effect ran again and replaced the graph with a new GraphData(docs) whose nodes have no positions, and react-force-graph-2d (via react-kapsule) destroyed its force-graph instance on hide, which wipes its data, then created an empty one on reveal and never handed it the data again because the prop's identity had not changed. The refetch was in fact what rescued the graph from staying empty. The zoom was lost with the canvas, and the live listener was torn down for the hidden period.

The tool now draws with force-graph directly and ties the documents, the listener and the instance to the component's lifetime rather than to its effects. Both effects hand their cleanup to disposeOnUnmount: a cleanup whose DOM is still connected is a hide, which only pauses the animation loop; only a real unmount destroys the instance and unsubscribes. Nothing is fetched again on reveal, the listener has kept the graph current while hidden, the canvas keeps its zoom and node positions, and the simulation is not handed the same data again (which would reheat it). The listener's handler is a pure applyMutation over the latest state (the same logic as before, with the node objects kept so positions survive), so one subscription serves the whole lifetime instead of a new listen connection per mutation. One adjustment to that logic: its link filters used to read l.source.id, which assumes force-graph has already swapped the string endpoints for node objects; links appended while the tool is hidden keep their string endpoints until the reveal (the graphData effect is cleaned up), so the filters now read an endpoint's id in either form.

React does not run a hidden tool's effect cleanups again when it later unmounts that tool (the Studio evicts the oldest of its three mounted tools), so for a hidden tool disposeOnUnmount arms onRemovedFromDocument, a single shared MutationObserver that disposes the listener and destroys the instance once the container leaves the document; the effect disarms it when it runs again on reveal (or because query/client changed while visible, which is the same cleanup path).

Why not stay on react-force-graph-2d: its lifecycle is bound to effects, so the instance is always rebuilt on reveal; the best it allows is re-feeding the data (one blank frame, zoom reset, a reheated simulation).

What to review

• GraphView.tsx: the loaded / forceGraph refs and their effects, whose cleanups are disposeOnUnmount(container, dispose) and disposeOnUnmount(container, destroy, pauseAnimation); applyMutation (moved out of the listener callback; linkEndpointId replaces the bare .id reads in its link filters, and the docsById[id] !== null filter it had, which never filters, is kept as is; exported with GraphData for the unit test, not re-exported from the package); and the setter effects, which only re-apply callbacks. The drawing code is verbatim. The canvas host div is the last child of GraphRoot after the Legend and the conditional HoverNode; React reconciles those unkeyed children by index, so toggling the hover label does not remount the host (pinned by a test).
• onRemovedFromDocument.ts: onRemovedFromDocument (the shared MutationObserver) and disposeOnUnmount, which turns isConnected into "dispose now" or "dispose once removed". Identical to the copy in fix(media): keep the browser alive while an Activity boundary hides it #2178, so the two can be shared later.
• package.json / pnpm-lock.yaml: react-force-graph-2d → force-graph (what it wrapped, already in the tree); @sanity/client (the MutationEvent type), @testing-library/react and jsdom as devDependencies. The lockfile change is the importer entries plus the three now-unreferenced packages removed.

Affects sanity-plugin-graph-view.

Testing

• GraphView.test.tsx (jsdom, force-graph and the Studio hooks mocked): across an <Activity> hide/show cycle the documents are fetched once, the listener subscribes once and is not unsubscribed, the force-graph instance is paused on hide and resumed (not recreated, not re-fed) on reveal; a real unmount destroys it and unsubscribes, and so does unmounting the tool while it is hidden (fails without the watcher). Hovering and unhovering a node (the onNodeHover callback) shows and hides the label as the host's previous sibling while the host element stays connected, with the same instance, listener and single fetch (fails if the host is remounted: a structure that really does remount it fails with isConnected false). Changing query while visible unsubscribes the old listener, fetches once more, keeps the instance, and leaves no armed watch behind (the old query would otherwise be disposed a second time on unmount; fails if the effect stops disarming). applyMutation directly: appear adds a node and its links and keeps the existing node objects, an update with changed references replaces the links (draft ids stripped), an update without a reference change keeps the graph, disappear removes the node and its links, a mutation without a document returns the same state, and a link appended with string endpoints (the hidden-period shape) is still removed by a later update or disappear of its document (fails with the old .id reads). onRemovedFromDocument.test.ts covers the watcher and disposeOnUnmount.
• pnpm --filter sanity-plugin-graph-view exec vitest run (17 tests, on Node 22 and on Node 26), pnpm build, pnpm lint, pnpm knip, pnpm format are clean.
• Measured in dev/test-studio (/home/graph-your-content, reactActivityMode on) with a headed Playwright harness that lets the simulation settle (force-graph ticks for 15s after a load), switches to Vision, waits 7s, switches back and records React commits, mounted components, requests and painted frames (CDP screencast) until the page is quiet for 30 frames. Before: 119 painted frames over 2.1s (blank canvas, then a relayout from phyllotaxis positions), 1 refetch of every document plus 2 listen reconnects, final pixels differ from before the hide. After: 1 painted frame, 0 requests, before/after screenshots pixel-identical.

Graph tool before: blank canvas then relayout
Graph tool after: one frame, same graph

Notes for release

Fixes the Graph tool refetching every document and laying the graph out from scratch when returning to it in a Studio with beta.reactActivityMode enabled; it now continues where it left off, zoom included.

_{To show artifacts inline, enable in settings.}

  

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
plugins-studio	Ready	Preview	Oct 29, 2026 4:30pm UTC

1 Skipped Deployment

Project	Deployment	Actions	Updated
plugins-e2e-test-studio	Ignored		Oct 29, 2026 4:30pm UTC

[github-actions]

✅ E2E Tests

🟢 46 passed • view full report • view run

Studio: https://plugins-e2e-test-studio-idp4ablio.sanity.dev

Datasets: pr-2180-chromium-40912010976, pr-2180-firefox-40912010976

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	39.3%	8125 / 20670
🔵	Statements	39.27%	8744 / 22262
🔵	Functions	35.03%	2225 / 6350
🔵	Branches	31.47%	4708 / 14959

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx	56.45%	61.15%	73.84%	55.62%	31, 57, 70-71, 91-92, 102-123, 145-163, 311, 321-326, 361-370, 398, 405-440, 454-544, 552
plugins/sanity-plugin-graph-view/src/tool/onRemovedFromDocument.ts	100%	100%	80%	100%

Generated in workflow #11472 for commit ce1e66c by the Vitest Coverage Report Action

[cursor]

Review: approve, with non-blocking suggestions

Verified

• All 6 tests pass on the branch. Removing the two onRemovedFromDocument(...) arms makes "releases everything when the hidden tool is unmounted" fail (destroyed stays false), so the DOM watcher is load-bearing and the claim that React 19.3 does not re-run a hidden tree's passive cleanups on deletion holds.
• A path the suite does not cover, changing query while visible, behaves correctly (details inline at the data effect cleanup); worth a test.
• react-kapsule 2.6.0 confirms the root cause in the description: useEffectOnce runs comp._destructor() in the hide cleanup and re-inits on reveal, while prop propagation diffs against a prevPropsRef that survives the hide, so an unchanged graphData is never re-applied, and force-graph's _destructor is pauseAnimation() + graphData({nodes: [], links: []}). Staying on react-force-graph-2d really cannot avoid the rebuild.
• _destructor, pauseAnimation and resumeAnimation are in force-graph's public .d.ts; nodeVal is triggerUpdate: false, so re-setting it on hover does not reheat; GraphData.clone() shares node objects, so positions survive mutations as described.
• Drawing code is verbatim: a token diff of nodeCanvasObject and onRenderFramePost shows only oxfmt line wrapping.
• Lockfile diff is the importer entries plus removal of react-force-graph-2d, react-kapsule and jerrypick; force-graph@1.52.0 was already in the tree. CI green. Props and exports unchanged.

Non-blocking

1. The hide-vs-unmount cleanup is written twice here and a third time in fix(media): keep the browser alive while an Activity boundary hides it #2178; a shared helper would shrink the lifecycle code (inline).
2. The Activity explanation appears three times (the util's JSDoc, the 12-line block in GraphView, and the inline comments at each isConnected). One canonical paragraph in onRemovedFromDocument plus one-liners at the call sites would read better.
3. applyMutation is pure now and deserves a direct unit test; the !== null filter it inherited never filters (inline).
4. onRemovedFromDocument observes the whole document with subtree: true for as long as the tool is hidden. check() is O(watches) per batch so it is cheap, but every Studio DOM mutation allocates mutation records while Graph (and, with fix(media): keep the browser alive while an Activity boundary hides it #2178, Media) is hidden. Fine for now; if it ever shows in a profile the alternative is a Studio-side unmount signal, not a narrower observer, so it is worth knowing the cost is accepted.

Description and measurements

The frame/request numbers come from a harness outside the repo and I could not reproduce them; every mechanism claim I could check in code holds, and the description matches the diff, including the dependency swap called out in the changeset.

Verdict

Approve. The rewrite is as small as the chosen approach allows, the lifetime logic is correct in every path I could exercise, and the suggestions above are cleanups rather than fixes.

[changeset-bot]

🦋 Changeset detected

Latest commit: ce1e66c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
sanity-plugin-graph-view	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[cursor]

Re-review (8d245a8): approve

All four non-blocking points are addressed; nothing new to raise.

Verified

• 15 tests pass on this head (3 lifecycle, 5 applyMutation, 6 watcher/disposeOnUnmount, 1 package exports).
• The new "loads a new query while visible and keeps the graph instance" test is a real guard: with the stopWatchingData.current?.() disarm removed it fails with expected 3 to be 2 (the old query is disposed a second time on unmount), exactly the failure the description names. The hidden-unmount test still fails without the DOM watch.
• disposeOnUnmount matches the sketch, is byte-identical to fix(media): keep the browser alive while an Activity boundary hides it #2178's copy (diffed), and both cleanups are one line; onHide runs only on the connected (hide) branch, _destructor pauses on its own in the unmount branch, so the pause is not doubled. Dropping the stopWatching*.current = null resets is fine because the unmount branch returns noop.
• applyMutation unit tests cover appear (node identity of existing nodes kept), update with changed refs (draft id stripped, links replaced), update without ref changes (same graph), disappear, and the no-document case. The loaded() helper mirrors force-graph's endpoint swap, which is what the l.source.id filters assume. GraphData/GraphState/applyMutation are module exports only; the package entry does not re-export them and knip is green.
• The docsById[id] !== null filter is kept verbatim and the description now says so explicitly; follow-up as agreed.
• Comment volume: the 12-line block is a 4-line note pointing at disposeOnUnmount, and the util's JSDoc is the one long-form explanation. CI green, lockfile unchanged from the first review.

Verdict

Approve. The non-blocking threads are addressed and can be resolved on your side.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Hover remounts wipe the graph
  • Moved HoverNode after the canvas host so toggling hover no longer remounts the container via index reconciliation and destroy the force-graph instance.

Or push these changes by commenting:

@cursor push 1865100c45

Preview (1865100c45)

diff --git a/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx b/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx
--- a/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx
+++ b/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx
@@ -561,12 +561,13 @@
             </LegendRow>
           ))}
         </Legend>
-        {hoverNode && <HoverNode theme={theme}>{labelFor(hoverNode.doc)}</HoverNode>}
         <div
           ref={(element) => {
             if (element) setContainer(element)
           }}
         />
+        {/* After the canvas host so toggling hover does not remount it (index reconciliation). */}
+        {hoverNode && <HoverNode theme={theme}>{labelFor(hoverNode.doc)}</HoverNode>}
       </GraphRoot>
     </GraphWrapper>
   )

_{You can send follow-ups to the cloud agent here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Hidden mutations leave stale graph links
  • applyMutation now resolves link endpoints via linkEndpointId so string ids and force-graph node objects both match when filtering on update or disappear.
• ✅ Fixed: Hidden mutations leave stale graph links
  • applyMutation now resolves link endpoints via linkEndpointId so string ids and force-graph node objects both match when filtering on update or disappear.

Or push these changes by commenting:

@cursor push 7f493de648

Preview (7f493de648)

diff --git a/plugins/sanity-plugin-graph-view/src/tool/GraphView.test.tsx b/plugins/sanity-plugin-graph-view/src/tool/GraphView.test.tsx
--- a/plugins/sanity-plugin-graph-view/src/tool/GraphView.test.tsx
+++ b/plugins/sanity-plugin-graph-view/src/tool/GraphView.test.tsx
@@ -334,4 +334,26 @@
     expect(next).toBe(state)
     expect(docNode).toBeUndefined()
   })
+
+  // While Activity hides the tool, force-graph never rewrites string endpoints to node objects.
+  // Mutations still chain through applyMutation, so filters must match both shapes.
+  test('replaces string-endpoint links when references change before force-graph rewrites them', () => {
+    const state = {documents: [author, post], graph: new GraphData([author, post])}
+    expect(state.graph.data.links).toEqual([{source: 'b', target: 'a'}])
+
+    const other = {...stamps, _id: 'c', _type: 'author', title: 'C'} as SanityDocument
+    const withOther = applyMutation(state, mutation('appear', 'c', other)).next
+    const doc = {...stamps, _id: 'b', _type: 'post', title: 'B', author: {_ref: 'c'}}
+    const {next} = applyMutation(withOther, mutation('update', 'b', doc))
+
+    expect(next.graph.data.links).toEqual([{source: 'b', target: 'c'}])
+  })
+
+  test('removes string-endpoint links when a document disappears before force-graph rewrites them', () => {
+    const state = {documents: [author, post], graph: new GraphData([author, post])}
+    const {next} = applyMutation(state, mutation('disappear', 'b'))
+
+    expect(next.graph.data.links).toEqual([])
+    expect(next.graph.data.nodes.map((n: {id: string}) => n.id)).toEqual(['a'])
+  })
 })

diff --git a/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx b/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx
--- a/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx
+++ b/plugins/sanity-plugin-graph-view/src/tool/GraphView.tsx
@@ -79,6 +79,11 @@
   return id.replace(/^drafts\./, '')
 }
 
+/** force-graph replaces string endpoints with node objects; mutations while hidden leave strings. */
+function linkEndpointId(endpoint: string | {id: string}): string {
+  return typeof endpoint === 'object' ? endpoint.id : endpoint
+}
+
 function deduplicateDrafts(docs: SanityDocument[]) {
   const deduped: Record<string, SanityDocument> = {}
   for (const doc of docs) {
@@ -219,7 +224,7 @@
     let graphChanged = !dequal(oldRefs, newRefs)
     if (graphChanged) {
       newGraph.data.links = newGraph.data.links
-        .filter((l: any) => l.source.id !== doc._id)
+        .filter((l: any) => linkEndpointId(l.source) !== doc._id)
         .concat(newRefs.map((ref) => ({source: doc._id, target: ref})))
     }
 
@@ -242,7 +247,7 @@
 
     const newGraph = graph.clone()
     newGraph.data.links = newGraph.data.links.filter(
-      (l: any) => l.source.id !== docId && l.target.id !== docId,
+      (l: any) => linkEndpointId(l.source) !== docId && linkEndpointId(l.target) !== docId,
     )
     newGraph.data.nodes = newGraph.data.nodes.filter((n: any) => n.id !== docId)

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit 4831228. Configure here.}