Tighten migration preflights from validation runs#2653
Conversation
|
Important Review skippedToo many files! This PR contains 177 files, which is 27 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (177)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![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: 5e6fe6af89
ℹ️ 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".
| bundle exec rails generate react_on_rails:install | ||
| ``` | ||
|
|
||
| If you use Yarn and `package.json` does not already declare it, set the package manager before running the generator: |
There was a problem hiding this comment.
This section tells Yarn users to set "packageManager" before running the generator, but the immediately preceding command block already runs bundle exec rails generate react_on_rails:install; users who copy the block top-to-bottom will still hit the Shakapacker 9 preflight failure the new guidance is trying to avoid. Please move the package-manager command ahead of the generator in the runnable sequence so the documented order matches the stated prerequisite.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7caa04f37. I split the vite_rails migration steps so the optional Yarn packageManager setup now appears before the generator instead of after it.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| If you use Yarn and `package.json` does not already declare it, set the package manager before running the generator: | ||
|
|
||
| ```bash | ||
| npm pkg set packageManager="yarn@1.22.22" |
There was a problem hiding this comment.
Using npm pkg set to configure Yarn as the package manager may confuse users who are switching away from npm, and it assumes npm is available (or that Node ships it). Consider using yarn set version 1.22.22 (requires Corepack/Yarn installed) or just documenting the manual JSON edit, which is unambiguous:
| npm pkg set packageManager="yarn@1.22.22" | |
| echo ' "packageManager": "yarn@1.22.22"' # → add this field to package.json manually |
Or, if you want to keep a runnable command, note that this just edits package.json and requires Node/npm already installed:
# This only edits package.json — it does not install Yarn
npm pkg set packageManager="yarn@1.22.22"There was a problem hiding this comment.
Fixed in 7caa04f37. The docs now call out that npm pkg set only edits package.json, explicitly mention the manual-edit alternative, and label the example as Yarn Classic-specific.
| 2. Remove `react_ujs` from `package.json`. | ||
| 3. Run `bundle install` and your package manager's install command. | ||
| 4. Commit changes. | ||
| 3. If the repo uses Yarn and `package.json` does not already declare it, add `"packageManager": "yarn@1.22.22"` before booting the app with Shakapacker 9. |
There was a problem hiding this comment.
This step tells users to add the field manually (no command shown), while the vite migration guide provides an explicit npm pkg set command for the same task. Consider adding the command here for consistency:
| 3. If the repo uses Yarn and `package.json` does not already declare it, add `"packageManager": "yarn@1.22.22"` before booting the app with Shakapacker 9. | |
| 3. If the repo uses Yarn and `package.json` does not already declare it, add the package manager field before booting the app with Shakapacker 9: `npm pkg set packageManager="yarn@1.22.22"` (or add it manually to `package.json`). |
There was a problem hiding this comment.
Fixed in 7caa04f37. The react-rails migration guide now uses the same explicit command/manual-edit guidance as the vite_rails guide so the package-manager preflight reads consistently.
| - If `bundle install` fails on older native gems such as `pg`, `nio4r`, or `msgpack`, refresh those gems or use the Ruby version already pinned by the app before introducing React on Rails. | ||
| - If `bundle install` fails on older native gems such as `pg`, `nio4r`, `mysql2`, or `msgpack`, refresh those gems or use the Ruby version already pinned by the app before introducing React on Rails. | ||
| - If the app has an older Bundler-era lockfile, refresh that lockfile first. | ||
| - If the repo uses Yarn and has a `yarn.lock` but no `"packageManager"` field in `package.json`, add one before introducing Shakapacker 9. Example: `"packageManager": "yarn@1.22.22"`. |
There was a problem hiding this comment.
yarn@1.22.22 is Yarn Classic (v1). Apps using Yarn Berry (v2+) need a different version string. Consider softening this to make clear it's an example for Yarn 1.x apps:
| - If the repo uses Yarn and has a `yarn.lock` but no `"packageManager"` field in `package.json`, add one before introducing Shakapacker 9. Example: `"packageManager": "yarn@1.22.22"`. | |
| - If the repo uses Yarn Classic and has a `yarn.lock` but no `"packageManager"` field in `package.json`, add one before introducing Shakapacker 9. Example for Yarn 1.x: `"packageManager": "yarn@1.22.22"`. Use the version that matches your project's Yarn installation. |
There was a problem hiding this comment.
Fixed in 7caa04f37. I softened the wording to make yarn@1.22.22 an example for Yarn Classic and explicitly tell readers to use the version that matches their project.
Greptile SummaryThis PR adds practical preflight checks to four migration/installation docs, capturing blockers discovered during real validation runs against open-source Rails apps (
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Start: Existing Rails App] --> B{bundle install passes?}
B -- No: native gem failure --> C[Fix pg / nio4r / mysql2 / msgpack first]
C --> B
B -- Yes --> D{packageManager in package.json?}
D -- No & uses Yarn --> E[Add packageManager field]
E --> D
D -- Yes or not Yarn --> F{App boots cleanly?}
F -- No: missing DB config or env vars --> G[Fix config/database.yml and env vars]
G --> F
F -- Yes --> H{Asset stack?}
H -- Vite-based react-rails --> I[Follow vite_rails migration guide]
H -- Shakapacker / Webpacker --> J[Follow react-rails or upgrade guide]
H -- New install --> K[Follow existing-app installation guide]
I --> L[rails generate react_on_rails:install]
J --> L
K --> L
|
Review: Documentation-only, well-motivated additionsThe preflight additions are grounded in real validation attempts (the PR description lists exact apps and failure modes), which makes the guidance credible and actionable. Overall this is a solid improvement. A few things worth addressing before merging: Hardcoded yarn@1.22.22 across all four docs yarn@1.22.22 is Yarn Classic (v1.x), which is in maintenance mode. All four changed files repeat this exact version string without qualification. Two concerns: (1) Apps using Yarn Berry (v2/3/4) need a different value entirely. (2) The exact patch version will drift as newer 1.x patches are released. Suggest clarifying that this is an example for Yarn Classic and that users should substitute the version returned by yarn --version on their machine. npm pkg set in the Vite migration guide Using npm to declare Yarn as the package manager is technically correct but likely to confuse users in a Yarn-based workflow, and silently assumes npm is available. A one-line clarification that this command only edits package.json would help. See inline comment on line 54 of migrating-from-vite-rails.md. Inconsistency between guides on how to set packageManager migrating-from-react-rails.md step 3 asks users to add the field manually with no command, while migrating-from-vite-rails.md provides the npm pkg set command. A consistent approach across all guides would be easier to follow. See inline comment on line 32 of migrating-from-react-rails.md. Minor nit (pre-existing) The Bundler version 2.3.26 is pinned in the bundle lock --update example in migrating-from-react-rails.md. Given the PR touches that section, noting that users should use their own modern Bundler version would be a small improvement. |
Review: Tighten migration preflights from validation runsGood documentation improvements grounded in real-world validation attempts. The additions are factually correct and the native gem / app-boot / packageManager preflight items will genuinely help users avoid confusing failures that have nothing to do with React on Rails itself. Duplication of packageManager instructions across 4 files: The Yarn Classic command now appears verbatim in all four files. If the recommended version changes, four places need updating. Consider extracting this to a shared doc and linking to it from each file. Rails age buried at preflight item 6 in the react-rails guide: The original item 3 was Rails age (5.2+ requirement), now moved to item 6. Rails version is a hard blocker with no workaround short of a Rails upgrade, while packageManager metadata only matters for Shakapacker 9 users. Item 3 or 4 better reflects its importance. Awkward paragraph placement in migrating-from-react-rails.md: The native gem compile error note appears between the Bundler lockfile code block and the numbered migration steps as hanging prose. It belongs either in the preflight section above or merged into the numbered flow below. Closing paragraph in migrating-from-vite-rails.md repeats preflight advice: The final paragraph largely restates what is already in the Preflight section at the top of the same file. Consider removing it or replacing it with a brief cross-reference to avoid future divergence. |
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, or `mysql2` versions that fail to compile on current macOS and Ruby before the migration even starts. | ||
| 4. **App boot readiness**: the React on Rails install generator boots the full Rails app. Make sure `config/database.yml` exists and required env vars for initializers are set. | ||
| 5. **Package manager metadata**: if the repo has a `yarn.lock` but no `"packageManager"` field in `package.json`, add one before introducing Shakapacker 9. Example for Yarn Classic: `npm pkg set packageManager="yarn@1.22.22"` (or add the field manually). Use the version that matches your project's Yarn installation. | ||
| 6. **Rails age**: current `react_on_rails` requires Rails 5.2+. Rails 5.1 / Webpacker 3 apps are usually a staged migration, not a one-command migration. |
There was a problem hiding this comment.
The original item 3 was Rails age (a hard, no-workaround blocker). It is now item 6, after the Shakapacker 9–specific packageManager metadata item. Users on Rails 5.1 will hit item 6 regardless of packageManager status, but not vice versa. Consider swapping items 5 and 6 to keep the hard Rails version constraint closer to the top of the list.
| bundle _2.3.26_ install | ||
| ``` | ||
|
|
||
| If the first failure is a native gem compile error, solve that before swapping to React on Rails. Typical examples are `nio4r` in older Rails/Webpacker apps and `pg` or `mysql2` in apps with older database adapters pinned in the lockfile. |
There was a problem hiding this comment.
This paragraph is floating between the Bundler lockfile code block and the numbered migration steps. It is easy to miss and creates a break in the visual flow. It would read better if it were folded into the Preflight section above (as a note after item 3) or appended to the Bundler note in item 2 of the steps below.
|
|
||
| The migration is mostly about asset/build integration, mounting strategy, and optional SSR capability. | ||
|
|
||
| One practical detail from real app migrations: if the generator fails while booting your app, treat that as an application preflight problem first, not a React on Rails problem. Missing `APP_URL`-style env vars or an absent `config/database.yml` can stop the migration before any React on Rails files are generated. |
There was a problem hiding this comment.
This paragraph restates advice already given in the Preflight section (lines 23–24). Having two copies creates a risk they drift apart on future edits. Consider removing this and replacing with a brief back-reference like "See the Preflight section above if the generator fails during app boot." or drop it entirely since the Preflight section already covers it.
| - Rails 7+ is recommended. Rails 5.2+ can work, but Webpacker-era apps usually need an incremental upgrade first. | ||
| - If your app still uses `webpacker`, expect this to be a two-step migration: move to `shakapacker`, then install React on Rails. | ||
| - If `bundle install` is already failing on older native gems such as `pg`, `nio4r`, `mysql2`, or `msgpack`, fix that baseline first. React on Rails installation cannot get past a broken app bundle. | ||
| - If your repo has a `yarn.lock` but no `"packageManager"` field in `package.json`, add one before introducing Shakapacker 9. Example for Yarn Classic: `npm pkg set packageManager="yarn@1.22.22"` (or add the field manually). Use the version that matches your project's Yarn installation. |
There was a problem hiding this comment.
The hardcoded yarn@1.22.22 appears verbatim in all four changed files. A user running a different Yarn 1.x patch (e.g. 1.22.19) who copies the command will silently set the wrong version in package.json. A small addition helps: suggest they confirm their installed version first with yarn --version before copying the command. The same note applies to the equivalent lines in the other three files.
justin808
force-pushed
the
jg-codex/migration-docs-validation-phase2
branch
from
7caa04f to
47d5442
Compare
justin808
force-pushed
the
jg-codex/evaluator-docs-phase1
branch
from
a7a1cd3 to
ef48b6a
Compare
Review: Tighten migration preflights from validation runsDocumentation-only change. The additions are well-motivated — grounding preflight warnings in real validation attempts (native gem failures, missing Inconsistency: The other three files all list Closing paragraph in Lines 201–201 repeat the generator-boot failure warning that already appears in the Preflight section at the top of the same file. The duplication will diverge over time. Either remove the closing paragraph or make it a brief cross-reference (e.g., "See Preflight above if the generator fails to boot your app."). Hanging prose in The native gem compile error note ("If the first failure is a native gem compile error...") sits as unattached prose between the Bundler lockfile snippet and the numbered migration steps. It reads as an afterthought. Moving it into the Preflight section (where the other native gem guidance already lives) or collapsing it into preflight item 3 would tighten the flow. Rails age moved from item 3 to item 6 in the react-rails migration guide The original ordering put Rails version (hard blocker, affects all users) at item 3. The new items inserted above it are more narrowly applicable (Shakapacker 9 |
| 1. **Webpacker vs Shakapacker**: if the app still uses `webpacker`, migrate to `shakapacker` first. | ||
| 2. **Bundler age**: some older `react-rails` apps still carry Bundler 1.x lockfiles. Those can fail on modern Ruby before you even reach the migration work. | ||
| 3. **Rails age**: current `react_on_rails` requires Rails 5.2+. Rails 5.1 / Webpacker 3 apps are usually a staged migration, not a one-command migration. | ||
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, or `mysql2` versions that fail to compile on current macOS and Ruby before the migration even starts. |
There was a problem hiding this comment.
Inconsistency with the other three docs: msgpack is listed in the native gem preflight for installation-into-an-existing-rails-app.md, migrating-from-vite-rails.md, and upgrading-react-on-rails.md, but is missing here.
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, or `mysql2` versions that fail to compile on current macOS and Ruby before the migration even starts. | |
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, `mysql2`, or `msgpack` versions that fail to compile on current macOS and Ruby before the migration even starts. |
| bundle _2.3.26_ install | ||
| ``` | ||
|
|
||
| If the first failure is a native gem compile error, solve that before swapping to React on Rails. Typical examples are `nio4r` in older Rails/Webpacker apps and `pg` or `mysql2` in apps with older database adapters pinned in the lockfile. |
There was a problem hiding this comment.
This paragraph ("If the first failure is a native gem compile error...") is hanging prose between the Bundler code block and the numbered migration steps, making the flow hard to scan. The content already belongs in preflight item 3 above. Consider either removing this paragraph (the preflight already covers it) or merging it into item 3 as a clarifying sentence, rather than leaving it as free-floating text in the migration steps section.
|
|
||
| The migration is mostly about asset/build integration, mounting strategy, and optional SSR capability. | ||
|
|
||
| One practical detail from real app migrations: if the generator fails while booting your app, treat that as an application preflight problem first, not a React on Rails problem. Missing `APP_URL`-style env vars or an absent `config/database.yml` can stop the migration before any React on Rails files are generated. |
There was a problem hiding this comment.
This closing paragraph largely restates the Preflight section at the top of the same file ("Make sure config/database.yml exists and any required env vars for initializers are set before you run it."). The duplication will diverge over time as the two sections get edited independently. Consider removing this paragraph or replacing it with a brief cross-reference like: "If the generator fails while booting your app, revisit the Preflight checklist above — missing env vars or an absent config/database.yml are the most common causes."
| 1. **Webpacker vs Shakapacker**: if the app still uses `webpacker`, migrate to `shakapacker` first. | ||
| 2. **Bundler age**: some older `react-rails` apps still carry Bundler 1.x lockfiles. Those can fail on modern Ruby before you even reach the migration work. | ||
| 3. **Rails age**: current `react_on_rails` requires Rails 5.2+. Rails 5.1 / Webpacker 3 apps are usually a staged migration, not a one-command migration. | ||
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, or `mysql2` versions that fail to compile on current macOS and Ruby before the migration even starts. |
There was a problem hiding this comment.
Minor inconsistency: msgpack is listed in the equivalent preflight bullets in installation-into-an-existing-rails-app.md, migrating-from-vite-rails.md, and upgrading-react-on-rails.md, but is missing here.
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, or `mysql2` versions that fail to compile on current macOS and Ruby before the migration even starts. | |
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, `mysql2`, or `msgpack` versions that fail to compile on current macOS and Ruby before the migration even starts. |
|
|
||
| If the first failure is a native gem compile error, solve that before swapping to React on Rails. Typical examples are `nio4r` in older Rails/Webpacker apps and `pg` or `mysql2` in apps with older database adapters pinned in the lockfile. | ||
|
|
||
| 1. Update Deps |
There was a problem hiding this comment.
The numbered migration steps begin here with no section header separating them from the preflight content above (Bundler refresh code block + native gem paragraph). Readers might not realize the procedural steps have started. Consider adding ## Migration Steps before this list for clarity.
| 2. Remove `react_ujs` from `package.json`. | ||
| 3. Run `bundle install` and your package manager's install command. | ||
| 4. Commit changes. | ||
| 3. If the repo uses Yarn and `package.json` does not already declare it, add the package manager field before booting the app with Shakapacker 9: `npm pkg set packageManager="yarn@1.22.22"` (or add it manually to `package.json`). The example is for Yarn Classic; use the version that matches your project. |
There was a problem hiding this comment.
The packageManager instruction here is buried in a long inline list item and lacks a code block, making the command harder to read and copy. The dedicated section added to migrating-from-vite-rails.md (step 2) handles this much better. Consider either pulling it out into a sub-step with a fenced code block, or at minimum wrapping the command in backtick-fenced inline code as a shell snippet.
Review: Documentation-only PR — good quality, a few minor consistency notesOverall this is a well-grounded docs improvement. The preflight additions are accurate, grounded in real validation failures, and the vite_rails guide restructuring (numbered steps 1–9) is clean. No code changes, no security or performance concerns. Issues found1. 2. Missing section header before migration steps (inline comment on line 29) 3. Minor nitThe blank line removed from |
justin808
force-pushed
the
jg-codex/evaluator-docs-phase1
branch
from
ef48b6a to
33c2fa5
Compare
justin808
force-pushed
the
jg-codex/migration-docs-validation-phase2
branch
from
47d5442 to
cbe2d51
Compare
Review: Documentation-only preflight tighteningGood practical improvements. The three preflight categories (native gem failures, app boot readiness, package manager metadata) are grounded in real validation attempts and consistently applied across all four guides. The Vite routing note at the top of the react-rails guide is a useful redirect. A few issues worth addressing: 1. msgpack missing from react-rails migration guide Three of the four updated docs list pg, nio4r, mysql2, and msgpack. The migrating-from-react-rails.md guide omits msgpack in both the preflight checklist item 3 and in the inline prose describing native gem failures. A user migrating a react-rails app that uses msgpack would miss the warning. 2. npm pkg set requires npm in Yarn-only environments The suggested command requires npm to be available. In projects that use Yarn exclusively (especially older Yarn Classic setups on CI), npm may not be installed. The manual alternative is mentioned parenthetically but presented as secondary. Consider flipping the primary recommendation to add the field manually and offering npm pkg set as a convenience option. 3. Users may not know their Yarn version Saying use the version that matches your project Yarn installation is good advice, but many users won't know offhand which version they have. Adding yarn --version as a hint would make this immediately actionable. 4. Duplicated content across four files The packageManager guidance including the identical npm pkg set example appears in all four docs. This works today but creates a maintenance burden if the recommended approach changes. Consider centralizing this in a shared troubleshooting doc and cross-linking. None of these block merging, but items 1 and 3 are quick fixes. |
| 1. **Webpacker vs Shakapacker**: if the app still uses `webpacker`, migrate to `shakapacker` first. | ||
| 2. **Bundler age**: some older `react-rails` apps still carry Bundler 1.x lockfiles. Those can fail on modern Ruby before you even reach the migration work. | ||
| 3. **Rails age**: current `react_on_rails` requires Rails 5.2+. Rails 5.1 / Webpacker 3 apps are usually a staged migration, not a one-command migration. | ||
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, or `mysql2` versions that fail to compile on current macOS and Ruby before the migration even starts. |
There was a problem hiding this comment.
The other three updated docs all list msgpack alongside pg, nio4r, and mysql2, but this guide omits it. Should be consistent.
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, or `mysql2` versions that fail to compile on current macOS and Ruby before the migration even starts. | |
| 3. **Native gem age**: older lockfiles often pin `nio4r`, `pg`, `mysql2`, or `msgpack` versions that fail to compile on current macOS and Ruby before the migration even starts. |
| bundle _2.3.26_ install | ||
| ``` | ||
|
|
||
| If the first failure is a native gem compile error, solve that before swapping to React on Rails. Typical examples are `nio4r` in older Rails/Webpacker apps and `pg` or `mysql2` in apps with older database adapters pinned in the lockfile. |
There was a problem hiding this comment.
Same inconsistency — the inline prose also omits msgpack while the other guides include it.
| If the first failure is a native gem compile error, solve that before swapping to React on Rails. Typical examples are `nio4r` in older Rails/Webpacker apps and `pg` or `mysql2` in apps with older database adapters pinned in the lockfile. | |
| If the first failure is a native gem compile error, solve that before swapping to React on Rails. Typical examples are `nio4r` in older Rails/Webpacker apps, `pg` or `mysql2` in apps with older database adapters pinned in the lockfile, and `msgpack` in apps with older serialization dependencies. |
| If you use Yarn and `package.json` does not already declare it, set the package manager before running the generator. This only updates `package.json`; it does not install or switch Yarn for you. | ||
|
|
||
| ```bash | ||
| npm pkg set packageManager="yarn@1.22.22" |
There was a problem hiding this comment.
npm pkg set requires npm to be available. In a project that uses only Yarn Classic, npm may not be installed. Consider leading with the manual edit and offering this as a shortcut, e.g.:
Add or update the
"packageManager"field inpackage.jsonmanually, for example:"packageManager": "yarn@1.22.22"Or as a convenience if npm is available:
npm pkg set packageManager="yarn@1.22.22"
Also worth noting: users can discover their version with yarn --version rather than having to know it in advance.
|
Review summary posted via inline comments below. |
| If you use Yarn and `package.json` does not already declare it, set the package manager before running the generator. This only updates `package.json`; it does not install or switch Yarn for you. | ||
|
|
||
| ```bash | ||
| npm pkg set packageManager="yarn@1.22.22" |
There was a problem hiding this comment.
Yarn Berry (2+) note needed: This command works for Yarn Classic (1.x). For Yarn 2+ (Berry), the packageManager field requires a corepack hash suffix (e.g., yarn@3.6.0+sha224.abc123...). A simple npm pkg set won't produce the correct value. Consider noting: "If you use Yarn 2+, run yarn --version and check whether corepack has already set this field."
Fix two doctor issues affecting Pro/RSC apps: 1. **Prerender consistency check (#2770):** `uses_prerender_in_views?` now detects `stream_react_component`, `cached_stream_react_component`, and `rsc_payload_react_component` as implicit prerender signals. Previously the doctor recommended removing `server_bundle_js_file` for RSC apps, which crashes them. 2. **Server rendering engine display:** `check_server_rendering_engine` now regex-parses the Pro initializer to detect NodeRenderer. Shows correct engine info for Pro+NodeRenderer, Pro+ExecJS, and base apps. Warns when NodeRenderer is configured but ExecJS fallback is unavailable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Summary Recent commits (15f6cfc, 078a8b0, 97f4625) updated `update_changelog.rake` and `update-changelog.md` with significant improvements (orphaned prerelease link cleanup, v prefix fix for compare links, consolidate/deduplicate blocks, explicit version support, auto-commit/push/PR). However `releasing.md` was not updated to match, leaving stale instructions. - **releasing.md**: Update step 1 to reflect `/update-changelog` now supports explicit versions, collapses prereleases with dedup, and auto-commits/pushes/opens a PR (removes manual "commit and push CHANGELOG.md" step) - **releasing.md**: Add note about `v` prefix requirement in CHANGELOG compare links (per #2628 fix) - **releasing.md**: Update Option A post-release instructions to match new auto-PR behavior - **update-changelog.md**: Fix stale description saying version computed from "changelog headings and git tags" -- prerelease index is now determined solely from git tags No changes to `release.rake` -- it was not affected by the recent changes. ## Test plan - [ ] Verify `releasing.md` step 1 accurately describes current `/update-changelog` behavior - [ ] Verify compare link `v` prefix note matches actual `update_changelog_links()` behavior - [ ] Verify `update-changelog.md` description matches `compute_auto_version()` logic 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only updates that align release/changelog instructions with current automation behavior; no runtime code paths or release tooling logic changed. > > **Overview** > Aligns release and changelog docs with the updated `/update-changelog` automation: explicit version support, skipping auto-versioning when provided, prerelease section collapse/dedup, and auto commit/push/PR creation. > > Clarifies versioning/link rules (prerelease index derived from git tags only; CHANGELOG compare links must use `v`-prefixed tags), updates instructions from `master` to `main`, and refreshes post-release catch-up steps to use the new auto-PR flow. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a48394a. 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** * Added support for explicit prerelease/versions (e.g., 16.5.0.rc.10); auto-version computation is skipped when explicit version provided * Clarified prerelease changelog handling: collapse prior prerelease sections, deduplicate entries, and derive prerelease index from git tags * Documented automated workflow: command can automatically commit, push, and open a PR (now requires working on main) * Require changelog compare links to use v-prefixed tag names and compare against main <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…pages (#2785) - Add RSC landing page (`docs/pro/react-server-components/index.md`) — top-level introduction to React Server Components support, linking to tutorial, migration guide, and deep dives - Add ExecJS Limitations page (`docs/oss/core-concepts/execjs-limitations.md`) — documents timer/async constraints, common errors, workarounds, and migration path to Node renderer - Add Debugging Guide (`docs/oss/building-features/debugging.md`) — SSR debugging workflows, hydration mismatch diagnosis, log locations, and configuration tips - Add Performance Benchmarks page (`docs/oss/core-concepts/performance-benchmarks.md`) — ExecJS vs Node renderer comparison, bundle splitting impact, streaming SSR benefits, and real-world results - Add cross-references from existing docs (client-vs-server-rendering, react-server-rendering, troubleshooting, oss-vs-pro) to the new pages Closes #2781 - [ ] Verify all internal links resolve correctly - [ ] Confirm new pages follow existing doc formatting conventions - [ ] CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: documentation-only changes plus minor wording/link updates; main risk is broken or incorrect doc links/claims. > > **Overview** > Adds new documentation pages for **debugging SSR/hydration issues** (including recommended `ReactOnRails.configure` flags), **ExecJS SSR limitations** (timers/async/browser APIs and workarounds), and **SSR performance benchmarks** comparing ExecJS vs the Pro Node renderer. > > Introduces a Pro **React Server Components** landing page that links into tutorials/migration/deep-dives, and updates several existing docs to cross-link to the new pages, fix a component-registration console snippet (`registeredComponents()`), and standardize Node renderer performance claims to *3–10x* with a benchmark reference. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit cb657d0. 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 --> * **Documentation** * Added a comprehensive debugging guide for SSR, hydration mismatches, log inspection, and debug flags for enhanced server-side diagnostics. * Documented ExecJS runtime limitations (timers, async, browser globals, pool-size caveats) and Node renderer migration guidance. * Added performance benchmarks comparing rendering approaches, streaming SSR, and bundle impacts. * Introduced React Server Components docs for Pro with streaming, integration, and migration guidance. * Minor cross-reference and troubleshooting updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…port (#2807) ## Summary - Updates the Version Stamping section header in `.claude/commands/update-changelog.md` to mention explicit version support (e.g., `16.5.0.rc.10`), aligning it with Step 4 and the "Before a release" summary which already documented this correctly. Fixes #2806 ## Test plan - [x] Verified the updated line matches the suggested fix in the issue 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk documentation-only change clarifying that the changelog stamping rake task can be invoked with an explicit version string. No runtime or release logic was modified. > > **Overview** > Updates `.claude/commands/update-changelog.md` to explicitly document that version stamping via the rake task applies not only to `release`/`rc`/`beta` modes but also when an explicit version (e.g., `16.5.0.rc.10`) is provided. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 283793b. 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** * Updated guidance on when to use the version-stamping rake task, now including scenarios with explicit version arguments in addition to release modes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tterns in RSC docs (#2803) Fix fictional API names, missing associations, form_with, and CSRF patterns in RSC docs Key changes: - Replace fictional stream_react_component_with_async_props and getReactOnRailsAsyncProp with real stream_react_component API - Add as_json(include: [:specs, :reviews]) for nested associations accessed by React components - Add as_json(only: [...]) whitelists for user serialization - Replace CSRF token props with ReactOnRails.authenticityToken() in client components - Add local: true to form_with for Rails 5.1-6.0 compatibility - Remove redundant prerender: true from stream_react_component examples - Fix stale "with Async Props" heading Follow-up issue #2813 tracks streaming narrative clarity improvements. Fixes #2802 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Summary Fixes #2804 Audits and corrects ExecJS technical claims across four docs pages added in #2785: - **Runtime auto-detection**: Corrected "By default, ExecJS uses the Node.js runtime" — ExecJS actually auto-detects via `best_available`, preferring mini_racer and Bun over Node.js - **Node.js process model**: Changed "Persistent subprocess via IPC" to "New process per eval call" — ExecJS's `ExternalRuntime` spawns a fresh Node.js process per evaluation via `IO.popen` - **Timer polyfill behavior**: Corrected debugging page claim that users see `setTimeout is not defined` errors — React on Rails polyfills timers as silent no-ops (or console.error with `trace: true`) - **SSR timing logging**: Removed incorrect claim that `logging_on_server` logs render durations — it actually replays console messages to `Rails.logger` - **pool_size explanation**: Improved accuracy of why `pool_size=1` is required on MRI, citing the actual threading/deadlock cause and linking to #1438 ## Test plan - [ ] Verify all four docs pages render correctly in the documentation site - [ ] Confirm no broken markdown links - [ ] Cross-reference claims against ExecJS source code and React on Rails `RubyEmbeddedJavaScript` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Docs-only changes; risk is limited to potentially confusing readers if any updated ExecJS/runtime claims are still inaccurate. > > **Overview** > Updates OSS docs to correct several ExecJS-related technical claims across debugging, rendering, limitations, and performance pages. > > Clarifies that ExecJS *auto-selects* the best available runtime (override via `EXECJS_RUNTIME`), explains that timer APIs are stubbed as no-ops during SSR (with `clearInterval` runtime-dependent) and how `trace`/`replay_console`/`logging_on_server` surface warnings, fixes the Node.js runtime process model description, and refines guidance on SSR timing measurement and why MRI requires `server_renderer_pool_size = 1` (deadlock explanation + issue link). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5296d8b. 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** * Clarified ExecJS runtime auto-detection: it now prefers the best available runtime (may choose mini_racer or Bun over Node) and can be overridden via an environment variable. * Clarified SSR timer behavior: common timer functions are stubbed as no-ops, some clear operations may vary by runtime, and trace/logging settings emit warnings that can be replayed to browser devtools and Rails logs. * Updated guidance on renderer/process behavior, server render timing measurement, and server-render pool-size considerations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…#2808) ## Summary - Adds quick-action menu (`f`, `f+i`, `d`, `r`, `m`) after triage to support a "fix what matters, merge, follow-up the rest" pattern - Adds range syntax (`N-M`) for selecting items (e.g., `r7-9`, `1,3-5`) - Adds follow-up GitHub issue creation to bundle deferred discuss/skipped items - Adds merge-ready signaling after completing actions - Updates both `.claude/commands/address-review.md` and `.agents/workflows/address-review.md` Closes #2805 ## Test plan - [ ] Run `/address-review` on a PR with mixed must-fix, discuss, and skipped items - [ ] Verify quick-action menu appears after triage - [ ] Test `f` action: fixes must-fix, reply-skips rest, signals merge-ready - [ ] Test `f+i` action: fixes must-fix, creates follow-up issue, signals merge-ready - [ ] Test `r` with range syntax (e.g., `r3-5`) - [ ] Test `m` action: creates follow-up issue for everything - [ ] Test direct item selection with ranges (e.g., `1,3-5`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only workflow changes that adjust expected `/address-review` behavior (including follow-up issue creation and thread resolution). Low implementation risk, but reviewers should verify the new instructions align with desired operational safeguards (push/resolve semantics). > > **Overview** > Updates the `/address-review` workflow docs to add a **post-triage quick-action menu** (`f`, `f+i`, `d`, `r`, `m`) and specify per-action behavior for fixing, discussing, replying, and deferring review feedback. > > Adds **range-based item selection** (e.g., `3-5`) with validation rules, introduces a **follow-up GitHub issue** creation template for deferred items, and clarifies **merge-ready signaling**, `git push` confirmation requirements, and inclusion of `html_url` in fetched comment payloads for easier linking. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0834a5f. 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** * Redesigned address-review workflow with a required quick-action menu (f, f+i, d, r, m) and explicit per-action behaviors. * Added support for direct item selection, range syntax (N-M) and presets (e.g., "all must-fix") for bulk operations. * Include fetched comment links in review outputs and require git-push confirmation before pushing. * Clarified follow-up issue creation rules, merge-readiness signaling, and thread-resolution/rationale handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add Wave 3 execution plan for 19 long-tail backlog issues that had no open PR at snapshot time. - Stacked on top of #2811 to preserve mergeable review slices. - Not run (documentation/analysis only). - Refs #2527 - Refs #2524 - Refs #1999 - Refs #1985 - Refs #1959 - Refs #1958 - Refs #1949 - Refs #1929 - Refs #1862 - Refs #1828 - Refs #1746 - Refs #1692 - Refs #1658 - Refs #1627 - Refs #1590 - Refs #1583 - Refs #1468 - Refs #1465 - Refs #1196 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk because it only adds an analysis/markdown planning document and does not change runtime, build, or CI behavior. > > **Overview** > Adds `analysis/open-issue-deep-dive/wave-3-long-tail.md`, documenting the Wave 3 execution plan for **19 long-tail issues** from the 2026-03-22 triage snapshot. > > The doc includes a domain breakdown, notes on issues resolved as duplicates after the snapshot, and per-issue triage context plus the *exact* follow-up question posted to each issue. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 75c95ed. 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 --> * **Documentation** * Added Wave 3 Long-Tail Issues report documenting open issues without active pull requests, including domain analysis, issue metadata, labels, creation dates, and triage context. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
- Fix latent bug in git_utils.rb: guard git_installed before running backtick command to avoid Errno::ENOENT when git is not installed - Extract shared ci_environment? and clean_worktree? helpers to remove duplication between uncommitted_changes? and warn_if_uncommitted_changes - Fix shell injection risk in cli_exists? by using multi-arg system() instead of string interpolation - Update stale comment in install_generator.rb to reflect non-blocking warn_if_uncommitted_changes behavior - Add Rails version comment to STOCK_RAILS_BIN_DEV constant - Use regex with global flag instead of string replace in create-app.ts to patch all occurrences of npm install in bin/setup - Remove misleading env: undefined from spawnSync options in utils.ts - Add HelloWorld component file assertion for Rspack smoke test - Add changelog note to RC version examples in upgrading docs - Minor wording improvements in docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use Open3.capture2e instead of backtick for git status to properly detect command failures instead of silently treating them as clean - Remove COVERAGE env var from CI check since it's set by SimpleCov during local development, not a CI signal - Add warning log when bin/setup pnpm replacement finds no match - Document why --ignore-warnings is passed in create-app flows - Add Rails source reference comment to STOCK_RAILS_BIN_DEV constant and log when bin/dev exists but doesn't match stock template - Update upgrade docs from RC version to stable 16.4.0 - Add RSC+Rspack template output assertions in smoke test - Update git_utils specs to stub Open3 instead of backtick Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix TS/JS mismatch: update file references to .tsx to match --typescript default - Use bin/rails (Rails binstub convention) instead of bundle exec rails - Fix react-rails URL to canonical reactjs/react-rails (shakacode fork is archived) - Consolidate two comparison doc links in introduction.md with clear distinction - Add cross-link to detailed feature matrix in comparing doc's Next Steps - Remove duplicate PostgreSQL entry in create-react-on-rails-app.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Swap npm/pnpm and PostgreSQL in prerequisites so "first four" correctly refers to CLI-validated items (Node.js, Ruby, Rails, npm/pnpm) - Remove dedicated comparison guide section from introduction.md to reduce triple-linking; guide remains discoverable via inline prose and use-case table Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nd package README Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
justin808
force-pushed
the
jg-codex/migration-docs-validation-phase2
branch
from
73aa0c6 to
79598e6
Compare
PR ReviewThis PR mixes CI/workflow hardening, documentation restructuring, and meaningful Ruby code changes across 183 files. The doc/link updates look reasonable overall, but there are several issues worth addressing before merge. Bug: Dependabot guard removed from Pro workflows
Security: Removing workflow-level These blocks provided a least-privilege default for all jobs in each workflow. Without them, jobs that don't declare their own Regression: Regression: Breaking change:
User-facing URLs ending in Error messages in Version downgrade: This looks like a rebase artifact. Publishing |
![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.
There are 3 total unresolved issues (including 1 from previous review).
Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 79598e6. Configure here.
| - All tasks should start with status: `"pending"` | ||
|
|
||
| ## Step 7: Present Triage and Quick-Action Menu | ||
| ## Step 6: Present Triage and Quick-Action Menu |
There was a problem hiding this comment.
Duplicate Step 6 heading in address-review command
Medium Severity
The step renumbering introduced a collision: both "Create Todo List" (line 139) and "Present Triage and Quick-Action Menu" (line 149) are labeled ## Step 6. The old Step 7 was decremented to Step 6 but the existing Step 6 was left unchanged. This also makes downstream step references (Steps 7–9) off by one from their intended targets.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 79598e6. Configure here.
| @@ -1,8 +1,5 @@ | |||
| name: Lint GitHub Actions | |||
|
|
|||
| permissions: | |||
There was a problem hiding this comment.
Top-level permissions removed from multiple workflow files
Medium Severity
The top-level permissions: contents: read block was removed from multiple workflow files. Workflows like actionlint.yml and check-markdown-links.yml have no job-level permissions at all, so they now inherit the repository's default token permissions instead of being explicitly restricted to read-only access. This violates the principle of least privilege for the GITHUB_TOKEN.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 79598e6. Configure here.
| @@ -36,9 +27,6 @@ defaults: | |||
|
|
|||
There was a problem hiding this comment.
The if: github.event_name != 'pull_request' || github.actor != 'dependabot[bot]' guard was removed here, but the original comment explained the reason: "Skip Dependabot PRs — they can't access the Pro license secret."
Without this guard, every Dependabot dependency-update PR will trigger the Pro integration tests, which require a PRO_LICENSE_KEY secret that Dependabot cannot access. This will cause guaranteed CI failures on all Dependabot PRs.
The same issue applies to pro-lint.yml and pro-test-package-and-gem.yml which had identical guards removed.
| return | ||
| end | ||
|
|
||
| server_bundle_path = determine_server_bundle_path |
There was a problem hiding this comment.
Removing the early-return guard for blank server_bundle_js_file is a regression. Apps that deliberately set config.server_bundle_js_file = "" (or leave it blank) to disable SSR will now receive a spurious ⚠️ Server bundle not found warning every time rails doctor runs.
The previous behavior correctly detected this case and emitted an informational message instead of a warning. Suggest restoring:
| server_bundle_path = determine_server_bundle_path | |
| def check_javascript_bundles | |
| if server_bundle_filename.to_s.strip.empty? | |
| checker.add_info("ℹ️ server_bundle_js_file is blank (SSR disabled), skipping SSR bundle existence check") | |
| return | |
| end | |
| server_bundle_path = determine_server_bundle_path |
| if uses_node_renderer | ||
| checker.add_info(" Pro uses NodeRenderer for server rendering") | ||
| if defined?(ExecJS) && ExecJS.runtime | ||
| checker.add_info(" ExecJS available as fallback: #{ExecJS.runtime.name}") |
There was a problem hiding this comment.
This collapses two previously distinct cases into one else branch:
- Before: if ExecJS unavailable and fallback enabled → warning; if ExecJS unavailable and fallback disabled → info
- After: if ExecJS unavailable (for any reason) → warning
Pro users who intentionally set renderer_use_fallback_exec_js = false will now see a spurious ⚠️ ExecJS fallback is enabled but ExecJS is not available warning even though they explicitly opted out of the fallback. Suggest restoring the pro_execjs_fallback_enabled? guard:
| checker.add_info(" ExecJS available as fallback: #{ExecJS.runtime.name}") | |
| if defined?(ExecJS) && ExecJS.runtime | |
| checker.add_info(" ExecJS available as fallback: #{ExecJS.runtime.name}") | |
| elsif pro_execjs_fallback_enabled? | |
| checker.add_warning(" ⚠️ ExecJS fallback is enabled but ExecJS is not available") | |
| checker.add_info(" 💡 Install mini_racer or set renderer_use_fallback_exec_js = false") | |
| else | |
| checker.add_info(" ℹ️ ExecJS fallback is disabled (renderer_use_fallback_exec_js = false)") | |
| end |
| return true if ALL_DEFAULT_CONFIG_CANDIDATES.any? { |path| File.file?(path) } | ||
|
|
||
| !resolved_webpack_config_path.nil? | ||
| File.exist?("config/webpack/webpack.config.js") || |
There was a problem hiding this comment.
The original code used File.file? which correctly returns false for directories. This replacement uses File.exist? which returns true for a directory with that name. While a directory named webpack.config.js is unlikely, the semantics are slightly different. Prefer File.file? for correctness:
| File.exist?("config/webpack/webpack.config.js") || | |
| File.file?("config/webpack/webpack.config.js") || | |
| File.file?("config/webpack/webpack.config.ts") || | |
| File.file?("config/rspack/rspack.config.js") || | |
| File.file?("config/rspack/rspack.config.ts") |
There was a problem hiding this comment.
💡 Codex Review
Restore missing route constants used by post-install messaging
The route constants were removed earlier in this file, but add_post_install_message still references HELLO_SERVER_ROUTE/HELLO_WORLD_ROUTE. Any successful generator run that reaches this branch will raise NameError before completion messaging, turning a normal install into a runtime failure.
ℹ️ 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".
| defaultConfig, | ||
| '<NOT PROVIDED AT MODULE LOAD>', | ||
| ), | ||
| 'Default settings': defaultConfig, |
There was a problem hiding this comment.
logSanitizedConfig now logs defaultConfig directly, and defaultConfig.password is sourced from RENDERER_PASSWORD. In production-like deployments this emits the raw renderer secret to logs, which is a security regression from the previous sanitized output.
Useful? React with 👍 / 👎.
| .option('-t, --template <type>', 'javascript or typescript', 'typescript') | ||
| .option('-p, --package-manager <pm>', 'npm or pnpm (auto-detected if not specified)') | ||
| .option('--rspack', 'Use Rspack instead of Webpack (~20x faster builds)', false) | ||
| .option('--standard', 'Generate open-source React on Rails setup (skip prompt)') | ||
| .option('--pro', 'Generate React on Rails Pro setup (installs react_on_rails_pro)') | ||
| .option('--rsc', 'Generate React Server Components setup (installs react_on_rails_pro)') | ||
| .option('--rsc', 'Generate React Server Components setup (installs react_on_rails_pro)', false) |
There was a problem hiding this comment.
The CLI now registers only --rsc, so npx create-react-on-rails-app ... --pro is treated as an unknown option and exits early. This removes the documented Pro-without-RSC path and conflicts with the same command’s help text that still advertises --pro/mode selection.
Useful? React with 👍 / 👎.
Closing this PRAfter investigation, this PR should be closed: Almost all work is already merged. Of the 94 commits on this branch, ~66 are copies of PRs that were independently landed to main (#2567, #2571, #2588, #2592–#2812, etc.). The branch accumulated cherry-picks/rebases of work that was separately merged. All CI is broken by a single TypeScript syntax error ( The ~28 commits without PR references are rebase fixup noise ("Fix link checker errors from rebase", "Fix docs formatting after conflict merge"), duplicate commits, superseded version bumps, and minor doc edits. The only potentially unique code changes — migration preflight tightening, Shakapacker package manager preflights, bundler config detection, RSC layout renaming — would need to be redone against current main anyway due to 3+ weeks of drift. Nothing of value is lost by closing. I (Claude Code) performed this investigation and closure. |
3 participants
Summary
Validation
bundle installonlockstep/rails_new(failed onnio4r 2.5.8native build before migration)bundle installonifmeorg/ifme(failed onpg 1.1.4native build before upgrade)bundle installonantiwork/gumroad(failed onmysql2 0.5.6linker issue before upgrade)bundle installonbigbinary/wheeland confirmed generator boot then failed untilconfig/database.ymland env vars were presentbundle install,bundle add shakapacker --strict, and generator attempts onarish-me/rails-vite-wheel; observed thepackageManagerrequirement and app-initializer/env-var boot blockersrails-vite-wheelgenerator against the localreact_on_railsgem path and confirmed current code gets past the earlier package-version validation dead endNote
Medium Risk
Medium risk because it changes multiple GitHub Actions workflow triggers/permissions and substantially rewrites/reshuffles documentation, which could affect CI behavior and user-facing setup guidance if any details are incorrect.
Overview
Updates CI workflow configuration and extensively revises docs/agent guidance.
GitHub Actions workflows drop explicit
permissions: contents: read, adjustpush/paths-ignorebehavior to rely on docs-only detection, and tweak benchmark ignore lists.Docs are heavily edited:
AGENTS_USER_GUIDE.mdis rewritten into an AI-agent reference with prescriptive setup/troubleshooting guidance; many pages update Pro/marketing links topro.reactonrails.com/reactonrails.com/pro, adjust prerequisites/version notes, and revise Node Renderer and deployment guidance (including reintroducingcapistrano-deployment.mdandelastic-beanstalk.mdwhile simplifyingdocs/oss/deployment/README.md).Housekeeping includes updating lychee exclusions (skip
analysis/), minor formatting fixes inanalysis/*, and removing.Codex/napkin.mdfrom.gitignore.Reviewed by Cursor Bugbot for commit 79598e6. Bugbot is set up for automated code reviews on this repo. Configure here.