Pin Pro RSC runtime to stable 19.0.5#3990
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:
WalkthroughThis PR promotes ChangesRSC Stable Pin Promotion
LLM Reference File Splitting
Sequence Diagram(s)No sequence diagrams apply to this PR. The RSC changes are config/validation updates, and the LLM generation change is an internal orchestration refactor with no new cross-component flows. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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 📦
|
Code ReviewOverall: This is a clean and well-coordinated promotion from a prerelease RC to the stable Concerns1. Peer dependency range is wider than the docs allow
The caret ( The PR description calls this out as a residual risk and notes that runtime checks gate mismatches — that's a reasonable mitigation, but the peer metadata misaligning with the docs is a point of confusion worth resolving. 2. Incidental acorn version bump in lockfileThe lockfile updates include Minor observations3. Test coverage gap: only
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
justin808
force-pushed
the
codex/3965-rsc-pin-range
branch
from
fbc5a10 to
0f5b833
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code Review — PR #3990: Pin Pro RSC runtime to stable 19.0.5OverviewClean, well-scoped promotion of Positives
Issues / SuggestionsMedium — Warning message is misleading for the prerelease-at-stable case
Semver technically agrees (prerelease < stable), but a user on The test at Low —
|
|
Addressed review feedback for current #3990 follow-up. Triage:
Validation after fixes:
Resolved the four Claude review threads after pushing |
Code Review: Pin Pro RSC runtime to stable 19.0.5Overall: Solid, well-scoped change. The RC → stable promotion is clean, the peer metadata is correct, and the split-output generator refactor is a nice improvement. Three issues worth addressing before merge. Must-fix / correctness1. When a user has
Semver-accurate, but a developer reading those patch numbers side by side will be confused. A simple fix: use "does not meet" instead of "is older than" — it's accurate for both the truly-old case and the prerelease-at-floor case without requiring extra branching. 2. Redundant (dead)
Should-fix / test coverage3. No test for a prerelease above the stable floor returning The code comment at lines 157–158 explicitly notes: "Higher prerelease tuples are outside this upgrade nudge." That invariant — e.g. Minor / informational
|
Code Review — PR #3990: Pin Pro RSC runtime to stable 19.0.5OverviewClean, well-scoped PR that promotes the RSC runtime from What works well
Issues1. When
Developers who see the same 2. Dead Inside 3. Missing test: prerelease above the stable floor should be The comment on the new prerelease check says "Higher prerelease tuples are outside this upgrade nudge", but no test verifies that 4. Everything that doesn't start with |
Code Review: Pin Pro RSC runtime to stable 19.0.5OverviewThis PR promotes StrengthsPeer compatibility logic ( Generator simplification Doc split ( Test coverage Minor concernsNon-obvious two-step path for allowed prereleases
No security or correctness bugs foundThe version parsing is defensive (malformed inputs coerce to Approve once minor concerns are addressed or acknowledged. The core logic is correct and well-tested. The inline comments below flag the two points worth a quick look before merge. |
justin808
force-pushed
the
codex/3965-rsc-pin-range
branch
from
5f511de to
1358d45
Compare
justin808
force-pushed
the
codex/3965-rsc-pin-range
branch
from
5805fa8 to
9ba40ab
Compare
![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: 9ba40abdf4
ℹ️ 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".
Code Review — PR #3990: Pin Pro RSC runtime to stable 19.0.5Overall: well-designed and ready with one small defensive fix recommended below. The coordination between three sources of truth — the Ruby generator's The prerelease gating logic is correct end-to-end:
One normalization asymmetry worth fixing (see inline), everything else is good. Specific callouts:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbd0eb5ca6
ℹ️ 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".
|
|
||
| return false; | ||
| }; | ||
|
|
There was a problem hiding this comment.
The last branch silently allows unlisted prereleases of any minor that isn't the recommendedMin minor (e.g., 19.2.0-rc.1 is allowed because rscTuple[1] === 2 !== 0 === recommendedMinMinor). This is intentional — isSupportedRscMinor already gates unsupported minors, and unlisted prereleases of a supported other minor (like the 19.2 line) are meant to pass through — but the design intent isn't obvious from the code alone. A short comment here would prevent future maintainers from removing this line thinking it's dead code when a new minor is added to supportedRanges:
| const recommendedMinMinor = parseTuple(recommendedMin)[1]; | |
| // Unlisted prereleases of other supported minors (e.g. 19.2.0-rc.1 while recommendedMin is 19.0.5) | |
| // pass through here; isSupportedRscMinor already rejected unsupported minors above. | |
| return rscTuple[1] !== recommendedMinMinor; |
| ); | ||
|
|
||
| expect(expectedStableRanges.every((stableRange) => peerRange.includes(stableRange))).toBe(true); | ||
| expect(peerPrereleases).toEqual([...RSC_PEER_SUPPORT.reactOnRailsRsc.allowedPrereleases]); |
There was a problem hiding this comment.
The toEqual assertion checks both content and insertion order. Since peerPrereleases is extracted left-to-right from the peer range string in package.json, the order of ||-separated prerelease entries there must stay in sync with the order of allowedPrereleases in rscPeerSupport.ts. This coupling is correct but non-obvious — a short comment noting it would prevent a silent reorder from breaking the invariant unexpectedly.
Code Review — PR #3990: Pin Pro RSC runtime to stable 19.0.5Overall this is a clean, well-scoped PR. The logic is correct and the cross-package invariant tests are an excellent pattern. Two minor clarity notes: 1.
|
Pro Node Renderer Benchmark Summary
▲/▼ non-zero change vs baseline · 0.0% exact/near-zero match · 🔴 significant regression · 🟢 significant improvement (tracked measures) · (n) = baseline |
|
Superseded by the current release-tracker direction. I rechecked #3823 after #4026 merged. The tracker now says the final path targets the React/RSC 19.2 line and that #3988/#3990 are 19.0.5-era stable-pin variants to treat as superseded/obsolete unless a maintainer explicitly revives the 19.0.5 path. Current head: Merge recommendation: do not merge this PR. Close as superseded and route final-release RSC package work through the 19.2 line after |
Code Review — Pin Pro RSC runtime to stable 19.0.5Overall: clean and well-reasoned. The stable pin promotion is straightforward, the prerelease gating logic (warn for explicitly listed RCs, error for anything else) is sound, and the cross-package invariant tests that guard generator pin ↔ FindingsMinor — Minor — The double- Nit — unsafe type assertion in test helper ( Non-issue confirmed — Non-issue confirmed — Overall merge confidence is high. The three findings above are non-blocking style/robustness nits. |
| const isAllowedRscPrerelease = ( | ||
| rscVersion: string, | ||
| { allowedPrereleases }: RscPeerSupport['reactOnRailsRsc'], | ||
| ): boolean => { | ||
| if (!isPrereleaseVersion(rscVersion)) return true; | ||
|
|
||
| const normalizedRscVersion = stripVersionPrefixAndBuild(rscVersion).toLowerCase(); | ||
| if ( | ||
| allowedPrereleases.some( | ||
| (allowedVersion) => stripVersionPrefixAndBuild(allowedVersion).toLowerCase() === normalizedRscVersion, | ||
| ) | ||
| ) { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| }; |
There was a problem hiding this comment.
The if (condition) { return true; } ... return false; pattern here is equivalent to return condition;, which is cleaner:
| const isAllowedRscPrerelease = ( | |
| rscVersion: string, | |
| { allowedPrereleases }: RscPeerSupport['reactOnRailsRsc'], | |
| ): boolean => { | |
| if (!isPrereleaseVersion(rscVersion)) return true; | |
| const normalizedRscVersion = stripVersionPrefixAndBuild(rscVersion).toLowerCase(); | |
| if ( | |
| allowedPrereleases.some( | |
| (allowedVersion) => stripVersionPrefixAndBuild(allowedVersion).toLowerCase() === normalizedRscVersion, | |
| ) | |
| ) { | |
| return true; | |
| } | |
| return false; | |
| }; | |
| const isAllowedRscPrerelease = ( | |
| rscVersion: string, | |
| { allowedPrereleases }: RscPeerSupport['reactOnRailsRsc'], | |
| ): boolean => { | |
| if (!isPrereleaseVersion(rscVersion)) return true; | |
| const normalizedRscVersion = stripVersionPrefixAndBuild(rscVersion).toLowerCase(); | |
| return allowedPrereleases.some( | |
| (allowedVersion) => stripVersionPrefixAndBuild(allowedVersion).toLowerCase() === normalizedRscVersion, | |
| ); | |
| }; |
| const stableTuple = (version: string) => | ||
| stableBase(version).split('.').map(Number) as [number, number, number]; |
There was a problem hiding this comment.
The as [number, number, number] cast tells TypeScript the array is always length 3 without actually enforcing it — if recommendedMin were ever malformed (e.g. only two segments) the downstream destructuring in versionBelowRecommendedMin would silently receive undefined for patch. Explicit indexing avoids the unsound assertion:
| const stableTuple = (version: string) => | |
| stableBase(version).split('.').map(Number) as [number, number, number]; | |
| const stableTuple = (version: string): [number, number, number] => { | |
| const parts = stableBase(version).split('.').map(Number); | |
| return [parts[0] ?? 0, parts[1] ?? 0, parts[2] ?? 0]; | |
| }; |
| return supportedMinors | ||
| .map((minor) => { | ||
| if (minor !== recommendedMinMinor) return `${supportedMajor}.${minor}.x`; | ||
|
|
||
| const stableRange = `${supportedMajor}.${minor}.x stable >= ${recommendedMin}`; | ||
| if (allowedPrereleases.length === 0) return stableRange; | ||
|
|
||
| return `${stableRange}, or ${allowedPrereleases.join(' / ')} while upgrading from the RC line`; | ||
| }) | ||
| .join(' or '); |
There was a problem hiding this comment.
With the current two-minor configuration (0 and 2) the function produces:
19.0.x stable >= 19.0.5, or 19.0.5-rc.6 / 19.0.5-rc.7 while upgrading from the RC line or 19.2.x
The , or … while upgrading from the RC line suffix on the first item, followed immediately by a plain or 19.2.x from .join(' or '), reads slightly ambiguously — a user could parse 19.2.x as part of the RC upgrade path. The tests use .toContain on substrings so this won't cause test failures. Low priority, but worth a small tweak if the message is ever revisited (e.g. wrapping the prerelease clause in parentheses, or promoting the supported stable ranges to a separate sentence).
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 |
1 participant
Summary
react-on-rails-rsc@19.0.5after confirmingpnpm view react-on-rails-rsc versionreturns19.0.5.~19.0.5 || 19.0.5-rc.6 || 19.0.5-rc.7, keeping the coordinated rc.6/rc.7 upgrade warning path explicit while preventing unvalidated 19.1+ RSC lines.llms-fulloutput, and peer-compatibility guardrail tests so the Ruby generator pin, Pro peer exceptions, and node-renderer runtime checks stay aligned.Issue Dispositions
react-on-rails-rsc@19.0.5is published and this branch pins/ranges the Pro RSC runtime accordingly.origin/mainafter the Add cross-agent coordination backend workflow #3977 coordination workflow.llms-fullfiles were refreshed from current sources only.Agent Merge Confidence
Mode: accelerated-rc (#3823)
Current head SHA: 5805fa8
Score: 9/10
Auto-merge recommendation: yes
Affected areas: Pro RSC peer compatibility, generator RSC dependency pin, RSC upgrade docs, generated LLM references, lockfile/package metadata
CI detector:
script/ci-changes-detector origin/main-> broad package/Pro/RSC/docs coverage selected; labelsfull-ci,benchmarkare present and appropriate.Validation run:
pnpm view react-on-rails-rsc version->19.0.5.pnpm install --lockfile-only --ignore-scripts-> passed.node script/generate-llms-full.mjs-> regeneratedllms-full.txtandllms-full-pro.txtunder the 2048 KiB split threshold.node script/generate-llms-full.mjs --check-> passed.bash script/generate-llms-full-test.bash-> passed, 5 tests.pnpm --dir packages/react-on-rails-pro-node-renderer exec jest tests/checkRscPeerCompatibility.test.ts --runInBand-> passed, 21 tests, including current-head review fix5805fa81b.pnpm --dir packages/react-on-rails-pro-node-renderer run type-check-> passed, including current-head review fix5805fa81b.pnpm --dir packages/react-on-rails-pro-node-renderer exec prettier --check packages/react-on-rails-pro-node-renderer/tests/checkRscPeerCompatibility.test.ts-> passed for current-head review fix5805fa81b.(cd react_on_rails && bundle exec rspec spec/react_on_rails/generators/js_dependency_manager_spec.rb spec/react_on_rails/generators/install_generator_spec.rb)-> passed, 608 examples, 0 failures.(cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop)-> passed, 218 files, no offenses.pnpm run lint-> passed afterpnpm install --ignore-scriptsrefreshed stale localnode_modulesfrom the committed lockfile.pnpm start format.listDifferent-> passed.script/check-docs-sidebar-> passed.PATH=/tmp/lychee-v0.23.0:$PATH lychee --config .lychee.toml docs/pro/react-server-components/upgrading-existing-pro-app.md docs/pro/react-server-components/create-without-ssr.md docs/pro/react-server-components/rspack-compatibility.md-> passed.git diff --check origin/main...HEADandgit diff --check-> passed.5805fa81b34b348549e1f90f25ace652d4b83e1c.Review/check gate:
5805fa81b34b348549e1f90f25ace652d4b83e1c; fullgh pr checks 3990shows all current-head checks passing or intentionally skipped by workflow selection. Passing coverage includes package JS tests, generator/unit specs, dummy app integration/e2e, Pro lint, builds, CodeQL, docs/link/sidebar checks, full benchmarks,claude-review, Cursor Bugbot, and CodeRabbit skipped.vis defensive future-proofing only, and using a set comparison for semver-range prerelease order is a maintenance nit while the range/order is currently stable and tested. No unresolved non-trivial concerns remain.claude-reviewpassed for5805fa81b34b348549e1f90f25ace652d4b83e1c; Cursor Bugbot passed; CodeRabbit skipped its review by automation and reported no blocking status.Feedback triage:
create-without-ssr.md,upgrading-existing-pro-app.md, and troubleshooting commands; generatedllms-full-pro.txtis current.llms-full.txtandllms-full-pro.txtfrom the current generator.pnpm install --lockfile-only --ignore-scriptsalso refreshed transitiveacornsnapshot references from8.15.0to8.16.0underacorn-jsx,espree,jsdom, andterser. No direct manifest dependency was added for that drift.Labels:
full-ci,benchmark. This branch changes Pro/RSC compatibility, generated docs, generator dependency behavior, package metadata, and lockfiles; broad CI plus benchmarks are appropriate.Known residual risk: Low. The remaining risk is normal dependency/lockfile-release surface risk; it is mitigated by local validation, full current-head CI, benchmarks, and current-head independent review checks.
Finalized by:
claude-reviewGitHub check for current head5805fa81b34b348549e1f90f25ace652d4b83e1c(independent named check/app identity), with Cursor Bugbot also passing for the same head.Current Readiness
Merge-ready under the accelerated-RC criteria: current-head local validation is complete, full GitHub checks and benchmarks are green or intentionally skipped, current-head review feedback is fixed or triaged above, and the release tracker identifies this stable RSC pin as the remaining final-only blocker path. Merge recommendation: squash merge now.
Note
Medium Risk
Touches Pro RSC dependency policy and startup peer validation, which can block or warn mis-versioned apps; scope is metadata/docs/runtime checks rather than core rendering logic.
Overview
Moves Pro RSC installs and docs from
react-on-rails-rsc@19.0.5-rc.7to stable19.0.5: the Ruby generator pin, root/Pro/dummypackage.jsonentries, lockfile, and Pro RSC upgrade guides (plus regeneratedllms-full*output).react-on-rails-prono longer uses a wildcard optional peer forreact-on-rails-rsc; it declares~19.0.5 || ~19.2.0plus explicit19.0.5-rc.6/19.0.5-rc.7exceptions so existing RC apps can still install while the node renderer nudges them to stable.The Pro node renderer’s
checkRscPeerCompatibility/RSC_PEER_SUPPORTnow treat19.0.5as the recommended floor, allow only listed prereleases, error on other prereleases, and warn when an allowed RC matches the stable minimum; Jest specs assert the generator pin, Pro peer range, and runtime metadata stay in sync.Reviewed by Cursor Bugbot for commit 0c90b7d. Bugbot is set up for automated code reviews on this repo. Configure here.