Harden renderer teardown polish

Closes #3647

## Summary
- Centralize `isRendererTeardownResult` in
`packages/react-on-rails/src/rendererTeardown.ts` and expose it as
`react-on-rails/@internal/rendererTeardown`, so OSS and Pro share one
teardown-result guard instead of carrying inline copies.
- Keep the Pro `wrapServerComponentRenderer/client` runtime guard so
teardown-shaped server-component render results fail with a clear error
before the generic component-shape check.
- Harden and clarify the renderer teardown tests around same-id async
races, DOM descriptor reuse, and Pro wrapper validation cleanup.
- Keep Pro parity coverage for non-native thenable teardown rejection
handling.

## Tests
- `pnpm install --frozen-lockfile`
- `pnpm exec prettier --check
packages/react-on-rails/src/rendererTeardown.ts
packages/react-on-rails/src/ClientRenderer.ts
packages/react-on-rails/src/createReactOutput.ts
packages/react-on-rails-pro/src/ClientSideRenderer.ts
packages/react-on-rails-pro/src/wrapServerComponentRenderer/client.tsx
packages/react-on-rails/tests/ClientRenderer.test.ts
packages/react-on-rails/package.json`
- `pnpm exec eslint --no-warn-ignored
packages/react-on-rails/src/rendererTeardown.ts
packages/react-on-rails/src/ClientRenderer.ts
packages/react-on-rails/src/createReactOutput.ts
packages/react-on-rails-pro/src/ClientSideRenderer.ts
packages/react-on-rails-pro/src/wrapServerComponentRenderer/client.tsx
packages/react-on-rails/tests/ClientRenderer.test.ts`
- `pnpm --filter react-on-rails run type-check`
- `pnpm --filter react-on-rails-pro run type-check`
- `pnpm --filter react-on-rails exec jest tests/ClientRenderer.test.ts
--runInBand`
- `pnpm --filter react-on-rails-pro exec jest
tests/ClientSideRenderer.test.ts
tests/wrapServerComponentRenderer.client.test.jsx --runInBand`
- `pnpm --filter react-on-rails run build`
- `pnpm --filter react-on-rails-pro run build`
- Follow-up focused reruns after review-comment cleanup: `pnpm exec
prettier --check packages/react-on-rails/tests/ClientRenderer.test.ts
packages/react-on-rails-pro/tests/wrapServerComponentRenderer.client.test.jsx`,
`pnpm exec eslint --no-warn-ignored
packages/react-on-rails/tests/ClientRenderer.test.ts
packages/react-on-rails-pro/tests/wrapServerComponentRenderer.client.test.jsx`,
`pnpm --filter react-on-rails exec jest tests/ClientRenderer.test.ts
--runInBand`, `pnpm --filter react-on-rails-pro exec jest
tests/wrapServerComponentRenderer.client.test.jsx --runInBand`.

## Design Calls
- Added an internal OSS export instead of documenting three synchronized
`isRendererTeardownResult` copies; Pro already consumes OSS internal
subpaths, and this keeps the guard shape single-sourced.
- Kept `invokeRendererTeardown` duplicated; that helper still has
package-specific runtime/logging context and remains covered by parity
tests.
- Kept public `RendererFunction` return typing unchanged pending
downstream migration data.
- Left docs and dummy-app renderer conversions out of this PR per scope;
this PR stays focused on renderer teardown diagnostics, guard reuse, and
targeted tests.
- No changelog entry: this is Pro/OSS renderer hardening plus an
invalid-shape diagnostic guard, not a new user-facing feature or
documented behavior change.

Labels: none. This is a focused test/diagnostic hardening PR with no CI
workflow, dependency, generator, SSR output, or broad Pro/core boundary
changes.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Internal guard deduplication and stricter runtime validation for
invalid server-component render shapes; behavior changes are limited to
clearer errors and test-documented teardown edge cases, with no auth or
data-path changes.
> 
> **Overview**
> **Centralizes** the `isRendererTeardownResult` type guard in
`rendererTeardown.ts` and exposes it as
`react-on-rails/@internal/rendererTeardown`, replacing inline copies in
OSS (`ClientRenderer`, `createReactOutput`) and Pro
(`ClientSideRenderer`, `wrapServerComponentRenderer/client`).
> 
> **Pro RSC wrapper** now rejects server-component render functions that
resolve to a teardown object with an explicit error (before DOM/context
checks), closing #3647-style misconfiguration diagnostics.
> 
> **Tests** add coverage for non-native thenable teardown rejections
(Pro), same-id async teardown races where a stale renderer must not
overwrite a newer mount (OSS), and `wrapServerComponentRenderer`
validation for missing `domNodeId`, DOM node, `railsContext`, and
teardown-shaped returns; webpack manifest test aliases the new internal
module.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
b92db44. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Added runtime validation that rejects invalid server-rendered results
(e.g., teardown objects) with clearer error messages.
* Improved handling and logging when asynchronous renderer teardown or
replacement fails or races.
* Improved validation and error messages for missing mount target or
missing context during server-component mounting.

* **Tests**
  * Expanded tests for server component rendering validation edge cases.
* Added tests ensuring async renderer teardown/replacement logs
correctly for promise/thenable scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: justin808

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

@@ -20,12 +20,12 @@ import type {
   RegisteredComponent,
   RendererFunction,
   RendererTeardown,
-  RendererTeardownResult,
   Root,
 } from 'react-on-rails/types';
 
 import { getRailsContext, resetRailsContext } from 'react-on-rails/context';
 import createReactOutput from 'react-on-rails/createReactOutput';
+import { isRendererTeardownResult } from 'react-on-rails/@internal/rendererTeardown';
 import { isServerRenderHash } from 'react-on-rails/isServerRenderResult';
 import { supportsHydrate, supportsRootApi, unmountComponentAtNode } from 'react-on-rails/reactApis';
 import reactHydrateOrRender from 'react-on-rails/reactHydrateOrRender';
@@ -74,14 +74,6 @@ function invokeRendererTeardown(teardown: RendererTeardown | undefined, domNodeI
   }
 }
 
-function isRendererTeardownResult(value: unknown): value is RendererTeardownResult {
-  return (
-    value != null &&
-    typeof value === 'object' &&
-    typeof (value as { teardown?: unknown }).teardown === 'function'
-  );
-}
-
 // Result of attempting renderer delegation. Pro awaits the renderer inside delegateToRenderer, so it
 // pre-resolves to an optional teardown. The core renderer has its own DelegationResult that instead
 // carries the raw (possibly still-pending) RendererResult because it cannot await; the two

packages/react-on-rails-pro/src/wrapServerComponentRenderer/client.tsx

@@ -26,6 +26,7 @@ import * as React from 'react';
 import * as ReactDOMClient from 'react-dom/client';
 import { ReactComponent, ReactComponentOrRenderFunction, RendererFunction } from 'react-on-rails/types';
 import isRenderFunction from 'react-on-rails/isRenderFunction';
+import { isRendererTeardownResult } from 'react-on-rails/@internal/rendererTeardown';
 import { ensureReactUseAvailable } from 'react-on-rails/reactApis';
 import { createRSCProvider } from '../RSCProvider.tsx';
 import getReactServerComponent from '../getReactServerComponent.client.ts';
@@ -75,6 +76,16 @@ const wrapServerComponentRenderer = (
       ? ((await componentOrRenderFunction(props, railsContext, domNodeId)) as ReactComponent)
       : componentOrRenderFunction;
 
+    // Preserve compatibility with existing 3-arg render functions: they are awaited before domNodeId
+    // validation; moving that check earlier is a future improvement. Keep this teardown-result guard
+    // before DOM checks so wrong-shaped render results get the clearer error.
+    if (isRendererTeardownResult(Component)) {
+      throw new Error(
+        `wrapServerComponentRenderer: render function for server component '${componentName}' ` +
+          'returned a renderer teardown result; expected a React component.',
+      );
+    }
+
     if (typeof Component !== 'function') {
       throw new Error(`wrapServerComponentRenderer: component '${componentName}' is not a function`);
     }

packages/react-on-rails-pro/tests/ClientSideRenderer.test.ts

@@ -395,6 +395,42 @@ describe('ClientSideRenderer', () => {
     }
   });
 
+  it('logs a non-native thenable teardown rejection without requiring .catch', async () => {
+    const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+    try {
+      const rejection = new Error('thenable teardown boom');
+      const teardown = jest.fn(
+        () =>
+          ({
+            then(_onFulfilled: () => void, onRejected: (error: Error) => void) {
+              onRejected(rejection);
+            },
+          }) as unknown as Promise<void>,
+      );
+      const TestRenderer: RendererFunction = (
+        _props?: Record<string, unknown>,
+        _railsContext?: RailsContext,
+        _domNodeId?: string,
+      ) => ({ teardown });
+      ComponentRegistry.register({ TestComponent: TestRenderer });
+      const componentSpec = setupTestComponentDom('dom-id-teardown-thenable-reject');
+      addRailsContext();
+
+      await renderOrHydrateComponent(componentSpec);
+      unmountAll();
+      expect(teardown).toHaveBeenCalledTimes(1);
+
+      await Promise.resolve(); // lets Promise.resolve(nonNativeThenable) settle
+      await Promise.resolve(); // lets the .catch() rejection handler run
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        'Error in renderer teardown for dom node "dom-id-teardown-thenable-reject":',
+        rejection,
+      );
+    } finally {
+      consoleErrorSpy.mockRestore();
+    }
+  });
+
   it('rejects with a render error when an async renderer rejects before returning a teardown', async () => {
     const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
     try {

packages/react-on-rails-pro/tests/wrapServerComponentRenderer.client.manifest.test.js

@@ -40,6 +40,7 @@ const runWebpack = () => {
     },
     resolve: {
       alias: {
+        'react-on-rails/@internal/rendererTeardown$': path.join(reactOnRailsSrcRoot, 'rendererTeardown.ts'),
         'react-on-rails/isRenderFunction$': path.join(reactOnRailsSrcRoot, 'isRenderFunction.ts'),
         'react-on-rails/reactApis$': path.join(reactOnRailsSrcRoot, 'reactApis.cts'),
         'react-on-rails/types$': path.join(reactOnRailsSrcRoot, 'types/index.ts'),

packages/react-on-rails-pro/tests/wrapServerComponentRenderer.client.test.jsx

@@ -37,6 +37,86 @@ const createBailoutDigestError = () => {
   return error;
 };
 
+const loadWrappedRendererWithMocks = () => {
+  jest.resetModules();
+
+  const unmount = jest.fn();
+  const render = jest.fn();
+  const hydrateRoot = jest.fn(() => ({ unmount }));
+  const createRoot = jest.fn(() => ({ render, unmount }));
+  const getReactServerComponent = jest.fn(() => jest.fn());
+
+  jest.doMock('react-dom/client', () => ({ createRoot, hydrateRoot }));
+  jest.doMock('react-on-rails-rsc/client.browser', () => ({}));
+  jest.doMock('../src/getReactServerComponent.client.ts', () => ({
+    __esModule: true,
+    default: getReactServerComponent,
+  }));
+
+  // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires
+  const wrapServerComponentRenderer = require('../src/wrapServerComponentRenderer/client.tsx').default;
+
+  return { wrapServerComponentRenderer, createRoot, hydrateRoot, render, unmount, getReactServerComponent };
+};
+
+const cleanupWrappedRendererMocks = () => {
+  document.body.innerHTML = '';
+};
+
+describe('wrapServerComponentRenderer/client validation (issue #3647)', () => {
+  const railsContext = { rscPayloadGenerationUrlPath: '/rsc_payload' };
+
+  beforeEach(() => {
+    document.body.innerHTML = '';
+  });
+
+  afterEach(() => {
+    cleanupWrappedRendererMocks();
+  });
+
+  it('rejects with an explicit message when domNodeId is missing', async () => {
+    const { wrapServerComponentRenderer } = loadWrappedRendererWithMocks();
+    const WrappedComponent = wrapServerComponentRenderer(() => null, 'MissingDomNodeIdComponent');
+
+    await expect(WrappedComponent({}, railsContext, undefined)).rejects.toThrow(
+      "RSCClientRoot: No domNodeId provided for server component 'MissingDomNodeIdComponent'",
+    );
+  });
+
+  it('rejects with an explicit message when the target DOM node is missing', async () => {
+    const { wrapServerComponentRenderer } = loadWrappedRendererWithMocks();
+    const WrappedComponent = wrapServerComponentRenderer(() => null, 'MissingDomNodeComponent');
+
+    await expect(WrappedComponent({}, railsContext, 'missing-rsc-root')).rejects.toThrow(
+      "RSCClientRoot: No DOM node found for id: missing-rsc-root (server component 'MissingDomNodeComponent')",
+    );
+  });
+
+  it('rejects with an explicit message when railsContext is missing', async () => {
+    const { wrapServerComponentRenderer } = loadWrappedRendererWithMocks();
+    const domNode = document.createElement('div');
+    domNode.id = 'rsc-root-without-context';
+    document.body.appendChild(domNode);
+    const WrappedComponent = wrapServerComponentRenderer(() => null, 'MissingRailsContextComponent');
+
+    await expect(WrappedComponent({}, undefined, domNode.id)).rejects.toThrow(
+      "RSCClientRoot: No railsContext provided for server component 'MissingRailsContextComponent'.",
+    );
+  });
+
+  it('rejects a render function that resolves to a renderer teardown result', async () => {
+    const { wrapServerComponentRenderer } = loadWrappedRendererWithMocks();
+    const renderFunction = (_props, _railsContext, _domNodeId) => ({ teardown: jest.fn() });
+    const WrappedComponent = wrapServerComponentRenderer(renderFunction, 'TeardownResultComponent');
+
+    // No DOM node needed; the teardown-result guard throws before getElementById is called.
+    await expect(WrappedComponent({}, railsContext, 'nonexistent-dom-id')).rejects.toThrow(
+      "wrapServerComponentRenderer: render function for server component 'TeardownResultComponent' " +
+        'returned a renderer teardown result; expected a React component.',
+    );
+  });
+});
+
 describe('wrapServerComponentRenderer/client recoverable errors', () => {
   let originalReportErrorDescriptor;
 

packages/react-on-rails/package.json

@@ -52,6 +52,7 @@
     "./ReactDOMServer": "./lib/ReactDOMServer.cjs",
     "./serverRenderReactComponent": "./lib/serverRenderReactComponent.js",
     "./@internal/sanitizeNonce": "./lib/sanitizeNonce.js",
+    "./@internal/rendererTeardown": "./lib/rendererTeardown.js",
     "./@internal/base/client": "./lib/base/client.js",
     "./@internal/base/full": {
       "react-server": "./lib/base/full.rsc.js",

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

@@ -4,7 +4,6 @@ import type {
   RailsContext,
   RendererFunction,
   RendererTeardown,
-  RendererTeardownResult,
   RenderReturnType,
 } from './types/index.ts';
 import ComponentRegistry from './ComponentRegistry.ts';
@@ -15,6 +14,7 @@ import { getRailsContext } from './context.ts';
 import { isServerRenderHash } from './isServerRenderResult.ts';
 import { onPageUnloaded } from './pageLifecycle.ts';
 import { supportsRootApi, unmountComponentAtNode } from './reactApis.cts';
+import { isRendererTeardownResult } from './rendererTeardown.ts';
 
 const REACT_ON_RAILS_STORE_ATTRIBUTE = 'data-js-react-on-rails-store';
 
@@ -41,14 +41,6 @@ function isThenable(value: unknown): value is PromiseLike<unknown> {
   );
 }
 
-function isRendererTeardownResult(value: unknown): value is RendererTeardownResult {
-  return (
-    value != null &&
-    typeof value === 'object' &&
-    typeof (value as { teardown?: unknown }).teardown === 'function'
-  );
-}
-
 // Track all rendered roots for cleanup
 const renderedRoots = new Map<string, RenderedEntry>();
 

packages/react-on-rails/src/createReactOutput.ts

@@ -1,25 +1,12 @@
 import { createElement, isValidElement, type ReactElement } from 'react';
-import type {
-  CreateParams,
-  ReactComponent,
-  RenderFunction,
-  CreateReactOutputResult,
-  RendererTeardownResult,
-} from './types/index.ts';
+import type { CreateParams, ReactComponent, RenderFunction, CreateReactOutputResult } from './types/index.ts';
 import { isServerRenderHash, isPromise } from './isServerRenderResult.ts';
+import { isRendererTeardownResult } from './rendererTeardown.ts';
 
 const unsupportedManualRendererMessage = (name: string) =>
   `ReactOnRails.render() does not support renderer functions ("${name}"). ` +
   'Use normal React on Rails component rendering so renderer teardowns are captured on navigation.';
 
-function isRendererTeardownResult(value: unknown): value is RendererTeardownResult {
-  return (
-    value != null &&
-    typeof value === 'object' &&
-    typeof (value as { teardown?: unknown }).teardown === 'function'
-  );
-}
-
 function createReactElementFromRenderFunctionResult(
   renderFunctionResult: ReactComponent,
   name: string,

packages/react-on-rails/src/rendererTeardown.ts

@@ -0,0 +1,10 @@
+import type { RendererTeardownResult } from './types/index.ts';
+
+// eslint-disable-next-line import/prefer-default-export -- single-export module; named export keeps the type guard's API tied to its predicate name.
+export function isRendererTeardownResult(value: unknown): value is RendererTeardownResult {
+  return (
+    value != null &&
+    typeof value === 'object' &&
+    typeof (value as { teardown?: unknown }).teardown === 'function'
+  );
+}