Commit 0ebdff0
## Summary
Follow-up to #3209, after **PR #3576** (the renderer-function teardown
contract) merged into `main` as
`55fcdcf4af0629f32969dfa55f6fd71794ce200f`. This adopts the explicit `{
teardown }` wrapper contract across the in-tree dummy renderer functions
and documents the optional teardown wrapper return value. **None of
these change the public runtime contract** — they adopt it in-tree and
document it, exactly the checklist deferred in #3578.
> ✅ **Retargeted to main.** #3576 has landed, so #3585 is now based on
`main` and the head branch has been rebased onto `origin/main`.
Closes #3578.
## 1. In-tree dummy renderers → return `{ teardown }`
Each renderer function (3-arg `(props, railsContext, domNodeId)` form)
now captures its React root and returns `{ teardown: () => ... }` so
React on Rails unmounts it on Turbo/Turbolinks navigation (page unload)
or same-id node replacement instead of leaking it.
**Decision:** use the explicit `{ teardown }` wrapper, not a bare
callback. #3576 made the runtime/type contract explicit so legacy
3-argument render functions that happened to return a function are not
misclassified as cleanup callbacks.
**OSS modern** (`react_on_rails/spec/dummy/client/app/startup/`):
- `ManualRenderApp.jsx`
- `ReduxApp.client.jsx` — `module.hot` path now reuses the existing root
instead of creating a second root
- `ReduxSharedStoreApp.client.jsx` — `module.hot` path now reuses the
existing root instead of creating a second root
**OSS legacy React 16**
(`react_on_rails/spec/dummy/client/app-react16/startup/`):
- `ManualRenderApp.jsx`, `ReduxApp.client.jsx`,
`ReduxSharedStoreApp.client.jsx` — teardown unmounts via
`ReactDOM.unmountComponentAtNode(domNode)` (no root handle in the React
16/17 API)
**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:**
- `loadable/loadable-client.imports-loadable.jsx` — returns a **promise
resolving to** `{ teardown }`, since the root is created inside
`loadableReady()`
- `@loadable/component` version confirmation: the Pro dummy app declares
`^5.16.3` and the lockfile resolves `5.16.7`, above the Promise API
floor (`5.12.0`).
### A note on the `module.hot` paths
The React 18 `createRoot` renderers (OSS modern Redux apps) now
re-render on the same root during hot reload, so HMR no longer leaks a
root or calls `createRoot` twice on the same node. The React 16/17
renderers re-render into the same container idempotently
(`ReactDOM.render`/`hydrate`), so there is no separate root to unmount
first — their HMR path is unchanged, and the teardown handles
unload/replacement cleanup.
## 2. Docs
- `docs/oss/core-concepts/render-functions.md` — the `LazyHydrate`
example captures the root and resolves to `{ teardown }`.
- `docs/oss/api-reference/view-helpers-api.md` — new **"Cleaning up on
Turbo/Turbolinks navigation"** subsection with a Turbo-navigation
example (React 18 + legacy) and the async best-effort caveat.
- `docs/oss/api-reference/javascript-api.md` — short reference note on
the `{ teardown }` return value.
## 3. Imperative `ReactOnRails.render(...)` — decision recorded
Flagged as out of scope in #3209 ("same family of problem; different
surface; deferred").
**Decision: document the caller's responsibility; do not auto-track.** I
added a `@remarks` block to the `render()` JSDoc in
`packages/react-on-rails/src/types/index.ts` stating that a root created
by this imperative API is **not** tracked internally and the caller must
`unmount()` it themselves (e.g. on `turbo:before-render`), or use a
renderer function for automatic cleanup.
Rationale: auto-tracking these roots in `renderedRoots` would be a
behavior change that risks **double-unmount** for callers that already
manage `unmount()` themselves, and #3209 explicitly deferred this
surface. Documentation-only, no behavior change. If we later want
tracked imperative renders, that can be a separate, opt-in API.
## Retarget/rebase status
- #3576 merged into `main` as `55fcdcf4af0629f32969dfa55f6fd71794ce200f`
on 2026-06-05.
- #3585 auto-closed when the old stacked base branch was deleted, then
was reopened and retargeted to `main`.
- The #3585 head branch was rebased onto latest `origin/main` and
force-pushed with lease; current rebased top commit is `667d1954`.
## Follow-up
Non-blocking review nits from the final green #3585 pass are tracked in
#3689 instead of delaying this PR. The `@loadable/component` Promise API
floor is recorded above, and the legacy React 16 dummy `props` mutation
cleanup is intentionally deferred because #3585 preserves existing
fixture behavior while adding teardown.
The intentionally deferred surface remains the imperative
`ReactOnRails.render(...)` API documented below; if tracked imperative
roots are desired later, that should be a separate opt-in API issue/PR.
## Acceptance criteria
- [x] All in-tree dummy renderer functions return `{ teardown }` that
unmounts their root (and the `module.hot` createRoot path reuses the
previous root instead of creating a new one).
- [x] Renderer-function docs show the optional `{ teardown }` return
value with a Turbo-navigation example.
- [x] A decision (documentation) is recorded for the imperative
`ReactOnRails.render(...)` path.
## Verification
- Rebased #3585 branch onto latest `origin/main` after #3576 merged; no
conflicts.
- `pnpm exec prettier --check ...changed files...` — clean
- `pnpm exec eslint --report-unused-disable-directives ...changed JS/TS
files...` — clean
- `pnpm --filter react-on-rails run type-check` — clean
- `pnpm --filter react-on-rails-pro run type-check` — clean
- `pnpm --dir react_on_rails_pro/spec/dummy exec tsc -p tsconfig.json
--noEmit --pretty false` — no new errors; still reports the 4
pre-existing `tests/strict-mode-support.test.tsx` unknown-prop errors
documented before
- `git diff --check origin/main...HEAD` — clean
- `pnpm run lint` — currently blocked by unrelated
`test/shakaperf/rsc-fouc` artifact lint errors on main; those files are
not touched by this PR.
No CHANGELOG entry: per the project changelog guidelines these are test
fixtures + docs + a JSDoc comment (not user-visible behavior), and #3576
already documents the user-facing contract.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Changes are limited to test fixtures, documentation, and JSDoc; public
teardown behavior was already introduced in the prior contract PR.
>
> **Overview**
> Adopts the **optional `{ teardown }` wrapper** from the merged
renderer teardown contract across in-tree dummy **3-argument renderer
functions**, plus documentation and a JSDoc note—**no new runtime
behavior** in this PR.
>
> **Dummy apps:** OSS React 16 and Pro auto-load/loadable client
startups now resolve the mount DOM node up front (throw if missing),
capture the React root (or legacy container), and **return `{ teardown:
() => root.unmount() }`** (or a **promise** that resolves to it for
`loadableReady`). That lets React on Rails unmount renderer-owned trees
on Turbo/Turbolinks navigation and same-id node replacement instead of
leaking roots.
>
> **Docs:** `render-functions.md` and `view-helpers-api.md` document
optional teardown (React 18 + legacy examples, hydrate-vs-render notes,
async teardown best-effort in OSS). `javascript-api.md` briefly mentions
the return shape on 3-param renderers.
>
> **Types:** `ReactOnRails.render()` JSDoc now states that **imperative
mounts are not auto-tracked**—callers must `unmount()` themselves or use
a renderer function with `{ teardown }` for automatic cleanup.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
200865b. 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
* **New Features**
* Renderer functions and client entrypoints may return optional teardown
handlers so mounted React roots are unmounted cleanly during
Turbo/Turbolinks navigation or same-id node replacement.
* Client startups now validate target DOM nodes before hydrating.
* **Documentation**
* Expanded API and core-concepts docs with examples for teardown return
values, hydration patterns, and sync/async teardown behavior.
* **Bug Fixes**
* Prevents leaked mounted React roots across navigations and
hot-reloads.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent af4d304 commit 0ebdff0
13 files changed
Lines changed: 202 additions & 27 deletions
File tree
- docs/oss
- api-reference
- core-concepts
- packages/react-on-rails/src/types
- react_on_rails_pro/spec/dummy/client/app
- loadable
- ror-auto-load-components
- react_on_rails/spec/dummy/client/app-react16/startup
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | | - | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
34 | 37 | | |
35 | 38 | | |
36 | 39 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
109 | 162 | | |
110 | 163 | | |
111 | 164 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| 23 | + | |
| 24 | + | |
23 | 25 | | |
24 | 26 | | |
25 | 27 | | |
| |||
38 | 40 | | |
39 | 41 | | |
40 | 42 | | |
41 | | - | |
42 | | - | |
43 | | - | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
44 | 47 | | |
45 | | - | |
46 | | - | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
47 | 54 | | |
48 | | - | |
49 | 55 | | |
50 | 56 | | |
51 | 57 | | |
52 | 58 | | |
| 59 | + | |
| 60 | + | |
53 | 61 | | |
54 | 62 | | |
55 | 63 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
524 | 524 | | |
525 | 525 | | |
526 | 526 | | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
527 | 538 | | |
528 | 539 | | |
529 | 540 | | |
| |||
Lines changed: 12 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
16 | 23 | | |
17 | 24 | | |
18 | 25 | | |
19 | 26 | | |
20 | 27 | | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
21 | 33 | | |
Lines changed: 18 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
| 26 | + | |
| 27 | + | |
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
30 | 39 | | |
31 | 40 | | |
32 | 41 | | |
| |||
39 | 48 | | |
40 | 49 | | |
41 | 50 | | |
| 51 | + | |
| 52 | + | |
42 | 53 | | |
43 | 54 | | |
44 | 55 | | |
45 | 56 | | |
46 | 57 | | |
47 | 58 | | |
48 | 59 | | |
49 | | - | |
| 60 | + | |
50 | 61 | | |
51 | 62 | | |
52 | 63 | | |
| |||
57 | 68 | | |
58 | 69 | | |
59 | 70 | | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
60 | 76 | | |
Lines changed: 18 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
| 21 | + | |
21 | 22 | | |
22 | 23 | | |
23 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
24 | 33 | | |
25 | 34 | | |
26 | 35 | | |
| |||
29 | 38 | | |
30 | 39 | | |
31 | 40 | | |
| 41 | + | |
| 42 | + | |
32 | 43 | | |
33 | 44 | | |
34 | 45 | | |
35 | 46 | | |
36 | 47 | | |
37 | 48 | | |
38 | | - | |
| 49 | + | |
39 | 50 | | |
40 | 51 | | |
41 | 52 | | |
| |||
45 | 56 | | |
46 | 57 | | |
47 | 58 | | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
48 | 64 | | |
Lines changed: 11 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
12 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
13 | 20 | | |
14 | 21 | | |
15 | 22 | | |
16 | 23 | | |
17 | 24 | | |
18 | | - | |
| 25 | + | |
| 26 | + | |
19 | 27 | | |
20 | | - | |
21 | 28 | | |
22 | 29 | | |
Lines changed: 11 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
26 | 32 | | |
27 | 33 | | |
28 | 34 | | |
29 | 35 | | |
30 | 36 | | |
31 | | - | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
32 | 42 | | |
Lines changed: 5 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
20 | 19 | | |
21 | 20 | | |
22 | 21 | | |
23 | | - | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
24 | 27 | | |
0 commit comments