Skip to content

Dummy: add streaming hydration demo page#4593

Open
AbanoubGhadban wants to merge 5 commits into
mainfrom
4385-streaming-hydration-demo
Open

Dummy: add streaming hydration demo page#4593
AbanoubGhadban wants to merge 5 commits into
mainfrom
4385-streaming-hydration-demo

Conversation

@AbanoubGhadban

@AbanoubGhadban AbanoubGhadban commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

The demo illustrates:

  • Header & Hero: hydrate_on: :immediate — hydrate as soon as the bundle loads
  • Main & Sidebar: hydrate_on: :idle — hydrate when the browser is idle
  • Footer: hydrate_on: :visible — hydrate only when scrolled into view

Each section tracks its hydration state via data-hydrated attribute and records events to window.__STREAMING_HYDRATION_EVENTS__ for testing.

Note: Full streaming SSR requires React on Rails Pro's Node Renderer. This demo uses the OSS hydrate_on scheduling feature to approximate the user experience benefits.

Test plan

  • Visit /streaming_hydration_demo in the dummy app
  • Verify all sections render with SSR content initially
  • Verify header and hero hydrate immediately (buttons become clickable)
  • Verify main and sidebar hydrate during browser idle time
  • Verify footer hydrates only when scrolled into view
  • Run pnpm run lint — no errors
  • Run pnpm run type-check — no errors

Part of #4385 (demo page only — the selective-hydration investigation stays open)

🤖 Generated with Claude Code

https://claude.ai/code/session_01FYSpsrA5yeBBomT5TnBB53

Summary by CodeRabbit

  • New Features
    • Added a Streaming Hydration Demo page with a “How it works” explainer.
    • Demonstrates independent, per-section hydration for header, hero, main, sidebar, and footer.
    • Each section shows hydration status and an interactive button that activates after hydration, with click counting.
    • Supports three hydration strategies: immediate, idle, and on visibility.
    • Added a new public route to access the demo page.

Add a demo page at `/streaming_hydration_demo` that illustrates
independent section hydration using different `hydrate_on` strategies.
Each section (header, hero, main, sidebar, footer) hydrates on its own
schedule, demonstrating the concept from issue #4385.

Closes #4385

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FYSpsrA5yeBBomT5TnBB53
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds a routed dummy-app page demonstrating five independently hydrated React sections. The component tracks hydration state, records hydration events, enables interaction after hydration, and is registered in both client and server bundles.

Changes

Streaming hydration demo

Layer / File(s) Summary
Section hydration component
react_on_rails/spec/dummy/client/app/startup/StreamingHydrationDemo.tsx
Defines section-specific content, hydration event recording, data-hydrated status, and click behavior enabled after hydration.
Demo route and section rendering
react_on_rails/spec/dummy/config/routes.rb, react_on_rails/spec/dummy/app/views/pages/streaming_hydration_demo.html.erb, react_on_rails/spec/dummy/client/app/packs/client-bundle.ts, react_on_rails/spec/dummy/client/app/packs/server-bundle.ts
Adds the demo route, registers StreamingHydrationDemo in both bundles, and renders five prerendered sections with immediate, idle, and visible hydration strategies.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant RailsRoute
  participant DemoView
  participant ReactOnRails
  participant StreamingHydrationDemo
  participant WindowEvents
  Browser->>RailsRoute: GET streaming_hydration_demo
  RailsRoute->>DemoView: render five configured sections
  DemoView->>ReactOnRails: initialize registered components
  ReactOnRails->>StreamingHydrationDemo: render each section
  StreamingHydrationDemo->>WindowEvents: record hydration event
  StreamingHydrationDemo->>StreamingHydrationDemo: mark section hydrated and enable clicks
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR delivers a demo page that exercises independent hydration timings and event tracking, matching the investigation’s core goal.
Out of Scope Changes check ✅ Passed The changes stay focused on the new demo page, its component, and bundle/route wiring needed to support it.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a dummy streaming hydration demo page.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 4385-streaming-hydration-demo

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a dummy app page for streaming-style hydration demos. The main changes are:

  • A new /streaming_hydration_demo route.
  • A new ERB page with five independently hydrated sections.
  • A new StreamingHydrationDemo React component.
  • Client-bundle registration for the new component.

Confidence Score: 4/5

The new demo route can fail while server-rendering the first React section.

  • The ERB page uses prerender: true for a component registered only in the client bundle.
  • Deferred hydration modes can also raise in Pro-enabled dummy app runs.
  • The React component itself follows the existing hydration scheduling pattern.

react_on_rails/spec/dummy/app/views/pages/streaming_hydration_demo.html.erb and the dummy server bundle registration.

Important Files Changed

Filename Overview
react_on_rails/spec/dummy/app/views/pages/streaming_hydration_demo.html.erb Adds the demo page and scheduled hydration calls; the prerendered component path can fail without server-bundle registration.
react_on_rails/spec/dummy/client/app/packs/client-bundle.ts Registers the new component for client hydration.
react_on_rails/spec/dummy/client/app/startup/StreamingHydrationDemo.tsx Adds the section component and hydration event tracking with client-only effect usage.
react_on_rails/spec/dummy/config/routes.rb Adds the new dummy app route using the existing implicit view rendering pattern.

Reviews (1): Last reviewed commit: "Dummy: add streaming hydration demo page" | Re-trigger Greptile

different <code>hydrate_on</code> strategies.
</p>

<hr>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Prerendered Component Missing Server Registration

When /streaming_hydration_demo renders, this prerender: true call asks the server renderer for StreamingHydrationDemo, but the PR only registers that component in the client bundle. The comparable prerendered hydration demo registers its component in the server bundle too, so this route can fail during SSR with an unregistered-component error before the page loads.

Context Used: CLAUDE.md (source)

props: { section: "main" },
prerender: true,
id: "streaming-main",
hydrate_on: :idle) %>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Deferred Hydration Breaks Pro Runs

In Pro-enabled dummy app runs, non-:immediate hydrate_on values are rejected while rendering options are normalized. This new route reaches :idle here and :visible later in the same view, so the page can raise during ERB rendering instead of showing the demo.

Context Used: CLAUDE.md (source)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a33848ff94

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


<%= react_component("StreamingHydrationDemo",
props: { section: "header" },
prerender: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Register the demo before enabling prerendering

Visiting /streaming_hydration_demo will ask the server renderer to prerender StreamingHydrationDemo, but this commit only imports/registers it in client-bundle.ts; the server entrypoint (react_on_rails/spec/dummy/client/app/packs/server-bundle.ts) still registers only HelloString and HydrationSchedulingProbe. In this state ExecJS/Node looks up the component in the server bundle and raises “Could not find component registered with name StreamingHydrationDemo,” so the new demo page 500s before any hydration behavior can be shown.

Useful? React with 👍 / 👎.

The component was only registered in the client bundle, but needs to be
in the server bundle as well for `prerender: true` to work.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FYSpsrA5yeBBomT5TnBB53
@AbanoubGhadban

Copy link
Copy Markdown
Collaborator Author

Regarding the P2 comment about deferred hydration in Pro runs:

The new /streaming_hydration_demo page follows the same pattern as the existing /hydration_scheduling page, which also uses :idle and :visible hydrate_on modes.

Looking at the code in react_on_rails/lib/react_on_rails/react_component/render_options.rb:

  • OSS React on Rails supports :immediate, :idle, and :visible
  • React on Rails Pro currently only supports :immediate

The OSS dummy app (where this demo lives) doesn't have the Pro gem installed, so these modes work correctly. The existing hydration_scheduling.spec.js Playwright tests use the same modes without issue.

If Pro support for deferred hydration is needed in the future, that would be a separate enhancement affecting all pages using these modes, not specific to this demo.

Comment on lines +63 to +64
<p>
<a href="https://github.com/shakacode/react_on_rails/issues/4385" target="_blank">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor best-practice nit: target="_blank" without rel="noopener noreferrer" allows the opened page limited access back to window.opener (reverse tabnabbing). Low risk here since the destination is a trusted github.com URL, but it's the only target="_blank" link in the dummy app views, so worth adding rel="noopener noreferrer" for consistency/safety:

Suggested change
<p>
<a href="https://github.com/shakacode/react_on_rails/issues/4385" target="_blank">
<a href="https://github.com/shakacode/react_on_rails/issues/4385" target="_blank" rel="noopener noreferrer">

Comment on lines +15 to +21
/* eslint-disable no-underscore-dangle -- double-underscore marks the test-only window global */
window.__STREAMING_HYDRATION_EVENTS__ ||= [];
window.__STREAMING_HYDRATION_EVENTS__.push({
component,
timestamp: Date.now(),
});
/* eslint-enable no-underscore-dangle */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: the closest sibling in this file (HydrationSchedulingProbe.tsx) avoids no-underscore-dangle entirely by using bracket notation (window[hydrationSchedulingEventsKey] ||= []) instead of an eslint-disable/eslint-enable pair. Not a bug, but for consistency with the most recently added, near-identical demo component, consider the same approach here:

const streamingHydrationEventsKey = '__STREAMING_HYDRATION_EVENTS__';

const recordHydrationEvent = (component: string) => {
  window[streamingHydrationEventsKey] ||= [];
  window[streamingHydrationEventsKey].push({ component, timestamp: Date.now() });
};

HelloTurboStream,
HydrationSchedulingProbe,
ManualRenderComponent,
StreamingHydrationDemo,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note (not a bug): manual registration here is likely unnecessary. spec/dummy/config/initializers/react_on_rails.rb sets auto_load_bundle = true and components_subdirectory = "startup", so react_on_rails:generate_packs already auto-generates both client and server registration for any unsuffixed component file placed in startup/ (confirmed by sibling demo components like PureComponent, ImageExample, and CssModulesImagesFontsExample, all used with prerender: true and none manually registered here or in server-bundle.ts). Registering StreamingHydrationDemo manually is harmless (idempotent), it just mirrors an already-redundant pattern (HydrationSchedulingProbe, ManualRenderComponent, HelloTurboStream) rather than relying on the auto-generated pack.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review: Streaming Hydration Demo Page

Clean, small, self-contained addition to the dummy app. It correctly consumes the existing OSS hydrate_on scheduling feature (immediate / idle / visible, confirmed in packages/react-on-rails/src/ClientRenderer.ts) rather than reinventing it, and follows the existing pattern set by hydration_scheduling (route without an explicit controller action, relying on Rails' implicit render convention — consistent with PagesController).

Overview

  • New route streaming_hydration_demo → implicit pages#streaming_hydration_demo render.
  • New view renders 5 react_component calls (header/hero/main/sidebar/footer) with hydrate_on: :immediate|:idle|:visible.
  • New StreamingHydrationDemo.tsx component + registration in both client and server bundles.

Suggestions (non-blocking)

  1. Type safety on sectionConfig (StreamingHydrationDemo.tsx): it's typed as Record<string, Omit<SectionProps, 'testId'>> rather than Record<StreamingHydrationDemoProps['section'], ...>. With the wider string key type, a typo in a config key wouldn't be caught at compile time — the runtime if (!config) guard is the only safety net. Narrowing the key type would let TS catch missing/misspelled sections and guarantee exhaustiveness.

  2. Style inconsistency with sibling file: HydrationSchedulingProbe.tsx avoids the no-underscore-dangle lint rule by using bracket access (window[hydrationSchedulingEventsKey]), whereas this new file uses window.__STREAMING_HYDRATION_EVENTS__ wrapped in eslint-disable/eslint-enable comments. Not wrong, just a minor inconsistency in an otherwise very similar file — worth aligning for consistency across the two demo probes.

  3. No automated test coverage: there's no system/feature spec exercising the three hydration strategies (e.g. asserting data-hydrated flips at the right time for idle vs visible). This matches the existing hydration_scheduling demo, which also ships without a spec, so it's not a regression — but since the PR description's test plan is purely manual/checkbox-based, consider whether a lightweight Capybara/Playwright spec (mirroring how __STREAMING_HYDRATION_EVENTS__ is designed for testing) would be worth adding, since the component was clearly built with testability in mind (data-testid, event log) but nothing currently reads it.

No issues found with

  • Security: no XSS/injection surface — no user input rendered, only static config-derived strings.
  • Correctness: the hydrate_on values used are valid and match the client-side HydrateOnMode type; component registration is duplicated correctly for both client and server bundles.
  • Performance: dummy-app-only demo, no impact on production code paths.

Overall: safe, well-scoped demo addition; suggestions above are polish, not blockers.

- Add rel="noopener noreferrer" to external link for security
- Use bracket notation for window global to avoid eslint-disable,
  matching the pattern in HydrationSchedulingProbe.tsx

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FYSpsrA5yeBBomT5TnBB53

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
react_on_rails/spec/dummy/client/app/startup/StreamingHydrationDemo.tsx (1)

73-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Tighten sectionConfig type for compile-time key safety.

Record<string, ...> allows any string key, so TypeScript won't flag a missing or misspelled section key. Using the section union type as the key constraint ensures all five sections are defined and makes the if (!config) guard genuinely defensive rather than compensating for a loose type.

♻️ Proposed type-safe refactor
-const sectionConfig: Record<string, Omit<SectionProps, 'testId'>> = {
+const sectionConfig: Record<StreamingHydrationDemoProps['section'], Omit<SectionProps, 'testId'>> = {
   header: {
     name: 'Header Section',
     description: 'Navigation and branding. First to hydrate.',
     color: '`#e91e63`',
   },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@react_on_rails/spec/dummy/client/app/startup/StreamingHydrationDemo.tsx`
around lines 73 - 99, Update the sectionConfig declaration to use the existing
section union type as its key constraint instead of Record<string, ...>. Keep
the current SectionProps value shape and all five section entries unchanged, so
TypeScript enforces complete, correctly spelled section keys and the lookup
guard remains defensive.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@react_on_rails/spec/dummy/app/views/pages/streaming_hydration_demo.html.erb`:
- Around line 63-66: Add rel="noopener noreferrer" to the target="_blank" anchor
in the streaming hydration demo, preserving its existing href and link text.

In `@react_on_rails/spec/dummy/config/routes.rb`:
- Line 53: Add the missing PagesController#streaming_hydration_demo action in
PagesController so the existing streaming_hydration_demo route dispatches
successfully and renders its matching view.

---

Nitpick comments:
In `@react_on_rails/spec/dummy/client/app/startup/StreamingHydrationDemo.tsx`:
- Around line 73-99: Update the sectionConfig declaration to use the existing
section union type as its key constraint instead of Record<string, ...>. Keep
the current SectionProps value shape and all five section entries unchanged, so
TypeScript enforces complete, correctly spelled section keys and the lookup
guard remains defensive.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b4a38df-5730-4304-a362-f8864b6c64a9

📥 Commits

Reviewing files that changed from the base of the PR and between 4747499 and a33848f.

📒 Files selected for processing (4)
  • react_on_rails/spec/dummy/app/views/pages/streaming_hydration_demo.html.erb
  • react_on_rails/spec/dummy/client/app/packs/client-bundle.ts
  • react_on_rails/spec/dummy/client/app/startup/StreamingHydrationDemo.tsx
  • react_on_rails/spec/dummy/config/routes.rb

get "manual_render_test" => "pages#manual_render_test"
get "root_error_callbacks" => "pages#root_error_callbacks"
get "hydration_scheduling" => "pages#hydration_scheduling"
get "streaming_hydration_demo" => "pages#streaming_hydration_demo"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify PagesController has the streaming_hydration_demo action
rg -n 'def streaming_hydration_demo' --type rb react_on_rails/spec/dummy/app/controllers/

# Also check for any catch-all or method_missing patterns
rg -n 'method_missing\|action_missing' --type rb react_on_rails/spec/dummy/app/controllers/

Repository: shakacode/react_on_rails

Length of output: 216


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the dummy app controllers and inspect the relevant controller.
git ls-files 'react_on_rails/spec/dummy/app/controllers/**' | sed -n '1,200p'

# Find the controller/action declaration.
rg -n 'streaming_hydration_demo|class PagesController|method_missing|action_missing' react_on_rails/spec/dummy/app/controllers

# Read the controller file if present.
controller_file=$(git ls-files 'react_on_rails/spec/dummy/app/controllers/**/pages_controller.rb' | head -n 1 || true)
if [ -n "${controller_file}" ]; then
  echo "=== ${controller_file} ==="
  cat -n "${controller_file}"
fi

# Inspect the route context too.
routes_file=react_on_rails/spec/dummy/config/routes.rb
echo "=== ${routes_file} ==="
cat -n "${routes_file}" | sed -n '40,70p'

Repository: shakacode/react_on_rails

Length of output: 2292


🏁 Script executed:

#!/bin/bash
set -euo pipefail

controller_file=react_on_rails/spec/dummy/app/controllers/pages_controller.rb
echo "=== ${controller_file} ==="
cat -n "${controller_file}"

Repository: shakacode/react_on_rails

Length of output: 1944


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for a matching view or any dynamic definition for this action.
git ls-files 'react_on_rails/spec/dummy/app/views/pages/**' | rg 'streaming_hydration_demo'
rg -n 'streaming_hydration_demo' react_on_rails/spec/dummy/app

Repository: shakacode/react_on_rails

Length of output: 238


Add the missing PagesController#streaming_hydration_demo action

The route points to pages#streaming_hydration_demo, but react_on_rails/spec/dummy/app/controllers/pages_controller.rb doesn’t define that action. The matching view exists, but Rails still needs the controller method to dispatch the request.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@react_on_rails/spec/dummy/config/routes.rb` at line 53, Add the missing
PagesController#streaming_hydration_demo action in PagesController so the
existing streaming_hydration_demo route dispatches successfully and renders its
matching view.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review: Streaming hydration demo page

Overview
Adds a dummy-app-only demo page (/streaming_hydration_demo) with a StreamingHydrationDemo React component rendered 5x with different hydrate_on strategies (:immediate, :idle, :visible), tracking hydration via data-hydrated plus a window.__STREAMING_HYDRATION_EVENTS__ array. Scope is small and self-contained (view, component, pack registration, route).

Correctness

  • Verified this does not need manual registration in server-bundle.ts for prerender: true to work: auto_load_bundle = true plus components_subdirectory = "startup" (spec/dummy/config/initializers/react_on_rails.rb) already auto-generates client and server registration for any component dropped in startup/, confirmed by sibling demo components (PureComponent, ImageExample, CssModulesImagesFontsExample) that use prerender: true without any manual ReactOnRails.register call. So there is no SSR bug here, but see the inline note on client-bundle.ts — the manual registration this PR adds is redundant (harmless, but unnecessary).
  • hydrate_on: :immediate | :idle | :visible values match the supported OSS modes in ReactOnRails::ReactComponent::HYDRATE_ON_MODES (lib/react_on_rails/react_component/render_options.rb).
  • Route wiring (config/routes.rb) and the implicit-view convention (no explicit controller action) match the existing hydration_scheduling route.
  • The defensive if (!config) return <div>Unknown section...</div> in the component is reasonable since props arrive as untyped JSON from the Rails view, bypassing compile-time TS checks.

Significant overlap with existing demo
This page is functionally very similar to the pre-existing hydration_scheduling page and HydrationSchedulingProbe component (spec/dummy/app/views/pages/hydration_scheduling.html.erb, spec/dummy/client/app/startup/HydrationSchedulingProbe.tsx), which already exercises hydrate_on: :immediate/:idle/:visible with the same data-hydrated plus global-event-array pattern, and already has full Playwright e2e coverage (spec/dummy/e2e/playwright/e2e/react_on_rails/hydration_scheduling.spec.js).

The PR description itself notes real streaming SSR requires Pro's Node Renderer, which isn't exercised here, so this page mainly re-demonstrates the existing hydrate_on scheduling feature with nicer visuals/copy rather than adding new technical coverage for issue #4385. Worth considering whether this should extend/reuse the existing hydration_scheduling page (or its Playwright spec) instead of adding a parallel, untested demo.

Test coverage

  • No Playwright/RSpec test was added for the new route/component, unlike the sibling hydration_scheduling page.
  • The PR description's own test plan checklist is entirely unchecked, suggesting manual verification (visiting the page, confirming hydration timing per section) hasn't been done yet.

Minor nits (see inline comments)

  • target="_blank" link missing rel="noopener noreferrer" (only such link in the dummy app views).
  • Style inconsistency: uses eslint-disable no-underscore-dangle where the closest sibling component (HydrationSchedulingProbe.tsx) avoids the lint issue via bracket notation.
  • Manual client-bundle.ts registration appears redundant given auto_load_bundle.

None of the above are blocking. The main things I'd want addressed before merge are (1) confirming manual QA per the test plan and (2) considering whether a Playwright spec (or reuse of the existing one) should back this new page, since it's shipping as an OSS-visible demo tied to a public issue (#4385).

@AbanoubGhadban

Copy link
Copy Markdown
Collaborator Author

Regarding the CodeRabbit comment about missing controller action:

The PagesController doesn't define explicit action methods for any routes - it uses Rails' implicit view rendering. For example, /hydration_scheduling has no def hydration_scheduling method, yet works because Rails automatically renders the view template.

The controller only has:

  • initialize_shared_store (private)
  • data (private, sets instance variables)
  • before_action filters

This is the established pattern in the dummy app, so /streaming_hydration_demo follows the same convention.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overview

This PR adds a dummy-app-only demo page at /streaming_hydration_demo (view + route + StreamingHydrationDemo React component registered on both client and server bundles) illustrating independent section hydration via the OSS hydrate_on: :immediate/:idle/:visible options. Scope is limited to spec/dummy, so blast radius is minimal — no gem/library code is touched.

Code Quality

  • hydrate_on values used (:immediate, :idle, :visible) match ReactOnRails::ReactComponent::HYDRATE_ON_MODES, so the demo is valid against the current implementation.
  • Component closely mirrors the existing HydrationSchedulingProbe.tsx pattern (global event-recording via window.__..._EVENTS__, data-hydrated attribute, data-testids), which is good for consistency with prior art in this repo.
  • sectionConfig is typed as Record<string, ...>, which weakens type safety against the closed section union already declared in StreamingHydrationDemoProps. Typing it as Record<StreamingHydrationDemoProps['section'], ...> would let TS guarantee all five keys exist and make the if (!config) runtime fallback provably dead code — left an inline comment on this.
  • window[key] ||= [] mutating pattern matches the existing probe component, so this is consistent even though it's a bit unconventional for typed global state.

Test Coverage

  • No automated test (RSpec/system/Playwright) accompanies this PR, even though the component explicitly records events to window.__STREAMING_HYDRATION_EVENTS__ "for testing." The PR's own test plan is a manual, unchecked checklist. Given HydrationSchedulingProbe also currently has no consuming spec in the repo, this may be intentional/deferred, but it's worth confirming whether a follow-up test PR is planned, since an untested demo page can silently bit-rot.

Minor / Consistency

  • streaming_hydration_demo route is unconditional, unlike the neighboring view_transitions_demo route which is env-gated and has a comment linking back to its issue/docs. If this demo is meant to be temporary/experimental like that one, consider matching the pattern (or at least adding a short comment linking to Investigate selective/independent hydration for traditional (non-RSC) React components #4385) — left an inline note.
  • The inline style={{...}} objects in Section are fine for a throwaway demo but recreate objects on every render; not a real concern given this is a dummy-app demo component, not production code.

Security

No security concerns — dummy-app-only demo content, no user input reflected unsafely, rel="noopener noreferrer" correctly used on the external issue link.

Risk

Low risk overall: additive-only change confined to the dummy app, no production/library code paths affected.

}}
>
<h2 style={{ color }}>{name}</h2>
<p>{description}</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Record<string, ...> loosens the key type so sectionConfig[section] is typed as possibly undefined, which is why the !config guard below is needed. Since StreamingHydrationDemoProps.section is already a closed union ('header' | 'hero' | 'main' | 'sidebar' | 'footer'), typing this as Record<StreamingHydrationDemoProps['section'], ...> would let TypeScript guarantee exhaustiveness and catch typos/missing entries at compile time, making the runtime Unknown section fallback (which can never actually be hit given the prop type) unnecessary.

get "manual_render_test" => "pages#manual_render_test"
get "root_error_callbacks" => "pages#root_error_callbacks"
get "hydration_scheduling" => "pages#hydration_scheduling"
get "streaming_hydration_demo" => "pages#streaming_hydration_demo"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: other experimental/demo-only routes added for a specific issue (e.g. view_transitions_demo right below) are gated behind an env var and have a comment linking back to the issue/docs. This route is unconditional and has no comment. Not a blocker for a dummy-app demo, but for consistency consider either a short comment referencing #4385, or gating it the same way if it's meant to be temporary.

- Use Record<StreamingHydrationDemoProps['section'], ...> for type safety
- Remove unnecessary runtime check since TypeScript guarantees key exists
- Add descriptive comment to route referencing issue #4385

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FYSpsrA5yeBBomT5TnBB53
props: { section: "footer" },
prerender: true,
id: "streaming-footer",
hydrate_on: :visible) %>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The footer is the only section demonstrating hydrate_on: :visible, but nothing pushes it below the fold — all five sections are short cards stacked directly on top of each other with no spacer. On a typical viewport (including Playwright's default 1280×720), the footer is likely to already be within the initial viewport when the page loads, so the IntersectionObserver will report it visible immediately and it will hydrate right away — contradicting the page's own copy ("Footer: hydrate_on: :visible — hydrate only when scrolled into view") and the PR's test plan item "Verify footer hydrates only when scrolled into view."

The sibling hydration_scheduling.html.erb demo avoids exactly this problem by inserting <div style="height: 220vh" aria-hidden="true"></div> before its visible-mode island. Consider doing the same here (or adding enough vertical spacing) so the footer is reliably off-screen on load.

}
}

const streamingHydrationEventsKey = '__STREAMING_HYDRATION_EVENTS__';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component builds the exact same data-testid + window.__..._EVENTS__ instrumentation pattern as HydrationSchedulingProbe (which has a full Playwright suite at e2e/playwright/e2e/react_on_rails/hydration_scheduling.spec.js), but no corresponding e2e spec is added for /streaming_hydration_demo in this PR. The test plan in the PR description is a manual, unchecked checklist rather than an automated test. Given the established convention for these OSS demo pages, it'd be worth adding a Playwright spec (or noting that this is intentionally deferred) so regressions in the hydrate_on scheduling paths this page exercises get caught automatically.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Summary

This PR adds a /streaming_hydration_demo dummy-app page that showcases the OSS hydrate_on scheduling feature (:immediate, :idle, :visible) across five independently-hydrating sections, as an approximation of the streaming SSR + selective hydration concept from #4385. It's scoped entirely to spec/dummy (demo/docs code), so production risk is low.

What's good

  • Follows the existing HydrationSchedulingProbe conventions closely (component registered in both client and server bundles, data-testid attributes, a window.__..._EVENTS__ array for test instrumentation, Omit<>-based typed section config).
  • Clear, well-written demo copy explaining the three hydrate_on strategies and their tradeoffs, plus an honest caveat that full streaming SSR needs the Pro Node Renderer.
  • No security concerns — no user input, all props are hardcoded, external link has rel="noopener noreferrer".

Issues found (see inline comments)

  1. Footer's hydrate_on: :visible likely won't demonstrate deferred hydration — unlike the sibling hydration_scheduling.html.erb demo, which explicitly adds a height: 220vh spacer before its visible-mode island, this page stacks all five short sections with no spacer. The footer may already be in the initial viewport on load, causing it to hydrate immediately and undermining the page's own claim that it "hydrates only when scrolled into view."
  2. No automated test coverage — the page builds the same testid/event-recording instrumentation used by hydration_scheduling.spec.js, but ships without an equivalent Playwright spec. The PR's test plan is a manual, unchecked checklist.

Minor/non-blocking

  • In dev, this bundle wraps registered components in StrictMode, so Section's useEffect will double-invoke and push two hydrated events per section into window.__STREAMING_HYDRATION_EVENTS__. This mirrors existing behavior in HydrationSchedulingProbe and isn't unique to this PR, but is worth keeping in mind if/when tests are added (assert with toContainEqual/.poll, not exact array length).

The footer section uses hydrate_on: :visible, which should only trigger
when scrolled into view. Without a spacer, the footer was visible on page
load on typical viewports, causing immediate hydration that contradicts
the demo's intent.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FYSpsrA5yeBBomT5TnBB53
@AbanoubGhadban

Copy link
Copy Markdown
Collaborator Author

Regarding the e2e spec comment:

The Playwright e2e spec is intentionally deferred for this PR. This demo page is primarily educational — it showcases the hydrate_on scheduling feature that already has comprehensive test coverage in hydration_scheduling.spec.js.

The demo page follows the same patterns and uses the same StreamingHydrationDemo component that tracks events to window.__STREAMING_HYDRATION_EVENTS__, so future e2e tests could be added if needed. For now, manual verification is sufficient for this demo-only page.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review: Streaming hydration demo page

Overview: Adds a dummy-app-only demo page (/streaming_hydration_demo) showing independent section hydration using the existing OSS hydrate_on scheduling feature (:immediate / :idle / :visible). New files: an ERB view, a StreamingHydrationDemo React component + Section sub-component, bundle registrations, and a route. This is illustrative/example code only — no production (gem or react-on-rails package) code is touched, so the blast radius is minimal.

Code quality

  • Structure and style closely mirror the existing HydrationSchedulingProbe demo (event-recording to a window global, data-hydrated/data-testid attributes, hydrate_on per section), so it's consistent with established conventions in this dummy app.
  • rel="noopener noreferrer" is correctly used on the external target="_blank" link.
  • The footer's <div style="height: 150vh"> spacer to force hydrate_on: :visible to fire only after scroll matches the pattern already used in hydration_scheduling.html.erb.

Minor suggestions

  • Section is wrapped in memo() (StreamingHydrationDemo.tsx:28), but each StreamingHydrationDemo instance mounts its own independent Section exactly once per react_component call — there's no re-render from a changing parent to guard against, so the memoization doesn't do anything useful here and can just be a plain function component.
  • The component records events to window.__STREAMING_HYDRATION_EVENTS__ and exposes data-testid/data-hydrated attributes "for testing," but no automated test (RSpec feature spec or Playwright e2e) actually exercises this page in the diff. That said, the sibling hydration_scheduling demo has the same gap, so this isn't a regression introduced by this PR — just flagging it since the PR description's test plan is entirely manual/unchecked. Worth confirming the manual verification steps were actually run before merging.

Risks

  • No security concerns — static demo content, no user input, XSS-safe interpolation via JSX/ERB defaults.
  • No performance concerns — dummy app only, not part of any published package.
  • Overall risk is low given the change is scoped to spec/dummy.

color: string;
};

const Section = memo(function Section({ name, testId, description, color }: SectionProps) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: memo() here doesn't buy anything — each Section is a distinct instance mounted once by its own react_component call, so there's no re-render from a changing parent props to skip. A plain function component would be equivalent and simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant