Skip to content

Revert "Eliminate double JSON.stringify in RSC payload embedding (#2835)" #2878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
justin808 merged 1 commit into from
Mar 28, 2026
Merged

Revert "Eliminate double JSON.stringify in RSC payload embedding (#2835)" #2878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,6 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ

### [Unreleased]

#### Performance

- **[Pro]** **Reduce RSC payload overhead by eliminating double JSON.stringify**: RSC Flight data embedded in
HTML stream script tags was being JSON.stringified twice — once when wrapping in the result object envelope,
and again when embedding in the `<script>` tag's `.push()` call. The second stringify is now eliminated by
embedding JSON directly as a JavaScript expression (JSON is a strict subset of JS). This saves ~38KB (~24%
of raw Flight data) on a typical product search page with 36 results.
[PR 2835](https://github.com/shakacode/react_on_rails/pull/2835) by
[justin808](https://github.com/justin808).
Fixes [Issue 2522](https://github.com/shakacode/react_on_rails/issues/2522).

### [16.5.1] - 2026-03-27

#### Fixed
Expand Down
66 changes: 22 additions & 44 deletions packages/react-on-rails-pro/src/getReactServerComponent.client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,13 @@

import * as React from 'react';
import { createFromReadableStream } from 'react-on-rails-rsc/client.browser';
import { RailsContext, RSCPayloadChunk } from 'react-on-rails/types';
import {
createRSCPayloadKey,
fetch,
wrapInNewPromise,
extractErrorMessage,
sanitizeNonce,
replayConsoleLog,
} from './utils.ts';
import { RailsContext } from 'react-on-rails/types';
import { createRSCPayloadKey, fetch, wrapInNewPromise, extractErrorMessage } from './utils.ts';
import transformRSCStreamAndReplayConsoleLogs from './transformRSCStreamAndReplayConsoleLogs.ts';

declare global {
interface Window {
REACT_ON_RAILS_RSC_PAYLOADS?: Record<string, RSCPayloadChunk[]>;
REACT_ON_RAILS_RSC_PAYLOADS?: Record<string, string[]>;
}
}

Expand Down Expand Up @@ -98,36 +91,20 @@ const fetchRSC = ({
}
};

/**
* Creates a ReadableStream of raw Flight data from preloaded RSC payload objects.
*
* The payloads are objects (not strings) because injectRSCPayload embeds JSON
* directly as JavaScript expressions, avoiding the double JSON.stringify overhead.
* This function extracts the html field and replays console logs from each chunk.
*/
const createRSCStreamFromPreloadedPayloads = (payloads: RSCPayloadChunk[], cspNonce?: string) => {
const encoder = new TextEncoder();
const sanitizedNonceValue = sanitizeNonce(cspNonce);
let streamController: ReadableStreamController<Uint8Array> | undefined;
let closed = false;
const stream = new ReadableStream<Uint8Array>({
const createRSCStreamFromArray = (payloads: string[]) => {
let streamController: ReadableStreamController<string> | undefined;
const stream = new ReadableStream<string>({
start(controller) {
// Browser-only by design (callers read from window.REACT_ON_RAILS_RSC_PAYLOADS).
// If called outside the browser, close immediately to avoid hanging streams.
if (typeof window === 'undefined') {
closed = true;
controller.close();
return;
}
Comment on lines 97 to 100
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P1 Stream controller never closed in non-browser environments

When typeof window === 'undefined', the start callback returns early without closing controller and without assigning streamController. The code executed after the new ReadableStream(...) constructor also silently no-ops because streamController is undefined. Any consumer reading from this stream will hang indefinitely. The version from PR #2835 explicitly called controller.close() in this branch. While this path is unlikely to be exercised in practice (callers guard on window.REACT_ON_RAILS_RSC_PAYLOADS), it is worth tracking for the follow-up PR.

const handleChunk = (chunk: RSCPayloadChunk) => {
if (closed) return;
controller.enqueue(encoder.encode(chunk.html ?? ''));
replayConsoleLog(chunk.consoleReplayScript, sanitizedNonceValue);
const handleChunk = (chunk: string) => {
controller.enqueue(chunk);
};

payloads.forEach(handleChunk);
// eslint-disable-next-line no-param-reassign
payloads.push = (...chunks: RSCPayloadChunk[]) => {
payloads.push = (...chunks) => {
chunks.forEach(handleChunk);
return chunks.length;
};
Expand All @@ -137,32 +114,33 @@ const createRSCStreamFromPreloadedPayloads = (payloads: RSCPayloadChunk[], cspNo

if (typeof document !== 'undefined' && document.readyState === 'loading') {
document.addEventListener('DOMContentLoaded', () => {
closed = true;
streamController?.close();
});
} else {
// Once parsing is past "loading", all inline <script> tags that push into this array
// have already executed, so the preloaded payload list is complete and can be closed now.
closed = true;
streamController?.close();
}

return stream;
};

/**
* Creates React elements from preloaded RSC payloads embedded in the page.
* Creates React elements from preloaded RSC payloads in the page.
*
* The payloads are RSCPayloadChunk objects pushed to the global array by
* injectRSCPayload's script tags. This processes them directly without
* JSON.parse overhead (the objects are already parsed by the JS engine).
* This function:
* 1. Creates a ReadableStream from the array of payload chunks
* 2. Transforms the stream to handle console logs and other processing
* 3. Uses React's createFromReadableStream to process the payload
*
* @param payloads - Array of RSC payload chunk objects from the global array
* This is used during hydration to avoid making HTTP requests when
* the payload is already embedded in the page.
*
* @param payloads - Array of RSC payload chunks from the global array
* @returns A Promise resolving to the rendered React element
*/
const createFromPreloadedPayloads = (payloads: RSCPayloadChunk[], cspNonce?: string) => {
const stream = createRSCStreamFromPreloadedPayloads(payloads, cspNonce);
const renderPromise = createFromReadableStream<React.ReactNode>(stream);
const createFromPreloadedPayloads = (payloads: string[], cspNonce?: string) => {
const stream = createRSCStreamFromArray(payloads);
const transformedStream = transformRSCStreamAndReplayConsoleLogs(stream, cspNonce);
const renderPromise = createFromReadableStream<React.ReactNode>(transformedStream);
return wrapInNewPromise(renderPromise);
};

Expand Down
45 changes: 6 additions & 39 deletions packages/react-on-rails-pro/src/injectRSCPayload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,8 @@ function createRSCPayloadInitializationScript(cacheKey: string, sanitizedNonce?:
return createScriptTag(cacheKeyJSArray(cacheKey), sanitizedNonce);
}

function createRSCPayloadChunk(jsonLine: string, cacheKey: string, sanitizedNonce?: string) {
// Embed the JSON line directly as a JavaScript expression instead of re-stringifying it.
// Valid JSON is a strict subset of JavaScript expressions, so parseable JSON is safe to embed.
// createScriptTag's escapeScript() handles HTML-unsafe sequences (</script>, <!--)
// using JS-compatible escape sequences that preserve the parsed value.
try {
JSON.parse(jsonLine);
} catch {
throw new Error(`Malformed NDJSON line in RSC stream: ${jsonLine.slice(0, 100)}`);
}
return createScriptTag(`(${cacheKeyJSArray(cacheKey)}).push(${jsonLine})`, sanitizedNonce);
function createRSCPayloadChunk(chunk: string, cacheKey: string, sanitizedNonce?: string) {
return createScriptTag(`(${cacheKeyJSArray(cacheKey)}).push(${JSON.stringify(chunk)})`, sanitizedNonce);
}

/**
Expand Down Expand Up @@ -97,6 +88,7 @@ export default function injectRSCPayload(
safePipe(pipeableHtmlStream, htmlStream, (err) => {
resultStream.emit('error', err);
});
const decoder = new TextDecoder();
let rscPromise: Promise<void> | null = null;

// ========================================
Expand Down Expand Up @@ -259,37 +251,12 @@ export default function injectRSCPayload(
const initializationScript = createRSCPayloadInitializationScript(rscPayloadKey, sanitizedNonce);
rscInitializationBuffers.push(Buffer.from(initializationScript));

// Process RSC payload stream asynchronously.
// Chunks may not align with JSON object boundaries, so we buffer
// incomplete lines (same approach as transformRSCStream).
// Process RSC payload stream asynchronously
rscPromises.push(
(async () => {
let lastIncompleteLine = '';
const decoder = new TextDecoder();
for await (const chunk of stream ?? []) {
const decodedChunk =
lastIncompleteLine +
(typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }));
const lines = decodedChunk.split('\n');
lastIncompleteLine = lines.pop() ?? '';

// Contract: upstream stream emits NDJSON (one payload object per line).
let bufferedPayloadInThisChunk = false;
for (const line of lines) {
const normalizedLine = line.trim();
if (normalizedLine !== '') {
const payloadScript = createRSCPayloadChunk(normalizedLine, rscPayloadKey, sanitizedNonce);
rscPayloadBuffers.push(Buffer.from(payloadScript));
bufferedPayloadInThisChunk = true;
}
}
if (bufferedPayloadInThisChunk) {
scheduleFlush();
}
}
const finalChunk = (lastIncompleteLine + decoder.decode()).trim();
if (finalChunk !== '') {
const payloadScript = createRSCPayloadChunk(finalChunk, rscPayloadKey, sanitizedNonce);
const decodedChunk = typeof chunk === 'string' ? chunk : decoder.decode(chunk);
Copy link
Copy Markdown
Contributor

@claude claude Bot Mar 28, 2026

Choose a reason for hiding this comment

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

Bug: TextDecoder reused without { stream: true } — will corrupt multibyte UTF-8 across chunk boundaries.

A single TextDecoder instance is created at line 91 and reused across every iteration of the for await loop. Without the { stream: true } option, the decoder flushes its internal buffer on every call, so a multibyte character (e.g. a 4-byte emoji or CJK codepoint) that is split across two consecutive binary chunks will be decoded as the replacement character U+FFFD in the first chunk and then corrupt the second.

Suggested change
const decodedChunk = typeof chunk === 'string' ? chunk : decoder.decode(chunk);
const decodedChunk = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true });

The corresponding test ("handles chunks that split a multibyte UTF-8 character") was removed by this revert — it should be kept (or re-added in the follow-up PR) to guard this path.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P1 Badge Decode RSC stream chunks with streaming TextDecoder

The RSC payload loop decodes each binary chunk with decoder.decode(chunk) (no { stream: true }), so a UTF-8 character split across chunk boundaries is replaced with U+FFFD before being embedded into <script> tags. This causes silent data corruption for non-ASCII/emoji content in Flight payloads, because the client only ever sees the already-corrupted string.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P1 TextDecoder without { stream: true } corrupts multi-byte UTF-8 across chunk boundaries

decoder.decode(chunk) (without { stream: true }) signals to the TextDecoder that this is the final call, flushing its internal state. Any multi-byte UTF-8 sequence (e.g., emoji, CJK characters) that straddles two consecutive stream chunks will produce a replacement character (U+FFFD) instead of the correct code point.

PR #2835 fixed this with decoder.decode(chunk, { stream: true }) and also made the decoder local to each stream's IIFE so that concurrent streams don't share state. Both regressions are reintroduced here. The test case "handles chunks that split a multibyte UTF-8 character" was removed as part of this revert — worth noting for the follow-up PR.

const payloadScript = createRSCPayloadChunk(decodedChunk, rscPayloadKey, sanitizedNonce);
rscPayloadBuffers.push(Buffer.from(payloadScript));
scheduleFlush();
}
Expand Down
54 changes: 28 additions & 26 deletions packages/react-on-rails-pro/src/transformRSCStreamAndReplayConsoleLogs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/

import { RSCPayloadChunk } from 'react-on-rails/types';
import { sanitizeNonce, replayConsoleLog } from './utils.ts';
import { sanitizeNonce } from './utils.ts';

/**
* Transforms an RSC stream and replays console logs on the client.
Expand Down Expand Up @@ -45,46 +45,48 @@ export default function transformRSCStreamAndReplayConsoleLogs(
let { value, done } = await reader.read();

const handleJsonChunk = (chunk: RSCPayloadChunk) => {
const { html, consoleReplayScript } = chunk;
const { html, consoleReplayScript = '' } = chunk;
controller.enqueue(encoder.encode(html ?? ''));
replayConsoleLog(consoleReplayScript, sanitizedNonce);
};

const parseAndHandleLines = (lines: string[]) => {
const jsonChunks = lines
.filter((line) => line.trim() !== '')
.map((line) => {
try {
return JSON.parse(line) as RSCPayloadChunk;
} catch (error) {
console.error('Error parsing JSON:', line, error);
throw error;
}
});

for (const jsonChunk of jsonChunks) {
handleJsonChunk(jsonChunk);
const replayConsoleCode = consoleReplayScript
.trim()
.replace(/^<script[^>]*>/i, '')
.replace(/<\/script>$/i, '');
if (replayConsoleCode?.trim() !== '') {
const scriptElement = document.createElement('script');
if (sanitizedNonce) {
scriptElement.nonce = sanitizedNonce;
}
scriptElement.textContent = replayConsoleCode;
document.body.appendChild(scriptElement);
}
Comment on lines +55 to 62
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P2 document accessed without an SSR guard

document.createElement('script') is called here without first checking typeof document !== 'undefined'. The extracted replayConsoleLog helper in PR #2835's version of utils.ts carried an explicit guard:

if (typeof document === 'undefined') {
  return;
}

Although this code path lives in a .client.ts file and is unlikely to run in SSR, the defensive check is a best practice and prevents accidental crashes if the function is ever invoked in a Node.js context during testing or SSR. Worth restoring in the follow-up PR.

};

try {
while (!done) {
const decodedValue = typeof value === 'string' ? value : decoder.decode(value, { stream: true });
const decodedValue = typeof value === 'string' ? value : decoder.decode(value);
Copy link
Copy Markdown
Contributor

@claude claude Bot Mar 28, 2026

Choose a reason for hiding this comment

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

Same TextDecoder streaming bug as in injectRSCPayload.ts.

decoder.decode(value) without { stream: true } will mishandle a multibyte character split between two stream chunks.

Suggested change
const decodedValue = typeof value === 'string' ? value : decoder.decode(value);
const decodedValue = typeof value === 'string' ? value : decoder.decode(value, { stream: true });

const decodedChunks = lastIncompleteChunk + decodedValue;
const chunks = decodedChunks.split('\n');
lastIncompleteChunk = chunks.pop() ?? '';

parseAndHandleLines(chunks);
const jsonChunks = chunks
.filter((line) => line.trim() !== '')
.map((line) => {
try {
return JSON.parse(line) as RSCPayloadChunk;
} catch (error) {
console.error('Error parsing JSON:', line, error);
throw error;
}
});

for (const jsonChunk of jsonChunks) {
handleJsonChunk(jsonChunk);
}

// eslint-disable-next-line no-await-in-loop
({ value, done } = await reader.read());
}

const finalDecodedValue = lastIncompleteChunk + decoder.decode();
if (finalDecodedValue.trim() !== '') {
parseAndHandleLines(finalDecodedValue.split('\n'));
}

controller.close();
Copy link
Copy Markdown
Contributor

@claude claude Bot Mar 28, 2026

Choose a reason for hiding this comment

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

Silent data loss: the final incomplete NDJSON line is never flushed.

When the upstream stream ends, lastIncompleteChunk may still contain a valid NDJSON line that never ended with \n. The loop exits and controller.close() is called immediately, dropping that data silently.

PR #2835 fixed this by flushing after the loop:

const finalDecodedValue = lastIncompleteChunk + decoder.decode(); // flush remaining bytes
if (finalDecodedValue.trim() !== '') {
  parseAndHandleLines(finalDecodedValue.split('\n'));
}

This is a pre-existing regression worth carrying forward as a fix in the follow-up PR.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P2 Badge Parse buffered tail line before closing RSC transform

The transformer keeps partial NDJSON in lastIncompleteChunk, but on EOF it closes immediately without parsing that buffered remainder. When the final payload line does not end with \n, the last RSC chunk is dropped, which can leave hydration with incomplete Flight data (missing final UI chunk or unresolved suspense).

Useful? React with 👍 / 👎.

Comment on lines 88 to 90
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Mar 28, 2026

Choose a reason for hiding this comment

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

P1 Last incomplete chunk silently dropped

After the while (!done) loop ends, lastIncompleteChunk holds any data that was buffered from a line not yet terminated with \n. The loop calls controller.close() immediately without flushing this remaining buffer. If the RSC stream's final NDJSON line does not end with a newline, the data is silently lost.

PR #2835 fixed this by processing lastIncompleteChunk after the loop:

const finalDecodedValue = lastIncompleteChunk + decoder.decode();
if (finalDecodedValue.trim() !== '') {
  parseAndHandleLines(finalDecodedValue.split('\n'));
}

This is a known regression being reintroduced and should be tracked for the planned follow-up PR.

} catch (error) {
console.error('Error transforming RSC stream:', error);
Expand Down
19 changes: 0 additions & 19 deletions packages/react-on-rails-pro/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,3 @@ export const sanitizeNonce = (nonce?: string) => {
const nonceWithAllowedCharsOnly = nonce?.replace(/[^a-zA-Z0-9+/=_-]/g, '');
return nonceWithAllowedCharsOnly?.match(/^[a-zA-Z0-9+/_-]+={0,2}$/)?.[0];
};

export function replayConsoleLog(consoleReplayScript: string | undefined, sanitizedNonce?: string) {
if (typeof document === 'undefined') {
return;
}

const replayConsoleCode = (consoleReplayScript ?? '')
.trim()
.replace(/^<script[^>]*>/i, '')
.replace(/<\/script>$/i, '');
if (replayConsoleCode?.trim() !== '') {
const scriptElement = document.createElement('script');
if (sanitizedNonce) {
scriptElement.nonce = sanitizedNonce;
}
scriptElement.textContent = replayConsoleCode;
document.body.appendChild(scriptElement);
}
}
6 changes: 3 additions & 3 deletions packages/react-on-rails-pro/tests/RSCRequestTracker.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,10 @@ describe('RSCRequestTracker', () => {
expect(result).toContain('<html><body>');
expect(result).toContain('</body></html>');

// Output must contain the RSC payload initialization and data scripts.
// The payload JSON is embedded directly as a JS expression (not re-stringified).
// Output must contain the RSC payload initialization and data scripts
expect(result).toContain('REACT_ON_RAILS_RSC_PAYLOADS');
expect(result).toContain(`.push(${payload})`);
expect(result).toContain('.push(');
expect(result).toContain(JSON.stringify(payload));
Comment on lines +227 to +228
Copy link
Copy Markdown
Contributor

@claude claude Bot Mar 28, 2026

Choose a reason for hiding this comment

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

Weakened assertion: the two toContain checks are independent and can match anywhere in the output.

The prior assertion expect(result).toContain(\.push(${payload})`)verified that the payload appeared as the argument to.push(in a single expression. Splitting it into two separate checks means the test passes even if.push(andJSON.stringify(payload)appear in completely different parts of the output (e.g. the payload in the HTML body and.push(` in an unrelated script tag).

Consider tightening it back to:

expect(result).toContain(`.push(${JSON.stringify(payload)})`);

});

it('does not deadlock with large multi-chunk payloads exceeding the default highWaterMark', async () => {
Expand Down
Loading
Loading