Fix Pro FOUC reveal gating#4047
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end FOUC prevention for RSC and client-only components by coordinating server-side stylesheet href embedding, client-side CSS-ready gating before hydration, and RSC stream preload-to-stylesheet promotion including across chunk boundaries. Pending Playwright FOUC tests are now unconditionally enabled. ChangesRSC/Client-Only FOUC CSS Gating
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ProHelper as Ruby ProHelper
participant injectRSCPayload
participant ClientSideRenderer
rect rgba(59, 130, 246, 0.5)
note over ProHelper: Server render
ProHelper->>Browser: component script with data-generated-stylesheet-hrefs=[...hrefs]
end
rect rgba(234, 179, 8, 0.5)
note over injectRSCPayload: RSC stream processing
injectRSCPayload->>injectRSCPayload: detect <link rel="preload" as="style"> in chunk
injectRSCPayload->>injectRSCPayload: handle split tag across chunk boundary (holdIncompleteLinkTagTail)
injectRSCPayload->>Browser: <link rel="stylesheet" data-precedence="rsc-css"> (before streamed HTML)
end
rect rgba(34, 197, 94, 0.5)
note over ClientSideRenderer: Client hydration gating
ClientSideRenderer->>Browser: parse data-generated-stylesheet-hrefs
ClientSideRenderer->>Browser: match <link rel="stylesheet"> by href
Browser-->>ClientSideRenderer: load event (or 10s timeout)
ClientSideRenderer->>Browser: reactHydrateOrRender (component revealed)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
size-limit report 📦
|
Greptile SummaryThis PR fixes two FOUC (Flash of Unstyled Content) bugs (#4031, #4032) in React on Rails Pro by gating component reveal on stylesheet readiness from two angles. Four previously-skipped Playwright FOUC tests are now enabled.
Confidence Score: 4/5Safe to merge; the new stylesheet-gating paths fail open on timeout or missing DOM links, so no regressions on existing pages without generated stylesheets. Both FOUC gating mechanisms are carefully implemented with defensive fallbacks: the client-side renderer times out after 10 seconds rather than blocking forever, and the stream-side link-tag promotion correctly handles mid-chunk splits and restores the original buffer when nothing changes. The only non-trivial risks are the unescaped regex interpolation in The split-chunk link-tag handling in Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-on-rails-pro/src/ClientSideRenderer.ts (1)
129-140: Useasync/awaitinwaitForGeneratedComponentStylesheetsinstead of.then(...)to align with repo conventions.The function uses
.then()at line 139, which conflicts with the TypeScript/JavaScript guideline that prefers async/await for promise handling.Proposed refactor
-function waitForGeneratedComponentStylesheets(componentName: string, componentSpec: Element): Promise<void> { +async function waitForGeneratedComponentStylesheets( + componentName: string, + componentSpec: Element, +): Promise<void> { const generatedStylesheetHrefs = generatedStylesheetHrefsForComponent(componentSpec); const stylesheetLinks = Array.from( document.querySelectorAll<HTMLLinkElement>('link[rel~="stylesheet"][href]'), ).filter((link) => generatedStylesheetMatchesComponent(link, componentName, generatedStylesheetHrefs)); if (stylesheetLinks.length === 0) { - return Promise.resolve(); + return; } - return Promise.all(stylesheetLinks.map(waitForStylesheet)).then(() => undefined); + await Promise.all(stylesheetLinks.map(waitForStylesheet)); }🤖 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 `@packages/react-on-rails-pro/src/ClientSideRenderer.ts` around lines 129 - 140, Convert the waitForGeneratedComponentStylesheets function to use async/await instead of .then() chaining to align with repository conventions. Mark the function as async, replace the Promise.all().then(() => undefined) chain with an await statement followed by the appropriate return, and ensure the function still returns a Promise<void> as expected by its callers.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@packages/react-on-rails-pro/src/ClientSideRenderer.ts`:
- Around line 129-140: Convert the waitForGeneratedComponentStylesheets function
to use async/await instead of .then() chaining to align with repository
conventions. Mark the function as async, replace the Promise.all().then(() =>
undefined) chain with an await statement followed by the appropriate return, and
ensure the function still returns a Promise<void> as expected by its callers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df5b240f-bb38-4f21-982f-564d0cb40497
📒 Files selected for processing (8)
CHANGELOG.mdpackages/react-on-rails-pro/src/ClientSideRenderer.tspackages/react-on-rails-pro/src/injectRSCPayload.tspackages/react-on-rails-pro/tests/ClientSideRenderer.test.tspackages/react-on-rails-pro/tests/injectRSCPayload.test.tsreact_on_rails/lib/react_on_rails/pro_helper.rbreact_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rbreact_on_rails_pro/spec/dummy/e2e-tests/rsc_fouc.spec.ts
|
Follow-up from the failed Root cause: the Rspack RSC stream can reveal client-component HTML from the Flight Fix in Local validation:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Reviewed by Cursor Bugbot for commit 704ed82. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 704ed821f0
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 915139ab5b
ℹ️ 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".
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-on-rails-pro/src/injectRSCPayload.ts (1)
199-205: 💤 Low valueStatic analysis flagged potential ReDoS, but risk is mitigated here.
The
revealedContentIdis extracted from React's own$RC()call and represents server-generated boundary IDs (e.g.,"RscFoucProbe-react-component-0S:0"), not arbitrary user input. Combined withescapeRegExpLiteralsanitizing the value and the[^>]*patterns being bounded by the>delimiter, catastrophic backtracking is unlikely in practice.Consider adding a length check on
revealedContentIdas a defensive measure if you want extra protection against malformed streams:+ if (revealedContentId && revealedContentId.length < 256) { - if (revealedContentId) {🤖 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 `@packages/react-on-rails-pro/src/injectRSCPayload.ts` around lines 199 - 205, Add a defensive length check on the revealedContentId variable in the hiddenBoundaryPattern block to protect against malformed streams that could potentially cause ReDoS issues. Before constructing the RegExp object with the hiddenBoundaryPattern regex, verify that revealedContentId is not excessively long by adding a reasonable maximum length constraint (such as checking against a defined constant or a reasonable upper bound) in addition to the existing escapeRegExpLiteral sanitization.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@packages/react-on-rails-pro/src/injectRSCPayload.ts`:
- Around line 199-205: Add a defensive length check on the revealedContentId
variable in the hiddenBoundaryPattern block to protect against malformed streams
that could potentially cause ReDoS issues. Before constructing the RegExp object
with the hiddenBoundaryPattern regex, verify that revealedContentId is not
excessively long by adding a reasonable maximum length constraint (such as
checking against a defined constant or a reasonable upper bound) in addition to
the existing escapeRegExpLiteral sanitization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5664e6b5-fd79-41d1-a60f-d330547594fb
📒 Files selected for processing (2)
packages/react-on-rails-pro/src/injectRSCPayload.tspackages/react-on-rails-pro/tests/injectRSCPayload.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70bf6dcf15
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ef364688f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d45f32cfd
ℹ️ 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".
|
Merge Ledger Block: formal review decision needed Current head: Closeout evidence:
Maintainer action requested: leave a formal GitHub review decision (approve, request changes, or comment with the intended review state). After that, rerun |
|
Approved by maintainer, pending more adversarial AI reviews. |
Pro Node Renderer Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Core Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Pro (shard 2/2) Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
Pro (shard 1/2) Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
|
Merge Ledger Block: formal review decision still needed Current head: Closeout evidence refreshed after adversarial review:
Maintainer action requested: submit a formal GitHub review decision for the current head. A regular PR comment such as “approved” still leaves GitHub |
|
Maintainer Waiver And Immediate Merge Decision Current head: Final live gate snapshot before merge:
Final waiver set:
Confidence note:
|
## Summary Stamps the `17.0.0.rc.5` changelog header ahead of the release. This collects the work merged since `v17.0.0.rc.4` (2026-06-14). A classification sweep of all 14 merged PRs in `v17.0.0.rc.4..origin/main` found exactly one user-visible change: - **[Pro]** RSC and client-only FOUC reveal gating ([PR 4047](#4047)) — already present under `[Unreleased]`, now moved under the new `17.0.0.rc.5` header. The other 13 PRs were all maintainer/agent tooling, CI-command workflows, and docs (release-process / internal), so no new entries were added. ## Changes - Inserted `### [17.0.0.rc.5] - 2026-06-16` immediately after `### [Unreleased]`. - Moved the existing Pro FOUC reveal-gating entry under the new header. - Updated compare links: `[unreleased]` → `v17.0.0.rc.5...main`; added `[17.0.0.rc.5]` → `v17.0.0.rc.4...v17.0.0.rc.5`. - Prior RC sections (`rc.4`…`rc.0`) and their compare links left intact (RC sections are coalesced only at the stable release). ## Next step After merge, run `rake release` (no args) — it reads `17.0.0.rc.5` from CHANGELOG.md and auto-creates the GitHub release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog and release-link updates with no application code impact. > > **Overview** > Prepares **17.0.0.rc.5** (2026-06-16) in `CHANGELOG.md` by adding the version section and moving the existing **[Pro] RSC and client-only FOUC reveal gating** fix (PR 4047) out of `[Unreleased]`. > > Compare links are updated so `[unreleased]` points at `v17.0.0.rc.5...main` and a new `[17.0.0.rc.5]` link covers `v17.0.0.rc.4...v17.0.0.rc.5`. No runtime or library code changes. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b7ad049. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated changelog documentation to prepare for version 17.0.0.rc.5 release with updated version comparison references. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Manual verification: reproduced the bug and confirmed the fix ✅Reproduced via the merged jest regression suites ( ReproductionSquash-merged at Results
The 9 failures are exactly the CSS-before-reveal / preload-promotion gating cases. CaveatMechanism-level against the real |
… gate The RSC FOUC ShakaPerf release gate detected the injected stylesheet with `link[rel="stylesheet"][data-precedence="ror-rsc"]`, but the Pro RSC renderer emits `data-precedence="rsc-css"` (packages/react-on-rails-pro/src/injectRSCPayload.ts). The `ror-rsc` wrapper this selector originally targeted (#3587) was reverted in #3860 and replaced by the current `rsc-css` mechanism in #4047, so the selector matched nothing and the gate's `hasStylesheet` assertion was silently inert. Point the selector at the value the renderer actually emits and add a comment pinning it to the source of truth so it does not drift again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Fixes #4031.
Fixes #4032.
Summary
CHANGELOG.md.Validation
pnpm exec jest tests/ClientSideRenderer.test.ts tests/injectRSCPayload.test.ts --runInBandbundle exec rspec spec/helpers/react_on_rails_helper_spec.rb:63 spec/helpers/react_on_rails_helper_spec.rb:1402pnpm run buildpnpm run type-checkpnpm exec eslint packages/react-on-rails-pro/src/ClientSideRenderer.ts packages/react-on-rails-pro/src/injectRSCPayload.ts packages/react-on-rails-pro/tests/ClientSideRenderer.test.ts packages/react-on-rails-pro/tests/injectRSCPayload.test.ts react_on_rails_pro/spec/dummy/e2e-tests/rsc_fouc.spec.tspnpm exec prettier --check packages/react-on-rails-pro/src/ClientSideRenderer.ts packages/react-on-rails-pro/src/injectRSCPayload.ts packages/react-on-rails-pro/tests/ClientSideRenderer.test.ts packages/react-on-rails-pro/tests/injectRSCPayload.test.ts react_on_rails_pro/spec/dummy/e2e-tests/rsc_fouc.spec.tsbundle exec rubocop react_on_rails/lib/react_on_rails/helper.rb react_on_rails/lib/react_on_rails/pro_helper.rb react_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rbcd react_on_rails_pro/spec/dummy && pnpm run build:testcd react_on_rails_pro/spec/dummy && pnpm exec playwright test e2e-tests/rsc_fouc.spec.tspnpm exec prettier --check CHANGELOG.mdgit diff --check/Users/justin/.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/mainReview Notes
/code-review origin/mainand/simplify origin/mainwere attempted withclaude-opus-4-8 --max-budget-usd 20, but both CLI runs hung without output and returned onlyExecution errorafter interrupt.Note
Medium Risk
Touches timing-sensitive Pro streaming and client hydration paths with substantial HTML stream rewriting; mitigated by scoped matching rules, inference timeouts, and fail-open behavior when CSS never loads.
Overview
Fixes Pro FOUC where client-only roots and streamed RSC Suspense reveals could show unstyled content because hydration/reveal ran before generated or chunk CSS was applied.
Client-only: Pro now waits in parallel for the component bundle and for matching generated stylesheet
linkelements (fromdata-generated-stylesheet-hrefson the spec tag plus/generated/{Component}href heuristics), with a 10s fail-open timeout. Rails emits that metadata whenauto_load_bundleresolves manifest preload hrefs forgenerated/{component}.Streamed RSC:
injectRSCPayloadpromotes only RSC client-chunkrel=preload as=stylelinks to real stylesheets (data-precedence="rsc-css"), infers extra stylesheet tags from Flight +loadable-stats.json, and can defer$RCreveal HTML until inference finishes—while still streaming fallbacks and handling link tags split across chunks or UTF-8 boundaries. App-authored style preloads are left unchanged.Playwright FOUC acceptance tests for #4031/#4032 are enabled (no longer
fixme); Jest coverage expanded for both paths.Reviewed by Cursor Bugbot for commit 69251bd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit