Fix unverified claims and errors in RSC migration docs#2788
Conversation
- Replace deprecated @rails/webpacker require with shakapacker API - Remove unverifiable Frigade case study statistics - Correct styled-components RSC support claim with verified release info - Soften unverifiable react-on-rails-rsc version matching claims - CVE numbers (C1) and Date serialization (C6) already correct/absent Closes #2525 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughDocs-only edits: removed a specific numeric performance claim, softened React-version matching guidance, clarified styled-components RSC compatibility details, and replaced a deprecated Webpacker example with a Shakapacker webpack.config snippet. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
|
Overall, this is a solid documentation cleanup. The factual corrections are well-reasoned and improve accuracy. I have one substantive bug to flag and two minor concerns. Bug: webpack example is missing module.exports The new Shakapacker snippet in rsc-troubleshooting.md modifies webpackConfig.plugins but never exports the result. As written, the file produces no output — any consumer of this config will receive undefined, and the EnvironmentPlugin addition will silently have no effect. Compare with the actual repo pattern in react_on_rails/spec/dummy/config/webpack/webpack.config.js — it always ends with module.exports = webpackConfig. The snippet needs that line appended. Minor: wrong filename in comment The snippet comment says '// config/webpack/environment.js (Shakapacker)' but environment.js is the old @rails/webpacker pattern. The generateWebpackConfig() API belongs in webpack.config.js (or a shared commonWebpackConfig.js). Leaving the wrong filename will confuse users migrating from webpacker who already have an environment.js. Minor: removal of security language In rsc-preparing-app.md, the original callout mentioned that 19.0.4 fixed 'upstream security patches'. The new text drops that justification. If those patches were real, removing the framing weakens the upgrade urgency for users still on 19.0.0-19.0.3. Worth confirming whether the CVE language was verified before removing it entirely. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 942107b615
ℹ️ 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.
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-third-party-libs.md`:
- Around line 97-100: Update the styled-components row in the table: remove the
incorrect claim that v6.3.9 "handling React 19's `precedence` attribute",
instead note the accurate v6.3.9 fix (createGlobalStyle unmount cleanup / no
precedence attr) and, if desired, mention that React 19 precedence-related
behavior was introduced in v6.3.0 while keeping the verified v6.3.3 entry about
suppressing server-side warnings; make these edits to the styled-components cell
where "v6.3.x" and the v6.3.3/v6.3.9 text appear so the table accurately
reflects releases.
🪄 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: 14098e41-e825-443a-be18-9d2cb3281cfa
📒 Files selected for processing (4)
docs/oss/migrating/migrating-to-rsc.mddocs/oss/migrating/rsc-preparing-app.mddocs/oss/migrating/rsc-third-party-libs.mddocs/oss/migrating/rsc-troubleshooting.md
Greptile SummaryThis PR addresses five categories of unverified or erroneous claims in the RSC migration docs. Four of the five fixes are clean and well-targeted; however, two of the four changed files introduce new accuracy concerns. Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR #2788: Fix unverified claims in RSC docs] --> B[C2: @rails/webpacker → shakapacker]
A --> C[C3: Remove Frigade 62% stat]
A --> D[C4: styled-components version details]
A --> E[C5: Remove major.minor version claim]
B --> B1[rsc-troubleshooting.md\nenvironment.plugins.prepend\n→ generateWebpackConfig + plugins.push]
B1 --> B2{API pattern matches\nenv filename?}
B2 -->|No| B3[⚠ generateWebpackConfig is webpack.config.js API\nenvironment.js should use environment DSL]
C --> C1[migrating-to-rsc.md\nRemoved specific Frigade link + stat\nReplaced with generic case study reference]
C1 --> C2OK[✅ No broken links, claim is now defensible]
D --> D1[rsc-third-party-libs.md\nv6.3.0+ → v6.3.x series with specifics\nv6.3.3 server warnings, v6.3.9 precedence attr]
D1 --> D2{v6.3.9 precedence attr\nclaim accurate?}
D2 -->|No: release notes show v6.3.7| D3[⚠ v6.3.9 fixed CSS block comments\nprecedence attr was v6.3.7]
E --> E1[rsc-preparing-app.md\nRemoved major.minor must match React\nReplace with README reference]
E1 --> E2[✅ Defers to authoritative source]
Last reviewed commit: "Fix unverified claim..." |
- Add missing `module.exports = webpackConfig;` to Shakapacker snippet (without it the EnvironmentPlugin addition silently has no effect) - Correct filename comment from `environment.js` to `webpack.config.js` (`generateWebpackConfig()` belongs in webpack.config.js, not environment.js) - Fix styled-components precedence attribute version from v6.3.9 to v6.3.7 (v6.3.9 fixed CSS block comments; precedence handling was v6.3.7) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
Re: the security language removal concern — The PR's explicit goal is removing unverified claims. The original text stated "upstream security patches" as justification for 19.0.4 but that framing was not verified against actual CVE records. The current text still recommends 19.0.4+ and explains the concrete reason (updated The webpack snippet and version attribution issues have both been addressed in 95ea3ef. |
Review: Fix unverified claims and errors in RSC migration docsOverall, this is a solid cleanup of factual issues. The changes are well-scoped and the editorial decisions are sound. Webpack snippet (rsc-troubleshooting.md) OK - The migration from Frigade stat removal (migrating-to-rsc.md) OK - Removing the specific 62% figure was the right call. The new phrasing is appropriately general. Version compatibility (rsc-preparing-app.md) Minor concern - The removal of 'upstream security patches' from the 19.0.4 requirement note is editorially correct (the specific CVE claims were unverifiable), but the resulting text 'updated in 19.0.4' gives readers no signal about why the upgrade matters. A middle ground like 'with security-related fixes' would convey urgency without making unverifiable claims. styled-components versions (rsc-third-party-libs.md) Minor concern - The PR self-corrects v6.3.9 to v6.3.7, which is good. These specific version claims (v6.3.3, v6.3.7) should be verified against the styled-components CHANGELOG before merge to avoid introducing the same category of error this PR is fixing. |
The previous commit changed v6.3.9 to v6.3.7 based on a reviewer suggestion, but the styled-components CHANGELOG confirms v6.3.9 is correct: it fixed createGlobalStyle unmount behavior related to React 19's precedence attribute. v6.3.7 addressed StrictMode/RSC compat for createGlobalStyle with no mention of precedence. Also updated the description to more precisely describe the v6.3.9 fix. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Review: Fix unverified claims and errors in RSC migration docsOverall this is a solid set of corrections — the changes are well-targeted and the motivations are clearly documented. What's good:
Two minor concerns (inline comments posted):
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4698be287
ℹ️ 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".
## Summary - Stamp `### [16.5.0.rc.0]` version header with today's date - Add 10 new changelog entries for PRs merged since v16.4.0 - Fix incomplete PR 2818 entry (missing author link) ### New entries added **Added:** - `create-react-on-rails-app --pro` support (PR 2818) - Global prerender env override `REACT_ON_RAILS_PRERENDER_OVERRIDE` (PR 2816) - `react_on_rails:sync_versions` rake task (PR 2797) - Pro/RSC setup checks in `react_on_rails:doctor` (PR 2674) **Changed:** - [Pro] Canonical env var for worker count is now `RENDERER_WORKERS_COUNT` (PR 2611) **Improved:** - Smoother `create-react-on-rails-app` and install generator flows (PR 2650) - Pro upgrade hint after install (PR 2642) **Fixed:** - Preserve runtime env vars across `Bundler.with_unbundled_env` (PR 2836) - Fix doctor prerender check and ExecJS display for Pro/RSC apps (PR 2773) - Fix doctor false positives for custom layouts (PR 2612) ### Skipped PRs (not user-visible) Docs-only: #2845, #2842, #2826, #2830, #2820, #2809, #2803, #2785, #2801, #2791, #2789, #2788, #2772, #2778, #2780, #2784, #2671, #2676, #2662, #2657, #2669 CI/internal tooling: #2825, #2817, #2819, #2812, #2815, #2810, #2808, #2807, #2634, #2798, #2761, #2760, #2658, #2639, #2667, #2656 ## Test plan - [x] Verified version header and diff links are correct - [x] Verified all entries follow changelog formatting conventions - [x] Verified file ends with newline - [ ] After merge, run `rake release` to publish 16.5.0.rc.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only change updating `CHANGELOG.md` with a new `16.5.0.rc.0` section and compare links; no runtime code is modified. > > **Overview** > Adds a new `16.5.0.rc.0` (2026-03-25) section to `CHANGELOG.md`, consolidating recent PR entries under **Added/Changed/Improved/Fixed** and correcting the previously incomplete `--pro` CLI entry author attribution. > > Updates the bottom compare links so `[unreleased]` now compares from `v16.5.0.rc.0` and adds a link definition for `[16.5.0.rc.0]`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 481a71c. 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 ## Release Notes - v16.5.0.rc.0 * **New Features** * Added sync_versions task for streamlined version management * Expanded doctor checks for Pro and RSC support * **Improvements** * Enhanced generator workflow and Pro upgrade guidance * Improved environment variable handling and preservation * **Bug Fixes** * Fixed detection issues with doctor tools and ExecJS/prerender functionality <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
## Summary Fixes factual errors and unverified claims in RSC migration docs flagged in #2525: - **C1 (CVEs)**: Already absent from codebase — no action needed - **C2 (`@rails/webpacker`)**: Replaced deprecated `require('@rails/webpacker')` with Shakapacker's `require('shakapacker')` API in `rsc-troubleshooting.md` - **C3 (Frigade stats)**: Removed unverifiable "62% reduction" statistic; replaced with generic attribution to RSC adoption case studies - **C4 (styled-components)**: Replaced vague "v6.3.0+ added RSC support" with verified release-specific details (v6.3.3 server warnings, v6.3.9 precedence attribute) - **C5 (version matching)**: Removed unverifiable claim that `react-on-rails-rsc` major.minor must match `react`; replaced with direction to check the package README - **C6 (`Date` objects)**: Correct for React 19's Flight protocol (which RSC requires) — no change needed Closes #2525 ## Test plan - [ ] Verify all four changed markdown files render correctly - [ ] Confirm no broken links introduced 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only updates that adjust factual claims and config examples; low risk aside from potential reader confusion if the new Shakapacker snippet doesn't match a given app's setup. > > **Overview** > Removes or softens unverified performance and compatibility claims in the RSC migration docs (e.g., replaces a specific bundle-size reduction stat with general case-study wording, and revises `react-on-rails-rsc` version guidance to point to the package docs). > > Clarifies third-party library notes (more specific `styled-components` v6.3.x behavior) and fixes a Shakapacker environment-variable injection example to use `generateWebpackConfig`/`webpackConfig.plugins.push` rather than the deprecated `@rails/webpacker`-style snippet. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a4698be. 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** * Reworded RSC migration performance claims to cite “significant reductions” instead of a specific percentage. * Clarified React-on-Rails-RSC version guidance, directing readers to the package README/changelog for supported React ranges. * Expanded styled-components compatibility notes with more granular v6.3.x behavior details. * Updated environment-variable injection guidance to use the Shakapacker workflow with an explicit webpack config example. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
## Summary Fixes factual errors and unverified claims in RSC migration docs flagged in #2525: - **C1 (CVEs)**: Already absent from codebase — no action needed - **C2 (`@rails/webpacker`)**: Replaced deprecated `require('@rails/webpacker')` with Shakapacker's `require('shakapacker')` API in `rsc-troubleshooting.md` - **C3 (Frigade stats)**: Removed unverifiable "62% reduction" statistic; replaced with generic attribution to RSC adoption case studies - **C4 (styled-components)**: Replaced vague "v6.3.0+ added RSC support" with verified release-specific details (v6.3.3 server warnings, v6.3.9 precedence attribute) - **C5 (version matching)**: Removed unverifiable claim that `react-on-rails-rsc` major.minor must match `react`; replaced with direction to check the package README - **C6 (`Date` objects)**: Correct for React 19's Flight protocol (which RSC requires) — no change needed Closes #2525 ## Test plan - [ ] Verify all four changed markdown files render correctly - [ ] Confirm no broken links introduced 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only updates that adjust factual claims and config examples; low risk aside from potential reader confusion if the new Shakapacker snippet doesn't match a given app's setup. > > **Overview** > Removes or softens unverified performance and compatibility claims in the RSC migration docs (e.g., replaces a specific bundle-size reduction stat with general case-study wording, and revises `react-on-rails-rsc` version guidance to point to the package docs). > > Clarifies third-party library notes (more specific `styled-components` v6.3.x behavior) and fixes a Shakapacker environment-variable injection example to use `generateWebpackConfig`/`webpackConfig.plugins.push` rather than the deprecated `@rails/webpacker`-style snippet. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a4698be. 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** * Reworded RSC migration performance claims to cite “significant reductions” instead of a specific percentage. * Clarified React-on-Rails-RSC version guidance, directing readers to the package README/changelog for supported React ranges. * Expanded styled-components compatibility notes with more granular v6.3.x behavior details. * Updated environment-variable injection guidance to use the Shakapacker workflow with an explicit webpack config example. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Summary
Fixes factual errors and unverified claims in RSC migration docs flagged in #2525:
@rails/webpacker): Replaced deprecatedrequire('@rails/webpacker')with Shakapacker'srequire('shakapacker')API inrsc-troubleshooting.mdreact-on-rails-rscmajor.minor must matchreact; replaced with direction to check the package READMEDateobjects): Correct for React 19's Flight protocol (which RSC requires) — no change neededCloses #2525
Test plan
🤖 Generated with Claude Code
Note
Low Risk
Documentation-only updates that adjust factual claims and config examples; low risk aside from potential reader confusion if the new Shakapacker snippet doesn't match a given app's setup.
Overview
Removes or softens unverified performance and compatibility claims in the RSC migration docs (e.g., replaces a specific bundle-size reduction stat with general case-study wording, and revises
react-on-rails-rscversion guidance to point to the package docs).Clarifies third-party library notes (more specific
styled-componentsv6.3.x behavior) and fixes a Shakapacker environment-variable injection example to usegenerateWebpackConfig/webpackConfig.plugins.pushrather than the deprecated@rails/webpacker-style snippet.Written by Cursor Bugbot for commit a4698be. This will update automatically on new commits. Configure here.
Summary by CodeRabbit