Dummy: add streaming hydration demo page#4593
Conversation
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
WalkthroughAdds 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. ChangesStreaming hydration demo
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
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds a dummy app page for streaming-style hydration demos. The main changes are:
Confidence Score: 4/5The new demo route can fail while server-rendering the first React section.
react_on_rails/spec/dummy/app/views/pages/streaming_hydration_demo.html.erb and the dummy server bundle registration. Important Files Changed
Reviews (1): Last reviewed commit: "Dummy: add streaming hydration demo page" | Re-trigger Greptile |
| different <code>hydrate_on</code> strategies. | ||
| </p> | ||
|
|
||
| <hr> |
There was a problem hiding this comment.
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) %> |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
💡 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, |
There was a problem hiding this comment.
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
|
Regarding the P2 comment about deferred hydration in Pro runs: The new Looking at the code in
The OSS dummy app (where this demo lives) doesn't have the Pro gem installed, so these modes work correctly. The existing 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. |
| <p> | ||
| <a href="https://github.com/shakacode/react_on_rails/issues/4385" target="_blank"> |
There was a problem hiding this comment.
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:
| <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"> |
| /* 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 */ |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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.
Review: Streaming Hydration Demo PageClean, small, self-contained addition to the dummy app. It correctly consumes the existing OSS Overview
Suggestions (non-blocking)
No issues found with
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
There was a problem hiding this comment.
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 winTighten
sectionConfigtype for compile-time key safety.
Record<string, ...>allows any string key, so TypeScript won't flag a missing or misspelled section key. Using thesectionunion type as the key constraint ensures all five sections are defined and makes theif (!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
📒 Files selected for processing (4)
react_on_rails/spec/dummy/app/views/pages/streaming_hydration_demo.html.erbreact_on_rails/spec/dummy/client/app/packs/client-bundle.tsreact_on_rails/spec/dummy/client/app/startup/StreamingHydrationDemo.tsxreact_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" |
There was a problem hiding this comment.
🎯 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/appRepository: 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.
|
Review: Streaming hydration demo page Overview Correctness
Significant overlap with existing demo 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 Test coverage
Minor nits (see inline comments)
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). |
|
Regarding the CodeRabbit comment about missing controller action: The The controller only has:
This is the established pattern in the dummy app, so |
Code ReviewOverviewThis PR adds a dummy-app-only demo page at Code Quality
Test Coverage
Minor / Consistency
SecurityNo security concerns — dummy-app-only demo content, no user input reflected unsafely, RiskLow risk overall: additive-only change confined to the dummy app, no production/library code paths affected. |
| }} | ||
| > | ||
| <h2 style={{ color }}>{name}</h2> | ||
| <p>{description}</p> |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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) %> |
There was a problem hiding this comment.
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__'; |
There was a problem hiding this comment.
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.
|
Review Summary This PR adds a What's good
Issues found (see inline comments)
Minor/non-blocking
|
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
|
Regarding the e2e spec comment: The Playwright e2e spec is intentionally deferred for this PR. This demo page is primarily educational — it showcases the The demo page follows the same patterns and uses the same |
Review: Streaming hydration demo pageOverview: Adds a dummy-app-only demo page ( Code quality
Minor suggestions
Risks
|
| color: string; | ||
| }; | ||
|
|
||
| const Section = memo(function Section({ name, testId, description, color }: SectionProps) { |
There was a problem hiding this comment.
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.
Summary
/streaming_hydration_demoshowcasing independent section hydrationhydrate_onstrategiesThe demo illustrates:
hydrate_on: :immediate— hydrate as soon as the bundle loadshydrate_on: :idle— hydrate when the browser is idlehydrate_on: :visible— hydrate only when scrolled into viewEach section tracks its hydration state via
data-hydratedattribute and records events towindow.__STREAMING_HYDRATION_EVENTS__for testing.Note: Full streaming SSR requires React on Rails Pro's Node Renderer. This demo uses the OSS
hydrate_onscheduling feature to approximate the user experience benefits.Test plan
/streaming_hydration_demoin the dummy apppnpm run lint— no errorspnpm run type-check— no errorsPart of #4385 (demo page only — the selective-hydration investigation stays open)
🤖 Generated with Claude Code
https://claude.ai/code/session_01FYSpsrA5yeBBomT5TnBB53
Summary by CodeRabbit