Renderer functions are never cleaned up on unmount/page-unload (memory leak)

[AbanoubGhadban]

Summary

When a registered component is a renderer function (3-arg form: (props, railsContext, domNodeId) => …), React on Rails delegates rendering to the user's function and never tracks any cleanup state for it. As a result:

• On Turbo / Turbolinks navigation (turbo:before-render, turbolinks:before-render, page:before-unload), the framework unmounts only roots it created itself; the renderer-function's mount is silently leaked.
• When the same domNodeId is replaced (e.g. async HTML injection), the framework cleans up its own roots but cannot clean up a renderer-function mount in the replaced node.
• The user has no contract through which to register cleanup — the RenderFunction type returns void/Promise<void> and RegisteredComponent has no teardown field.

This is the gap; renderer-function authors must hand-wire their own listeners on every Turbo event, and most don't.

Where this happens (current main)

Core — packages/react-on-rails/src/ClientRenderer.ts

• delegateToRenderer invokes the renderer and returns true — lines 36–65.
• renderElement returns immediately at lines 118–120 when delegation succeeds, so the root tracking at line 143 (renderedRoots.set(domNodeId, { root, domNode })) is never executed for renderer functions.
• unmountAllComponents at lines 207–222 only iterates renderedRoots, so renderer functions get no callback on onPageUnloaded (registered at line 225).
• The replaced-node cleanup path at lines 96–114 has the same gap.

Pro — packages/react-on-rails-pro/src/ClientSideRenderer.ts

• delegateToRenderer (lines 32–54) awaits the renderer and returns; the surrounding ComponentRenderer.render (lines ~115–120) early-returns without setting this.root.
• ComponentRenderer.unmount (lines ~124–150) only handles the React-root path, so renderer-function mounts are never torn down.

Type / registry

• RenderFunction in packages/react-on-rails/src/types/index.ts is (props, railsContext, domNodeId?) => ReactComponent | Promise<…>; no teardown contract.
• ComponentRegistry.ts (core line 27–28; Pro line 43) detects isRenderer = renderFunction && component.length === 3 and stores { name, component, renderFunction, isRenderer }.

Tests

• packages/react-on-rails/tests/ClientRenderer.test.ts:126 covers ‘handles renderer functions correctly' but only asserts no error is thrown — no cleanup assertions.

Reproduction

function MyRenderer(props, railsContext, domNodeId) {
  const node = document.getElementById(domNodeId);
  const root = ReactDOM.createRoot(node);
  root.render(<App {...props} />);
  // No way to tell ReactOnRails how to unmount this on Turbo navigation.
}
ReactOnRails.register({ MyRenderer });

Navigate via Turbo. The original root is leaked (its DOM node may be cached by Turbo, event listeners and React state retained). On a long-lived session this accumulates.

Proposed solution: let renderer functions return a cleanup function

Extend the RenderFunction contract so a renderer may return a teardown callback (or a promise resolving to one):

type RendererTeardown = () => void | Promise<void>;

type RenderFunction = (
  props: Record<string, unknown>,
  railsContext: RailsContext,
  domNodeId: string,
) => void | RendererTeardown | Promise<void | RendererTeardown>;

Implementation sketch

Core (ClientRenderer.ts):

1. Have delegateToRenderer capture the return value:

   const result = (component as RenderFunction)(props, railsContext, domNodeId);

2. Track renderer entries alongside React roots, e.g. extend the existing map:

   type RenderedEntry =
     | { kind: 'react'; root: RenderReturnType; domNode: Element }
     | { kind: 'renderer'; teardown?: RendererTeardown; domNode: Element };
   const renderedRoots = new Map<string, RenderedEntry>();

3. If result is a function (or resolves to one), store it as teardown. Else store teardown: undefined — we still record the entry so the replaced-node path can drop it.
4. unmountAllComponents and the replaced-node branch invoke teardown?.() for renderer entries (wrapped in try/catch like the existing root.unmount() calls), and call root.unmount() for React entries.

Pro (ClientSideRenderer.ts):

Mirror the change: ComponentRenderer gains a teardown?: RendererTeardown field set when delegation returns one; unmount() calls it for the renderer-function branch.

Why this shape

• Backward compatible. Existing renderer functions return nothing and continue to work; the framework just no-ops the teardown.
• No new public registration surface (vs. an alternative ReactOnRails.registerRendererCleanup(name, fn) API). The teardown closes over the same scope as the mount, which is what users actually need.
• Symmetric with React's existing patterns (useEffect cleanup, unmountComponentAtNode).
• Composable with async renderers. Pro already awaits the renderer; core can do the same when the return is a thenable.

Acceptance criteria

• RenderFunction type accepts an optional RendererTeardown (sync or async) return.
• Core and Pro client renderers store the teardown alongside the rendered entry for that domNodeId.
• On Turbo / Turbolinks page-unload, all renderer-function teardowns are invoked, errors caught and logged.
• On same-id node replacement, the prior teardown is invoked before the new mount runs.
• Tests:
  • ClientRenderer.test.ts: renderer returning a teardown gets called on onPageUnloaded and on node replacement; renderer returning nothing does not throw on unmount.
  • Pro ClientSideRenderer analog.
• Docs: update the renderer-functions docs page (referenced from Review and fix docs for render functions, renderer functions, react_component_hash, registerServerComponent, and wrapServerComponentRenderer #2911) with the new optional teardown return value and a Turbo-navigation example.

No prior issue tracking this

Searched gh search issues for: renderer function, renderer function cleanup, renderer function unmount, registerRenderer, domNodeId cleanup, memory leak, unmount. Closest tangential matches (#196, #1223, #629, #706, #1139, #1029) are all old/closed and don't cover this case.

Tasks

Framework API (the new contract)

• Extend RenderFunction in packages/react-on-rails/src/types/index.ts to allow returning void | RendererTeardown | Promise<void | RendererTeardown> where RendererTeardown = () => void | Promise<void>.
• packages/react-on-rails/src/ClientRenderer.ts: capture the renderer's return value in delegateToRenderer, track it in the rendered-roots map (extend the entry shape to a discriminated union of react | renderer), and invoke the teardown from both unmountAllComponents and the same-id replaced-node branch (lines 96-114). Wrap teardown calls in try/catch and log on failure.
• packages/react-on-rails-pro/src/ClientSideRenderer.ts: mirror the change in ComponentRenderer — store the teardown when delegateToRenderer resolves, invoke it from unmount(). Maintain the existing 'unmounted' state guard so a teardown isn't called after unmount() already ran.

Framework-shipped renderer (the highest-impact in-tree leak)

• packages/react-on-rails-pro/src/wrapServerComponentRenderer/client.tsx:52-99: capture hydrateRoot/createRoot return value and return () => root.unmount() instead of ''. Removes the // TODO: fix this behavior comment. This single change closes the leak for every registerServerComponent user.

Tests

• Replace packages/react-on-rails/tests/ClientRenderer.test.ts:126-173 — currently registers jest.fn() (which has .length === 0, so the registry never marks it as a renderer) and ends with expect(true).toBe(true). Replace with real assertions: register a 3-arg renderer, verify it's invoked, verify its returned teardown runs on onPageUnloaded and on same-id node replacement, verify a renderer that returns undefined does not throw on unmount.
• Add Pro analog in packages/react-on-rails-pro/tests/ covering the same cases through ComponentRenderer.unmount().
• Add a wrapServerComponentRenderer client test asserting the returned teardown unmounts the React root.

Convert in-tree dummy renderer functions to return teardowns

OSS modern (react_on_rails/spec/dummy/client/app/startup/):

• ManualRenderApp.jsx
• ReduxApp.client.jsx
• ReduxSharedStoreApp.client.jsx (also has module.hot re-mount path that needs to unmount the previous root first)

OSS legacy (react_on_rails/spec/dummy/client/app-react16/startup/):

• ManualRenderApp.jsx
• ReduxApp.client.jsx
• ReduxSharedStoreApp.client.jsx

Pro auto-load (react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/):

• ManualRenderApp.jsx
• ReduxApp.client.jsx
• ReduxSharedStoreApp.client.jsx
• ApolloGraphQLApp.client.jsx
• LazyApolloGraphQLApp.client.tsx

Pro loadable:

• react_on_rails_pro/spec/dummy/client/app/loadable/loadable-client.imports-loadable.jsx

Tip: the Pro hydrateOrRender helper at ror-auto-load-components/ManualRenderApp.jsx:6-14 already returns the root — each renderer there only needs to capture it and return () => root.unmount().

Docs

• docs/oss/core-concepts/render-functions.md:42-50 — update the LazyHydrate example to capture the root and return a teardown.
• docs/oss/api-reference/view-helpers-api.md:100-110 — document the new optional teardown return value with a Turbo-navigation example.
• docs/oss/api-reference/javascript-api.md:32 — short reference update if needed.

Out of scope (file as a follow-up)

• ReactOnRails.render(name, props, domNodeId, hydrate) in packages/react-on-rails/src/base/client.ts:271-285 and the duplicate at packages/react-on-rails/src/capabilities/core.ts:193-207. Imperative public API: returns the root to the caller but is not tracked by renderedRoots, so callers leak across Turbo nav unless they manage unmount themselves. Same family of problem; different surface; deferred.