Walkthrough

RSC payloads are rendered as NDJSON text (formats: [:text]) and served with Content-Type application/x-ndjson. The streaming helper gained an optional content_type: parameter. Request specs verify NDJSON output and absence of Rails view annotation comments.

Changes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant RailsCtrl as RscPayloadController
  participant StreamHelper as stream_view_containing_react_components
  participant ViewTemplate as rsc_payload.text.erb
  participant ClientParser as NDJSONParser

  Client->>RailsCtrl: GET /rsc_payload/:component?props=...
  RailsCtrl->>StreamHelper: stream_view_containing_react_components(template: ..., content_type: "application/x-ndjson")
  StreamHelper->>ViewTemplate: render template formats: [:text], layout: false
  ViewTemplate-->>StreamHelper: NDJSON chunks (no HTML comments)
  StreamHelper-->>RailsCtrl: write chunks (response.content_type set)
  RailsCtrl-->>Client: 200 application/x-ndjson (streamed NDJSON)
  Client->>ClientParser: parse NDJSON lines (valid JSON)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through code with nimble paws,
I chase the stray flaws,
I switched the stream to plain-text cheer,
NDJSON sings and bytes are clear,
Hooray — no crumbs, just tidy paws!

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 @coderabbitai help to get the list of available commands and usage tips.}

Greptile Summary

This PR fixes RSC payload JSON corruption caused by Rails' annotate_rendered_view_with_filenames feature injecting HTML comment annotations into NDJSON output. The fix renames rsc_payload.html.erb → rsc_payload.text.erb and passes formats: [:text] to the render call so Rails skips annotation injection, which only applies to HTML-format views. A regression request spec is added to verify the fix holds.

Key changes:

• rsc_payload.html.erb renamed to rsc_payload.text.erb — no content change, format suffix change only
• formats: [:text] added to stream_view_containing_react_components call — response Content-Type changes from text/html to text/plain; charset=utf-8, which is a side-effect worth documenting since any client or middleware asserting Content-Type: text/html would be affected
• custom_rsc_payload_template is a public extension point; callers who override it with a .html.erb path will get ActionView::MissingTemplate after this change — migration documentation is needed
• New regression spec correctly uses an around block with ensure to toggle the annotation setting and validates all non-empty NDJSON lines are parseable JSON

Confidence Score: 3/5

• Safe to merge after confirming no downstream apps override custom_rsc_payload_template with a .html.erb file, and after adding a CHANGELOG entry about the text/plain content-type change.
• The core fix (format: text to suppress HTML annotations) is correct and well-targeted. However, formats: [:text] is a breaking change for any user who overrides custom_rsc_payload_template to return a .html.erb path, and the response Content-Type change from text/html to text/plain is a silent behavioral change for clients. These are real compatibility concerns that lower confidence without explicit migration guidance.
• react_on_rails_pro/lib/react_on_rails_pro/concerns/rsc_payload_renderer.rb — verify custom_rsc_payload_template override impact across consuming applications.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Client as RSC Client
    participant Rails as Rails Router
    participant Controller as PagesController<br/>(RSCPayloadRenderer)
    participant Renderer as stream_view_containing<br/>_react_components
    participant Template as rsc_payload.text.erb

    Client->>Rails: GET /rsc_payload/:component_name
    Rails->>Controller: rsc_payload action
    Controller->>Renderer: stream_view_containing_react_components(<br/>template: "react_on_rails_pro/rsc_payload",<br/>layout: false,<br/>formats: [:text])
    Note over Renderer: formats: [:text] prevents<br/>annotate_rendered_view_with_filenames<br/>from injecting HTML comments
    Renderer->>Template: render rsc_payload.text.erb<br/>(text/plain format)
    Template-->>Renderer: NDJSON chunks (unpolluted)
    Renderer-->>Client: Streaming NDJSON<br/>Content-Type: text/plain

_{Last reviewed commit: 4080300}

Review: Fix RSC payload JSON corruption from Rails view annotations

The fix is correct and targets the root cause well. Using formats: [:text] combined with the template rename cleanly prevents Rails from applying HTML view annotations to the NDJSON stream.

Content-Type side effect worth documenting

The most notable side effect of formats: [:text] is that it silently changes the response Content-Type from text/html to text/plain. Neither is semantically correct for NDJSON (the proper type would be application/x-ndjson or application/octet-stream), but text/plain is a lateral move rather than a regression. Worth verifying no client-side fetch code or proxy configuration relies on the old text/html value.

Explicitly setting response.headers[Content-Type] = application/x-ndjson in the concern or controller would make the intended type unambiguous and prevent future confusion.

Minor issues in the spec

• Bare describe instead of RSpec.describe at the top level -- worth checking for consistency with the rest of the dummy app request specs.
• The test does not assert the Content-Type response header. Since the format switch is the core fix mechanism, asserting the content type would make the spec more defensive against regressions.
• JSON.parse(stripped_line) inside filter_map is not rescued. If a non-JSON line slips through, the test failure will surface as a JSON::ParserError rather than a clear RSpec assertion failure pointing at the corrupt line.

Overall

Small, focused, and low-risk. The regression spec is a welcome addition. The content-type concern is the one item worth addressing before merging.

Addressed all review threads:

• set explicit application/x-ndjson content type for the RSC payload endpoint
• improved request spec JSON parse failure output with the offending line
• updated spec to use RSpec.describe
• added migration guidance for custom custom_rsc_payload_template overrides in CHANGELOG.md

Verification rerun:

• bundle exec rubocop react_on_rails_pro/lib/react_on_rails_pro/concerns/rsc_payload_renderer.rb react_on_rails_pro/spec/dummy/spec/requests/rsc_payload_spec.rb
• cd react_on_rails_pro/spec/dummy && bundle exec rspec spec/requests/rsc_payload_spec.rb (with local node renderer running)

Review summary posted as inline comments. Four items flagged: (1) Content-Type header set before render_to_string may be overridden by Rails format negotiation - pass content_type as a render option instead. (2) custom_rsc_payload_template override point needs a comment warning that custom templates must use .text.erb not .html.erb. (3) The spec does not assert HTML annotation comments are absent from the body, only that JSON lines parse. (4) ActionView::Base.annotate_rendered_view_with_filenames is a global class-level flag; thread-safe under sequential runs but fragile if parallel execution is ever enabled.

Review Summary

The fix is correct and well-targeted — forcing formats: [:text] prevents Rails from treating the NDJSON payload as HTML and injecting annotation comments. The template rename and the Content-Type header addition are logical companions. The around-block teardown pattern in the spec is properly structured.

Two things worth addressing before merge:

1. Content-Type ordering / API choice (inline comment on line 15 of rsc_payload_renderer.rb)

response.headers["Content-Type"] is set before render_to_string is called. While render_to_string doesn't normally mutate the live response's headers, passing formats: [:text] resolves to text/plain and some Rails/middleware code paths do reset the content type as a side effect of format negotiation. The test passes today, but the ordering is not an explicit Rails contract and could silently regress on a Rails upgrade. Prefer response.content_type = "application/x-ndjson" (idiomatic Rails) and, if you want belt-and-suspenders safety, move the assignment to just before the first response.stream.write.

2. Missing baseline test (inline comment on spec line 16)

The new spec only exercises the annotated-view scenario. Adding a second example without the annotation override would confirm that the formats: [:text] change doesn't regress template lookup or content-type negotiation in the normal (non-development) case.

3. Breaking change documentation (minor)

The CHANGELOG correctly calls out that users overriding custom_rsc_payload_template must rename their template from .html.erb to .text.erb. Since custom_rsc_payload_template is a private method that is nonetheless meant to be overridden, a runtime warning (or at least a note in an upgrade guide) would reduce support burden for Pro users who miss the CHANGELOG entry.

Overall the implementation is sound — these are improvements rather than blockers.

Checked against current origin/master: this PR is merge-ready apart from a CHANGELOG.md conflict. Review is approved, CI is passing, and the branch merges cleanly outside the changelog. I am updating the branch with the changelog conflict resolution now.

Code Review

The fix is correct and well-targeted. Using formats: [:text] with the renamed .text.erb template prevents Rails from injecting HTML annotation comments into the NDJSON stream, and the regression spec directly covers the failure mode from #2520.

Ordering of Content-Type header assignment

response.headers["Content-Type"] = "application/x-ndjson" is set before stream_view_containing_react_components, which internally calls render_to_string(formats: [:text]). Depending on the Rails version, render_to_string can update response.content_type as a side-effect (to text/plain), which would then override the explicitly-set value when response.stream.write commits the headers. The test validates the end result so it appears fine in practice, but placing the header assignment after render_to_string returns (or inside stream_view_containing_react_components just before the first stream.write) would make the ordering invariant rather than relying on render_to_string being side-effect-free.

Test spec — class-level mutation

The around block mutates ActionView::Base.annotate_rendered_view_with_filenames at the class level. The ensure cleanup is correct for serial runs, but if the suite is ever run with parallel workers (e.g. parallel_tests), the global toggle could bleed across processes/threads. A brief comment noting this assumption would help future readers.

Test assertion breadth

expect(parsed_chunks).to all(include("html")) asserts that every parsed chunk has an "html" key. RSC payloads can emit multiple chunk types (module references, error boundaries, etc.). If the format ever produces a chunk without "html", this test would fail for the wrong reason. Consider relaxing to check that at least one chunk includes "html", or asserting only on chunks where that key is expected.

CHANGELOG / migration note

The CHANGELOG entry for custom custom_rsc_payload_template overrides is clear and correctly placed. No concerns there.