[Core npm] hydrate_on: visible — scheduleWhenVisible's disconnect-early guard strands the scheduled entry in renderedRoots, leaking the detached DOM node

[justin808]

Summary

In the core package's hydrate_on: :visible scheduling (new in #4037), scheduleWhenVisible's disconnect-early guard — added to avoid leaking an IntersectionObserver holding a detached node — disconnects the observer and returns without invoking the scheduled callback. Because the callback (runScheduledRender) is the only in-page path that removes the scheduled entry from renderedRoots, the entry keeps holding the detached DOM node (and its cancel closure) until unmountAllComponents runs on a Turbo/Turbolinks page swap. In non-Turbo apps that cleanup never runs, so each hydrate_on: visible island removed outside navigation is a retained-memory leak for the page lifetime. The fix is a one-liner: call callback() instead of returning — runScheduledRender already handles the disconnected case by deleting the renderedRoots entry.

Affected code

• The disconnect-early guard that never invokes the callback:
  

  react_on_rails/packages/react-on-rails/src/ClientRenderer.ts

  Lines 170 to 192 in 5264192

  	const observer = new IntersectionObserver(
  	(entries) => {
  	// Disconnect early if the target was removed outside Turbo navigation to avoid a
  	// leaked observer holding a live reference to a detached node.
  	const target = entries[0]?.target;
  	if (target && !target.isConnected) {
  	observer.disconnect();
  	return;
  	}
  	const isVisible = entries.some((entry) => entry.isIntersecting || entry.intersectionRatio > 0);
  	if (!isVisible) return;
  	observer.disconnect();
  	callback();
  	},
  	{ rootMargin: '200px 0px' },
  	);
  	observer.observe(domNode);
  	return () => {
  	observer.disconnect();
  	};
  	}

  (guard at L173–L178)
• runScheduledRender, which already handles the disconnected-node case by deleting the entry:
  

  react_on_rails/packages/react-on-rails/src/ClientRenderer.ts

  Lines 465 to 478 in 5264192

  	const runScheduledRender = (): void => {
  	if (renderedRoots.get(domNodeId) !== scheduledEntry) return;
  	if (!domNode.isConnected) {
  	renderedRoots.delete(domNodeId);
  	return;
  	}
  	try {
  	mountReactRoot();
  	} catch (scheduledError) {
  	renderedRoots.delete(domNodeId);
  	reportRenderError(name, scheduledError);
  	}
  	};

• The scheduled entry stored in renderedRoots (holds domNode + cancel):
  

  react_on_rails/packages/react-on-rails/src/ClientRenderer.ts

  Lines 480 to 485 in 5264192

  	scheduledEntry = {
  	kind: 'scheduled',
  	domNode,
  	cancel: scheduleHydration(hydrateOn, domNode, runScheduledRender),
  	};
  	renderedRoots.set(domNodeId, scheduledEntry);

• The only other cleanup path — page-unload (Turbo soft navigation) teardown:
  

  react_on_rails/packages/react-on-rails/src/ClientRenderer.ts

  Lines 546 to 558 in 5264192

  	function unmountAllComponents(): void {
  	renderedRoots.forEach((entry, domNodeId) => {
  	try {
  	teardownEntry(entry, domNodeId);
  	} catch (error) {
  	console.error(teardownErrorLabel(entry, domNodeId), error);
  	}
  	});
  	renderedRoots.clear();
  	}
  	// Register cleanup on page unload
  	onPageUnloaded(unmountAllComponents);

Introduced in

PR #4037 ("Add OSS hydrate_on scheduling"), merged 2026-06-18 (commit 1263119). The guard and its comment were added in that same PR (git log -S "Disconnect early" -- packages/react-on-rails/src/ClientRenderer.ts → 1263119cd Add OSS hydrate_on scheduling (#4037)).

git log --oneline 43ed0e138..origin/main -- packages/react-on-rails/src/ClientRenderer.ts:

9c26f260f Add mirrored-blocks lint gate (#4175)
03a69bcc7 Fix hydration error reporting for thrown values (#4120)
1263119cd Add OSS hydrate_on scheduling (#4037)
0b794fc3d Expose React 19 root error callbacks (rootErrorHandlers) + hydration-mismatch debugging guide (#3933)

Mechanism

1. A component rendered with hydrate_on: :visible goes through renderElement → scheduleHydration('visible', ...) → scheduleWhenVisible(domNode, runScheduledRender), and a scheduled entry is stored:

// ClientRenderer.ts L480-L485
scheduledEntry = {
  kind: 'scheduled',
  domNode,
  cancel: scheduleHydration(hydrateOn, domNode, runScheduledRender),
};
renderedRoots.set(domNodeId, scheduledEntry);

2. The IntersectionObserver callback contains the early guard:

// ClientRenderer.ts L170-L184
const observer = new IntersectionObserver(
  (entries) => {
    // Disconnect early if the target was removed outside Turbo navigation to avoid a
    // leaked observer holding a live reference to a detached node.
    const target = entries[0]?.target;
    if (target && !target.isConnected) {
      observer.disconnect();
      return;               // <-- callback (runScheduledRender) is never invoked
    }
    ...
    observer.disconnect();
    callback();
  },
  ...
);

The guard frees the observer, but the renderedRoots map still holds { kind: 'scheduled', domNode, cancel } for that domNodeId. Nothing else in the page's lifetime deletes it.

3. runScheduledRender was written to handle exactly this state — if it were called it would clean up:

// ClientRenderer.ts L465-L470
const runScheduledRender = (): void => {
  if (renderedRoots.get(domNodeId) !== scheduledEntry) return;
  if (!domNode.isConnected) {
    renderedRoots.delete(domNodeId);   // <-- the cleanup the guard skips
    return;
  }
  ...

4. The remaining cleanup paths do not apply:
   • unmountAllComponents runs only via onPageUnloaded (Turbo/Turbolinks soft-navigation page swap, per its own doc comment) — never in a non-Turbo app.
   • The re-render replacement path in renderElement (L383–L411) only fires if something later calls reactOnRailsPageLoaded/renderComponent for the same domNodeId; a node removed by app code (modal closed, list item deleted, innerHTML swap) and never re-rendered under that id stays in the map forever.

Failure scenario

A non-Turbo app (plain server-rendered pages, or an SPA-ish page that adds/removes DOM via its own code) uses hydrate_on: :visible for below-the-fold islands inside dismissible/replaceable containers:

1. Island renders with hydrate_on: :visible; a scheduled entry is stored and an IntersectionObserver attached.
2. Before the island scrolls into view, app code removes its container (closes a panel, re-renders a list, swaps a tab's innerHTML).
3. The observer fires (removal triggers an intersection record with the detached target, or the next viewport change does), hits the guard, disconnects, and returns.
4. The renderedRoots entry keeps the detached node's entire subtree, its props payload in adjacent script text if referenced, and the cancel closure reachable until full page unload. Repeat per island — e.g. an infinite-scroll feed that virtualizes items — and retained memory grows monotonically.

Impact

• Per-island retained-memory leak (detached DOM subtree + closures) for hydrate_on: :visible nodes removed outside navigation; page-lifetime in non-Turbo apps.
• Secondary correctness wrinkle: the stale map entry means a later reactOnRailsPageLoaded() for the same domNodeId takes the teardown-and-replace path instead of the fresh path — harmless today, but the map's contents lie about what is live.
• No user-visible error; found only via heap profiling. hydrate_on is a new 16.x feature (Declarative hydration scheduling: hydrate_on: :visible / :idle / :interaction #3890/Add OSS hydrate_on scheduling #4037), so usage is still growing.

Suggested fix

Invoke the callback instead of returning; runScheduledRender already contains the disconnected-node cleanup:

// ClientRenderer.ts, scheduleWhenVisible observer callback
const target = entries[0]?.target;
if (target && !target.isConnected) {
  observer.disconnect();
  callback();   // runScheduledRender deletes the stale renderedRoots entry and returns
  return;
}

This preserves the guard's original purpose (release the observer + detached node reference) and additionally releases the renderedRoots entry. runScheduledRender is idempotent for this case: it checks entry identity, sees !domNode.isConnected, deletes the entry, and returns without mounting. No behavior change for connected nodes.

Test plan

• Unit test alongside the existing hydrate_on tests (packages/react-on-rails/tests/, IntersectionObserver is already mocked there for Add OSS hydrate_on scheduling #4037's tests):
  1. Render a component with data-hydrate-on="visible".
  2. Remove the DOM node from the document.
  3. Trigger the mocked observer callback with an entry whose target.isConnected === false.
  4. Assert the internal roots map no longer contains the domNodeId (observable indirectly: a subsequent reactOnRailsPageLoaded() for a new node with the same id takes the fresh-schedule path, or expose the map size via a test-only hook as done for other renderer tests).
  5. Assert the observer was disconnected and no mount/hydration was attempted.
• Run: cd packages/react-on-rails && pnpm test ClientRenderer (or the repo's pnpm run test for the core package).

Notes

• Verification: read current ClientRenderer.ts at origin/main (5264192); confirmed the guard returns without calling callback() (L173–L178), that runScheduledRender deletes the entry when disconnected (L467–L470), that the entry is only otherwise removed via unmountAllComponents (page-unload, L546–L558) or a same-id re-render, and that the guard originated in Add OSS hydrate_on scheduling #4037.
• Confidence: high; the leak is a straightforward reachability argument (module-scope renderedRoots map → entry → domNode).
• Related findings by title: none in the core package; filed alongside four Pro-package findings from the same review.

Found in a post-2026-06-12 code review of merged changes (review base 43ed0e1). Filed by Claude Code on behalf of Justin Gordon.