Clarify streaming narrative in RSC docs (#2813)#2814
Conversation
…e data streaming
Rename misleading section titles ("Progressive Streaming Architecture",
"Streaming Slow Data") and update descriptions to accurately reflect that
with synchronous props, all data loads in Rails before rendering begins.
The streaming is HTML streaming (renderToPipeableStream), not progressive
data streaming where slow data arrives independently.
Also scope as_json(include:) calls with only: to avoid serializing all
columns of included associations, consistent with existing current_user
patterns.
Fixes #2813
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR updates migration docs to clarify that Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 clarifies the streaming narrative across four RSC migration docs, accurately distinguishing HTML streaming (what Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Rails
participant React as React renderToPipeableStream
Browser->>Rails: HTTP GET /products/123
note over Rails: Controller loads all data synchronously
Rails->>React: stream_react_component with all props
note over React: HTML streaming begins immediately
React-->>Browser: HTML chunk 1 header and nav
React-->>Browser: HTML chunk 2 main content
React-->>Browser: HTML chunk 3 recommendations and comments
Reviews (1): Last reviewed commit: "Clarify streaming narrative in RSC docs:..." | Re-trigger Greptile |
|
Review The terminology clarification here is accurate and a real improvement. The old 'progressive streaming' / 'streaming slow data' titles were genuinely misleading, and the new callout box distinguishing HTML streaming from progressive data streaming is the standout addition. Issue 1: Hardcoded example column names in as_json snippets The new only: scoping is the right pattern, but the specific column names (label, value, text, rating) are fictional placeholders. Without a prose note making that explicit, readers are likely to copy-paste them literally and get silent empty results or nil fields. A code comment or a sentence beneath each block noting 'replace with your actual column names' would prevent this. Issue 2: The streaming-ssr.md link goes to a Pro feature without flagging it The new callout in rsc-data-fetching.md ends with a link to streaming-ssr.md for progressive data streaming. Since that page covers a React on Rails Pro capability, non-Pro users following the link may be confused. Worth adding '(React on Rails Pro)' inline. Issue 3: Subsection title / parent section mismatch The renamed subsection 'Streaming HTML Delivery' now sits under the 'Loading States and Suspense Boundaries' parent section. The new description emphasises that all data is loaded synchronously before rendering begins, but 'Loading States' implies async states. The subsection content predates this PR so this is not a regression, but the rename makes the mismatch more visible — a bridging sentence or reorganisation may help. Minor nit In rsc-data-fetching.md, list item 5 says 'The component renders with zero JavaScript cost as a Server Component' which is only true when the tree contains no Client Components. Worth a qualifier, though this predates the PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c59a4adbc9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses review feedback: the callout linking to progressive data streaming docs now explicitly notes it covers a Pro feature, so non-Pro readers are not confused. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed the review feedback: Issue 2 (Pro link): Fixed in 637bf81 — added "(React on Rails Pro)" to the streaming-ssr.md link so non-Pro readers have clear context. Issue 1 (placeholder column names): Declining — the column names are intentionally generic example placeholders. Adding "replace with your actual columns" notes to every snippet would clutter the docs. Issue 3 (subsection/parent mismatch): Acknowledged but out of scope — the placement under "Loading States and Suspense Boundaries" predates this PR. The rename makes it slightly more visible, but restructuring parent sections would expand scope. Better addressed in a dedicated docs reorganization. Issue 4 (minor nit on "zero JavaScript cost"): Agreed this is pre-existing and not introduced by this PR, so leaving as-is. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/oss/migrating/rsc-data-fetching.md`:
- Around line 132-134: Combine the two adjacent blockquote lines so there's no
blank quoted line between them: remove the empty quoted line separating "**HTML
streaming vs. progressive data streaming:** ..." and "**More details:** ..." so
they form a contiguous blockquote, or alternatively split them into two separate
blockquote blocks without an empty quoted gap; ensure there are no blank lines
that start with ">" between the quoted lines to satisfy markdownlint MD028.
🪄 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: 496624a5-9a3e-4673-899c-d16110c52683
📒 Files selected for processing (1)
docs/oss/migrating/rsc-data-fetching.md
ReviewThe terminology clarification is accurate and a genuine improvement. The old "progressive streaming" / "streaming slow data" framing was misleading, and the new callout box distinguishing HTML streaming from progressive data streaming is the most valuable addition. The Inconsistent
|
…ta-fetching - Merge adjacent blockquotes with `>` continuation to fix markdownlint MD028 (no-blanks-blockquote) violation at the streaming callout box - Add `only:` scoping to unscoped `.as_json` calls in "How Streaming Works" and "Solution 3" code blocks for consistency with other examples in the PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed in 8e07d46 — the two unscoped |
|
Overall the changes are accurate and well-targeted. Distinguishing HTML streaming from progressive data streaming is a real source of confusion, and this PR addresses it clearly. The as_json(only: ...) scoping additions are a good security/performance practice. Three issues worth fixing before merge:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e07d469f3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Replying to review summary #2814 (review): The MD028 point was a duplicate of the inline thread and is now addressed in the docs update. No additional action needed beyond that inline fix. |
|
Replying to #2814 (comment): Thanks for the detailed review. The three blocking items called out there were addressed in this update:
|
|
Replying to #2814 (comment): Thanks for the thoughtful pass. Current status:
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
ReviewThe terminology clarification (HTML streaming vs. progressive data streaming) is accurate and the added callout pointing readers to Two issues worth fixing: 1. Merged blockquotes (rendering bug) 2. Dropped |
The two consecutive `>` lines (HTML streaming callout and More details callout) rendered as a single fused paragraph in CommonMark. Insert a blank `>` continuation line to create a visual paragraph break within the blockquote without triggering MD028. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Review: Clarify streaming narrative in RSC docs. The terminology clarifications are accurate and the HTML-streaming-vs-progressive-data-streaming callout is a clear improvement. Two issues are worth addressing before merge. (1) Inconsistent only: scoping - The PR correctly scopes several as_json calls but rsc-data-fetching.md lines 693-695 still have main_content.as_json, RecommendationService.for(@page).as_json, @page.comments.recent.as_json without only:. Same for rsc-data-fetching.md:645 and rsc-component-patterns.md:382. The doc itself warns against bare as_json at line 847. (2) includes(:author) dropped without explanation - product.reviews.includes(:author).as_json became product.reviews.as_json(only: [:id, :text, :rating]) which is correct, but rsc-component-patterns.md:382 still has post.comments.includes(:author).as_json without only:. One example drops eager loading silently while another keeps it without field-scoping. The updated examples also no longer illustrate N+1 prevention. (3) Minor: Streaming HTML Delivery section sits under Loading States and Suspense Boundaries heading which implies async rendering, though the content correctly explains synchronous props mean no Suspense boundaries are involved. |
| ### Streaming HTML Delivery | ||
|
|
||
| Structure your page so critical content renders alongside secondary content, with `stream_react_component` handling progressive HTML delivery: | ||
| With synchronous props, Rails loads all data before rendering begins. `stream_react_component` then streams the rendered HTML as React processes the component tree — the browser receives content as it's rendered rather than waiting for the entire page: |
There was a problem hiding this comment.
The updated description is accurate, but the code example that follows (lines 693–695) still uses unscoped as_json:
main_content: @page.main_content.as_json,
recommendations: RecommendationService.for(@page).as_json,
comments: @page.comments.recent.as_jsonThis contradicts both the only: scoping added elsewhere in this PR and the doc's own warning against bare as_json (~line 847). Consider adding only: with the fields the Page component actually renders to keep the examples consistent.
…olve-2661 * origin/main: Clarify streaming narrative in RSC docs (#2813) (#2814) Bump version to 16.5.1 Update CHANGELOG.md for 16.5.1 (#2873) fix: include lib/tasks/ in pro gem so rake tasks are available (#2872) docs: clarify how React on Rails compares to alternatives (#2856) docs: fix published setup guidance gaps (#2860) docs: refresh setup and runtime guidance (#2857) docs: refresh pro upgrade examples (#2859) docs: modernize dependency update commands (#2864) Update spec/dummy Gemfile.lock for async >= 2.29 (#2870) # Conflicts: # docs/oss/migrating/rsc-data-fetching.md
| reviews: product.reviews.includes(:author).as_json, | ||
| recommendations: RecommendationService.for(product).as_json }) %> | ||
| reviews: product.reviews | ||
| .as_json(only: [:id, :text, :rating]), |
There was a problem hiding this comment.
Good — only: scoping added here. Note that rsc-component-patterns.md:382 (Pattern 5) still uses post.comments.includes(:author).as_json without only:, which is now inconsistent with this pattern. That example also no longer demonstrates N+1 prevention since the eager-loaded :author association is never field-scoped. Worth addressing here or in a follow-up for consistency.
## Summary
- Rename misleading section titles ("Progressive Streaming Architecture"
→ "Streaming HTML Delivery", "Streaming Slow Data" → "Streaming HTML
Delivery") to accurately reflect what the code demonstrates
- Update descriptions across 4 docs files to clarify that with
synchronous props, all data loads in Rails before rendering begins — the
streaming is HTML streaming (`renderToPipeableStream`), not progressive
data streaming where slow data arrives independently
- Add a callout distinguishing HTML streaming from progressive data
streaming, with a link to `streaming-ssr.md` for the latter
- Scope `as_json(include:)` calls with `only:` to avoid serializing all
columns of included associations, consistent with existing
`current_user.as_json(only: [...])` patterns
Fixes #2813
## Test plan
- [ ] Verify all internal doc links resolve correctly
- [ ] Review that section titles match what the code examples actually
demonstrate
- [ ] Confirm the HTML streaming vs progressive data streaming
distinction is clear
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Documentation-only updates that reword streaming guidance and adjust
example `as_json` field selection; no runtime code paths are modified.
>
> **Overview**
> Clarifies the RSC migration docs’ streaming narrative to emphasize
that `stream_react_component` streams *rendered HTML* via React’s
`renderToPipeableStream` after Rails has synchronously loaded props (and
not progressive *data* streaming). This renames related section headings
(e.g., to **Streaming HTML Delivery**) and adds an explicit callout
contrasting HTML streaming vs progressive data streaming with a pointer
to the Pro streaming SSR guide.
>
> Updates ERB examples across the guides to scope `as_json`
serialization using `only:` for included associations (e.g., `specs`,
`reviews`, recommendations) to avoid implying full-column serialization
in recommended patterns.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
0ff60ae. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Documentation**
* Clarified that Rails loads all data synchronously and that HTML is
streamed to the browser as React renders component trees.
* Renamed/reframed sections to emphasize "Streaming HTML Delivery" and
explicit use of React's streaming rendering.
* Updated examples and migration guidance to use more selective prop
serialization and to contrast HTML streaming with progressive data
streaming.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ew-fixes * origin/main: Remove dependency on internal TanStack Router router.ssr flag (#2833) Revert "Eliminate double JSON.stringify in RSC payload embedding (#2835)" (#2878) Eliminate double JSON.stringify in RSC payload embedding (#2835) docs: align Pro references with canonical docs routes (#2866) docs: make Pro route entry points explicit (#2867) Bump fastify from 5.8.1 to 5.8.3 in the npm-security group across 1 directory (#2846) docs: add RoR-specific competitive landscape and template refs (#2869) Clarify streaming narrative in RSC docs (#2813) (#2814)
Summary
renderToPipeableStream), not progressive data streaming where slow data arrives independentlystreaming-ssr.mdfor the latteras_json(include:)calls withonly:to avoid serializing all columns of included associations, consistent with existingcurrent_user.as_json(only: [...])patternsFixes #2813
Test plan
🤖 Generated with Claude Code
Note
Low Risk
Documentation-only updates that reword streaming guidance and adjust example
as_jsonfield selection; no runtime code paths are modified.Overview
Clarifies the RSC migration docs’ streaming narrative to emphasize that
stream_react_componentstreams rendered HTML via React’srenderToPipeableStreamafter Rails has synchronously loaded props (and not progressive data streaming). This renames related section headings (e.g., to Streaming HTML Delivery) and adds an explicit callout contrasting HTML streaming vs progressive data streaming with a pointer to the Pro streaming SSR guide.Updates ERB examples across the guides to scope
as_jsonserialization usingonly:for included associations (e.g.,specs,reviews, recommendations) to avoid implying full-column serialization in recommended patterns.Written by Cursor Bugbot for commit 0ff60ae. This will update automatically on new commits. Configure here.
Summary by CodeRabbit