Skip to content

Add Pro/RSC setup checks to react_on_rails:doctor#2674

Merged
justin808 merged 10 commits intomainfrom
ihabadham/feature/rsc-doctor-checks
Mar 21, 2026
Merged

Add Pro/RSC setup checks to react_on_rails:doctor#2674
justin808 merged 10 commits intomainfrom
ihabadham/feature/rsc-doctor-checks

Conversation

@ihabadham
Copy link
Copy Markdown
Collaborator

@ihabadham ihabadham commented Mar 18, 2026

Summary

  • Extends react_on_rails:doctor with two new conditional diagnostic sections: React on Rails Pro Setup and React Server Components
  • Both sections are skipped entirely for base-only installs — zero noise for OSS users
  • Uses runtime config (ReactOnRailsPro.configuration) instead of regex parsing for reliable detection
  • Every error/warning includes actionable fix instructions

Pro Setup checks (3)

  • Pro initializer existence — warns if config/initializers/react_on_rails_pro.rb is missing
  • Renderer mode — reports NodeRenderer vs ExecJS, notes NodeRenderer is required for RSC
  • Base package import scanning — detects JS files importing from 'react-on-rails' instead of 'react-on-rails-pro'. The base package is a transitive dependency of Pro, so these imports resolve silently but load the base version without Pro features (verified via reproduction)

RSC checks (5, gated on enable_rsc_support)

  • Renderer mode — errors if RSC is enabled without NodeRenderer
  • RSC payload route — errors if rsc_payload_route is missing from routes (causes silent 404s)
  • RSC bundler config — errors if rscWebpackConfig.js is missing (checks both webpack and rspack paths)
  • React version — error for <19, warning for 19.0.0-19.0.3 (security vulns), warning for 19.1+ (unverified), success for 19.0.4+
  • Procfile RSC watcher — warns if RSC_BUNDLE_ONLY entry is missing (warning-only since custom process managers are valid)

Closes #2427

Test plan

  • 98 doctor specs pass (7 new test groups with 30 examples covering all checks)
  • RuboCop passes with zero offenses
  • CI passes
  • Manual: run rake react_on_rails:doctor on a base install — Pro/RSC sections should not appear
  • Manual: run on a Pro install — Pro section should appear with initializer/renderer/import checks

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Enhanced diagnostics for Pro and React Server Components (RSC): validates renderer mode, bundler config presence, payload route, Procfile watcher, React version compatibility, and flags incorrect base-package imports. Detects Rails environment load issues and reports clear warnings with guidance. Scans customizable JS source paths when checking imports.
  • Tests

    • Expanded test coverage for the new diagnostics, edge cases, and error/warning reporting.

ihabadham and others added 3 commits March 17, 2026 18:15
Extend the doctor diagnostic command with two new conditional sections:

- "React on Rails Pro Setup" — detects Pro gem, reports renderer mode
  (NodeRenderer vs ExecJS), provides guidance for RSC readiness.

- "React Server Components" — when RSC is enabled via Pro config,
  validates: renderer mode (NodeRenderer required), rsc_payload_route
  presence, RSC bundler config (webpack/rspack), react-on-rails-rsc
  npm package, React version compatibility (19.0.x required, 19.0.4+
  recommended), and Procfile RSC watcher entry.

Both sections are skipped entirely for base-only installs (no noise).
Uses runtime config (ReactOnRailsPro.configuration) instead of regex
parsing for reliable detection. Each check includes actionable fix
instructions.

Closes #2427

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two new checks in the Pro Setup section:

1. Pro initializer existence — warns if config/initializers/react_on_rails_pro.rb
   is missing, since Pro runs with all defaults without it.

2. Base package import scanning — detects JS files that import from
   'react-on-rails' instead of 'react-on-rails-pro'. The base package is a
   transitive dependency of Pro, so these imports resolve silently but load
   the base version without Pro features (streaming, caching, RSC).
   Reproduced: webpack compiles without errors, but components registered
   through the base import lack Pro capabilities.

Ref: #2427

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
react-on-rails-rsc is a peer dependency of react-on-rails-pro. When
missing, webpack fails with a clear "Cannot find module
'react-on-rails-rsc/WebpackPlugin'" error that points directly to the
cause. The doctor check added no diagnostic value over that error.

Verified via reproduction in a test app: the webpack error is loud,
specific, and includes the full require stack.

Ref: #2427

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds lazy Rails environment loading and new Pro and React Server Components (RSC) diagnostics to ReactOnRails::Doctor: initializer and renderer checks, JS/TS source import scanning, bundler config and route validation, React version detection, Procfile watcher checks, and extensive specs for these flows.

Changes

Cohort / File(s) Summary
Doctor implementation
react_on_rails/lib/react_on_rails/doctor.rb
Adds @rails_environment_loaded and ensure_rails_environment_loaded; appends check_pro_setup and check_rsc_setup; implements Pro initializer/renderer checks, JS/TS import scanning (Shakapacker source_path fallbacks), RSC bundler config and route checks, React version detection helpers, Procfile.dev watcher checks, and related constants/helpers with guarded error handling.
Doctor tests
react_on_rails/spec/lib/react_on_rails/doctor_spec.rb
Introduces extensive specs covering: environment loading, Pro presence/errors, base-package import scanning (including shakapacker.yml variations), RSC enablement paths, renderer validation, rsc_payload_route detection, bundler config presence (webpack/rspack candidates), React version compatibility (node_modules/package.json resolution), Procfile.dev watcher checks, and exception-wrapping behavior.
Test fixtures / temp artifacts
config/..., tmp/..., node_modules/..., config/webpack/*, config/rspack/*, Procfile.dev
New temporary files and fixtures used by specs (routes.rb, shakapacker.yml, package.json/node_modules/react, candidate bundler configs, Procfile.dev) to exercise detection and messaging logic.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as Doctor (rsc:doctor)
    participant Rails as Rails Env
    participant FS as File System
    participant Bundler as Bundler Configs
    participant Procfile as Procfile.dev
    participant Console as Console Output

    CLI->>Rails: ensure_rails_environment_loaded()
    Rails-->>CLI: loaded / failed (cached `@rails_environment_loaded`)
    CLI->>FS: resolve_js_source_path() and scan imports
    FS-->>CLI: report import findings
    CLI->>Bundler: check RSC bundler config paths
    Bundler-->>CLI: present / missing
    CLI->>FS: check_rsc_payload_route() in routes.rb
    FS-->>CLI: route present / missing
    CLI->>FS: detect_react_version_from_deps()
    FS-->>CLI: react version or compatibility warning
    CLI->>Procfile: inspect Procfile.dev for RSC watcher (RSC_BUNDLE_ONLY)
    Procfile-->>CLI: watcher present / missing
    CLI->>Console: emit aggregated success/info/warning/error messages
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

"I hopped through configs, files, and logs, 🐇
I sniffed for routes, bundles, and pro-blogs,
From package.json to Procfile's tune,
I found the gaps beneath the moon,
Fix them quick — then dance by noon! ✨"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add Pro/RSC setup checks to react_on_rails:doctor' accurately describes the main change: extending the doctor rake task with new Pro and RSC diagnostic sections.
Linked Issues check ✅ Passed The PR implementation addresses all coding requirements from issue #2427: Pro initializer validation, rsc_payload_route detection, bundler config verification, React version checks, and Procfile watcher configuration checks with actionable error/warning messages.
Out of Scope Changes check ✅ Passed All changes are scoped to doctor.rb and its spec file, implementing Pro/RSC diagnostic checks as specified. No unrelated functionality or file modifications were introduced.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ihabadham/feature/rsc-doctor-checks
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Mar 18, 2026

Code Review

Overall this is a well-structured addition. The gating logic (react_on_rails_pro? / enable_rsc_support) correctly ensures zero noise for base installs, error messages include actionable fix steps, and the test coverage is thorough with proper tmpdir isolation.

Issues

1. check_base_package_imports misses the client/ JS root (inline comment)
The scan only covers app/javascript/, but the existing doctor at line 1019 also checks client/**/*.{js,ts,jsx,tsx}. Projects using that layout will get a false "✅ No base imports found". Fix: add the client/ glob (and align to the brace-expansion style used elsewhere).

2. Regex matches commented-out imports — false positives (inline comment)
BASE_PACKAGE_IMPORT_PATTERN will flag // import ReactOnRails from 'react-on-rails'; as a live import. This produces a false-positive warning with the file name listed, which is confusing to users mid-migration. At minimum the warning could note that commented-out lines are included in the scan.

3. check_pro_setup lacks a top-level rescue (inline comment)
check_rsc_setup has rescue StandardError => e at the method level; check_pro_setup does not. If check_pro_initializer_existence throws outside its own rescue, the error propagates through the check runner rather than producing a friendly warning.

4. "NodeRenderer" string duplicated across two methods (inline comment)
The literal is used in check_pro_renderer_mode and check_rsc_renderer_mode. A single constant alongside RSC_BUNDLER_CONFIG_PATHS would make future renames safe.

5. Missing File.exist? guard in glob loop (inline comment)
The existing doctor guards each globbed path at line 1021 with next unless File.exist?(file). The new loop skips this, which is inconsistent and could bite on symlink edge cases.

Minor / nits

  • detect_react_version_from_deps doesn't check peerDependencies — unlikely to matter in practice since react is almost always in dependencies, but worth a comment.
  • The check_rsc_react_version branch major == 19 && minor.positive? will also catch React 20+ (e.g. 20.0.0) with a "not verified" warning rather than an error. This might be intentional but could surprise users on a future major.

🤖 Generated with Claude Code

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 18, 2026

Greptile Summary

This PR extends react_on_rails:doctor with two new diagnostic sections — React on Rails Pro Setup (3 checks) and React Server Components (5 checks) — both gated behind ReactOnRails::Utils.react_on_rails_pro? so OSS users see zero additional output. The implementation is well-structured, uses runtime config instead of brittle regex parsing, and comes with thorough spec coverage (7 new describe groups, ~611 lines).

Key issues found during review:

  • React version false-positive warning (P1): detect_react_version_from_deps strips the semver operator from the declared dependency range (e.g. "^19.0.0""19.0.0"), so it always checks the lower-bound of the range rather than the resolved/installed version. A project pinned to ^19.0.0 will see a "security vulnerabilities" warning on every doctor run even though their lock file likely resolves to 19.0.4+. Reading from node_modules/react/package.json (or the lock file) would give the actually-installed version.
  • check_pro_setup missing a top-level rescue: Unlike the sibling check_rsc_setup, there is no outer rescue StandardError in check_pro_setup. Unexpected I/O errors (e.g. Errno::EACCES) propagate unhandled and crash the entire doctor run.
  • React 20+ falls into a hard error branch: The else clause in check_rsc_react_version fires for any version that is not explicitly covered, including future major > 19 releases. This would report a categorical "not compatible" error for hypothetically compatible React 20+ versions rather than an "unverified" warning.
  • Top-level rescue in check_rsc_setup silently discards remaining sub-checks: A single rescue wrapping all five sub-check calls means the first unexpected exception causes the rest (routes, bundler config, procfile watcher) to be silently skipped.

Confidence Score: 3/5

  • Safe to merge with awareness of the version-range false-positive, which will produce misleading security warnings for the common ^19.0.0 package.json pattern.
  • The new checks are gated on Pro gem presence so there is zero regression risk for OSS users. Test coverage is solid and the overall design is correct. However, the detect_react_version_from_deps false-positive is a user-facing issue (misleading security warning shown on every doctor run for a common semver pattern), the missing top-level rescue is an unhandled crash path, and the React 20+ error branch will mislead future users. These are not blocking for a diagnostic/informational tool, but they lower confidence.
  • react_on_rails/lib/react_on_rails/doctor.rb — specifically detect_react_version_from_deps, check_pro_setup, and the else branch in check_rsc_react_version.

Important Files Changed

Filename Overview
react_on_rails/lib/react_on_rails/doctor.rb Adds ~240 lines covering Pro and RSC diagnostic checks. Key issues: version detection reads declared semver range (not installed version), causing false-positive security warnings; check_pro_setup lacks a top-level rescue unlike the sibling check_rsc_setup; React 20+ falls into an error branch rather than a warning; top-level rescue in check_rsc_setup silently skips remaining sub-checks on first failure.
react_on_rails/spec/lib/react_on_rails/doctor_spec.rb Adds 611 lines of test coverage across 7 new describe blocks. Good use of Dir.mktmpdir + Dir.chdir for file-system isolation, and unverified doubles are well-justified. One gap: no test exercises check_rsc_react_version with a semver-range value (e.g. ^19.0.0) to document the false-positive warning behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[run_all_checks] --> B{check_pro_setup}
    B -->|react_on_rails_pro? = false| SKIP1[skip — no output]
    B -->|react_on_rails_pro? = true| C[check_pro_initializer_existence]
    C --> D[check_pro_renderer_mode]
    D -->|NodeRenderer| D1[add_success]
    D -->|other| D2[add_info x2]
    D -->|raises| D3[add_warning rescue]
    D1 & D2 & D3 --> E[check_base_package_imports]
    E -->|no base imports| E1[add_success]
    E -->|base imports found| E2[add_warning with file list]
    E -->|raises| E3[add_warning rescue]

    A --> F{check_rsc_setup}
    F -->|react_on_rails_pro? = false| SKIP2[skip — no output]
    F -->|RSC disabled| SKIP3[skip — no output]
    F -->|RSC enabled| G[add_info x3]
    G --> H[check_rsc_renderer_mode]
    H -->|NodeRenderer| H1[pass — no message]
    H -->|other| H2[add_error]
    H1 & H2 --> I[check_rsc_payload_route]
    I -->|route present| I1[add_success]
    I -->|missing| I2[add_error]
    I1 & I2 --> J[check_rsc_bundler_config]
    J -->|config found| J1[add_success]
    J -->|missing| J2[add_error]
    J1 & J2 --> K[check_rsc_react_version]
    K -->|19.0.4+| K1[add_success]
    K -->|19.0.0-19.0.3| K2[add_warning — security vulns]
    K -->|19.1+| K3[add_warning — unverified]
    K -->|lt 19| K4[add_error]
    K -->|undetectable| K5[add_info skip]
    K1 & K2 & K3 & K4 & K5 --> L[check_rsc_procfile_watcher]
    L -->|RSC_BUNDLE_ONLY present| L1[add_success]
    L -->|missing| L2[add_warning]
    F -->|raises anywhere| ERR[add_warning rescue]
Loading

Last reviewed commit: "Remove redundant RSC..."

Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb Outdated
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3dd56c080a

ℹ️ 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".

Comment thread react_on_rails/lib/react_on_rails/doctor.rb Outdated
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Two bugs found by review:

1. The doctor rake task doesn't depend on :environment, so Rails
   initializers haven't run when Pro/RSC checks execute. This means
   ReactOnRailsPro.configuration returns defaults (ExecJS, RSC disabled)
   instead of the app's configured values. Fix: lazily load
   config/environment.rb before reading Pro config, with a warning
   fallback if the app can't boot.

2. The base-package import scan hardcoded app/javascript/**, but
   Shakapacker's source_path can be anything (both dummy apps use
   client/app). Fix: read source_path from Shakapacker config,
   falling back to app/javascript.

Ref: #2427

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Mar 18, 2026

Code Review

Overall this is a solid, well-structured addition. The separation of concerns is clean, the messages are actionable, and gating the new sections on Pro detection avoids noise for OSS users. A few things worth addressing before merge:

Bug: detect_react_version_from_deps reads the declared version, not the installed version. package.json frequently contains ranges like caret-19.0.0. After stripping the prefix the method yields 19.0.0 and the RSC check warns/errors even though 19.0.4 is actually installed. Reading from package-lock.json (.packages[node_modules/react].version) or yarn.lock would give the accurate installed version.

Bug: check_base_package_imports can abort early on encoding errors. The inner File.read loop has no per-file encoding rescue. A single binary asset or non-UTF-8 file inside the source path raises Encoding::InvalidByteSequenceError and silently skips all remaining files; the outer rescue on line 2294 only surfaces a generic warning for the whole method. The existing scan_pattern_for_async already rescues encoding errors per file -- the same pattern should apply here.

Fragile implicit ordering: check_rsc_setup depends on check_pro_setup having run first. check_rsc_setup reads ReactOnRailsPro.configuration but never calls ensure_rails_environment_loaded. It works today because check_pro_setup appears earlier in the checks array and calls it. If the order ever changes, or check_rsc_setup is invoked standalone in a test, it reads uninitialised default config values. Adding an explicit ensure_rails_environment_loaded call at the top of check_rsc_setup (mirroring check_pro_setup) makes the dependency self-contained.

Minor: check_rsc_react_version errors on React 20+. The version logic covers major == 19 but has no branch for major >= 20. Future users on React 20 hit the else-branch and see a hard error claiming incompatibility. Worth adding a major >= 20 warning (unverified) or at minimum a comment noting the conservative intent.

Nit: check_rsc_payload_route and check_rsc_procfile_watcher do not filter comments. String#include? will match a commented-out rsc_payload_route or RSC_BUNDLE_ONLY line and report a false success. Low likelihood in practice, but easy to address with a line-by-line filter similar to config_has_async_loading_strategy?.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d3cb788a1

ℹ️ 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".

Comment thread react_on_rails/lib/react_on_rails/doctor.rb Outdated
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Reuse the existing parse_shakapacker_config method (which passes
aliases: true) instead of a separate YAML.safe_load call. Without
aliases: true, the fallback fails on standard shakapacker.yml files
that use YAML anchors (default: &default / <<: *default).

Also add ensure_rails_environment_loaded to check_rsc_setup for
self-containment — it was relying on check_pro_setup running first.

Ref: #2427

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Mar 19, 2026

Review: Add Pro/RSC setup checks to react_on_rails:doctor

Good addition overall. The gating logic (skip entirely for non-Pro installs), the fallback chain for detecting the JS source path, and the decision to use Node module resolution instead of stripping semver ranges are all solid choices. Test coverage is thorough. Four issues to address (see inline comments for details):

Bugs

1. Duplicate warnings from ensure_rails_environment_loaded (line 2196)
On failure (missing file or boot error), the @rails_environment_loaded flag is never set, so the idempotency guard never fires on subsequent calls. Both check_pro_setup and check_rsc_setup call this method, meaning a broken boot emits the warning twice. The comment says "only loads once" but that only holds on the success path. A tri-state (:unknown/:loaded/:failed) fixes this.

2. Unreadable file aborts the entire import scan (line 2274)
File.read(file) without explicit encoding raises Encoding::UndefinedConversionError on non-UTF-8 or binary files. The method-level rescue StandardError swallows this along with all previously accumulated results, rather than just skipping the bad file.

Minor issues

3. Side-effect imports not detected (line 2263)
The bare import form without a from clause bypasses both regex patterns. Worth documenting as a known gap.

4. devDependencies silently wins in declared_react_version (line 2459)
(dependencies || {}).merge(devDependencies || {}) means the dev-declared version overrides the production one when react appears in both. Reversing the merge order makes the production dependency take priority.

Nits

  • The rubocop:disable Metrics/CyclomaticComplexity on check_base_package_imports appears unnecessary: the method has a single if/else and one unless, well below the threshold.
  • Missing blank line between the closing end of report_configure_private_output_path and the helpers section separator at line 2176.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21cc667181

ℹ️ 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".

Comment thread react_on_rails/lib/react_on_rails/doctor.rb
1. Cache failed Rails boot attempts so ensure_rails_environment_loaded
   doesn't retry and emit the same warning from both check_pro_setup
   and check_rsc_setup. Reproduced: confirmed 2 identical warnings
   before fix, 1 after.

2. Remove `npm install react@~19.0.4` from the React 19.1+/20+ warning
   since it would suggest downgrading. Now just states verified
   compatibility is 19.0.4+.

Ref: #2427

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
Comment thread react_on_rails/lib/react_on_rails/doctor.rb
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Mar 19, 2026

Review: Add Pro/RSC setup checks to react_on_rails:doctor

Overall this is solid work — the implementation is well-structured, the gating logic (skip for base installs) is clean, the test coverage is thorough, and the iterative fixes across commits (YAML alias handling, duplicate warning dedup, \z anchor, Open3 migration) show good attention to detail.

Three issues worth addressing before merge (inline comments attached):

Bugs

1. installed_react_version regex missing \z (line 2452)
The match /\A\d+\.\d+\.\d+/ passes for prerelease tags like 19.0.4-rc.1. Ruby's "4-rc".to_i == 4, so prerelease versions silently compare as their stable equivalent. declared_react_version correctly uses \z — apply the same fix here.

2. Open3.capture3("node", ...) has no timeout (line 2444–2445)
If node is on PATH but hangs (broken Node install, Docker environment with a mounted but unresponsive socket), the doctor command blocks indefinitely. A 5-second Timeout.timeout wrapper would keep the tool snappy even in edge cases.

3. Mid-scan error in check_base_package_imports loses partial results (line 2299)
A File.read failure (encoding error on a binary, permission issue) triggers the outer rescue, which discards any files_with_base_import already collected. Moving the rescue inside the per-file loop lets the scan continue and preserve matches found before the failure.

Minor observations (no action required)

  • @rails_environment_attempted is not initialised in initialize alongside @rails_environment_loaded. Works correctly since nil is falsy, but the asymmetry is slightly surprising when reading initialize.
  • check_base_package_imports carries a # rubocop:disable Metrics/CyclomaticComplexity comment, but the method has at most one conditional branch — the complexity disable appears unnecessary.
  • ensure_rails_environment_loaded's return value is not checked in check_pro_setup/check_rsc_setup. Downstream methods already rescue their own errors so this works in practice, but an explicit return after the failed-load case would make the control flow more obvious.

@justin808 justin808 merged commit dac5267 into main Mar 21, 2026
38 checks passed
@justin808 justin808 deleted the ihabadham/feature/rsc-doctor-checks branch March 21, 2026 23:20
@justin808
Copy link
Copy Markdown
Member

Thanks @ihabadham

justin808 pushed a commit that referenced this pull request Mar 30, 2026
## Summary

- Extends `react_on_rails:doctor` with two new conditional diagnostic
sections: **React on Rails Pro Setup** and **React Server Components**
- Both sections are skipped entirely for base-only installs — zero noise
for OSS users
- Uses runtime config (`ReactOnRailsPro.configuration`) instead of regex
parsing for reliable detection
- Every error/warning includes actionable fix instructions

### Pro Setup checks (3)
- **Pro initializer existence** — warns if
`config/initializers/react_on_rails_pro.rb` is missing
- **Renderer mode** — reports NodeRenderer vs ExecJS, notes NodeRenderer
is required for RSC
- **Base package import scanning** — detects JS files importing from
`'react-on-rails'` instead of `'react-on-rails-pro'`. The base package
is a transitive dependency of Pro, so these imports resolve silently but
load the base version without Pro features (verified via reproduction)

### RSC checks (5, gated on `enable_rsc_support`)
- **Renderer mode** — errors if RSC is enabled without NodeRenderer
- **RSC payload route** — errors if `rsc_payload_route` is missing from
routes (causes silent 404s)
- **RSC bundler config** — errors if `rscWebpackConfig.js` is missing
(checks both webpack and rspack paths)
- **React version** — error for <19, warning for 19.0.0-19.0.3 (security
vulns), warning for 19.1+ (unverified), success for 19.0.4+
- **Procfile RSC watcher** — warns if `RSC_BUNDLE_ONLY` entry is missing
(warning-only since custom process managers are valid)

Closes #2427

## Test plan

- [x] 98 doctor specs pass (7 new test groups with 30 examples covering
all checks)
- [x] RuboCop passes with zero offenses
- [x] CI passes
- [ ] Manual: run `rake react_on_rails:doctor` on a base install —
Pro/RSC sections should not appear
- [ ] Manual: run on a Pro install — Pro section should appear with
initializer/renderer/import checks


🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enhanced diagnostics for Pro and React Server Components (RSC):
validates renderer mode, bundler config presence, payload route,
Procfile watcher, React version compatibility, and flags incorrect
base-package imports. Detects Rails environment load issues and reports
clear warnings with guidance. Scans customizable JS source paths when
checking imports.

* **Tests**
* Expanded test coverage for the new diagnostics, edge cases, and
error/warning reporting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
justin808 pushed a commit that referenced this pull request Apr 6, 2026
## Summary

- Extends `react_on_rails:doctor` with two new conditional diagnostic
sections: **React on Rails Pro Setup** and **React Server Components**
- Both sections are skipped entirely for base-only installs — zero noise
for OSS users
- Uses runtime config (`ReactOnRailsPro.configuration`) instead of regex
parsing for reliable detection
- Every error/warning includes actionable fix instructions

### Pro Setup checks (3)
- **Pro initializer existence** — warns if
`config/initializers/react_on_rails_pro.rb` is missing
- **Renderer mode** — reports NodeRenderer vs ExecJS, notes NodeRenderer
is required for RSC
- **Base package import scanning** — detects JS files importing from
`'react-on-rails'` instead of `'react-on-rails-pro'`. The base package
is a transitive dependency of Pro, so these imports resolve silently but
load the base version without Pro features (verified via reproduction)

### RSC checks (5, gated on `enable_rsc_support`)
- **Renderer mode** — errors if RSC is enabled without NodeRenderer
- **RSC payload route** — errors if `rsc_payload_route` is missing from
routes (causes silent 404s)
- **RSC bundler config** — errors if `rscWebpackConfig.js` is missing
(checks both webpack and rspack paths)
- **React version** — error for <19, warning for 19.0.0-19.0.3 (security
vulns), warning for 19.1+ (unverified), success for 19.0.4+
- **Procfile RSC watcher** — warns if `RSC_BUNDLE_ONLY` entry is missing
(warning-only since custom process managers are valid)

Closes #2427

## Test plan

- [x] 98 doctor specs pass (7 new test groups with 30 examples covering
all checks)
- [x] RuboCop passes with zero offenses
- [x] CI passes
- [ ] Manual: run `rake react_on_rails:doctor` on a base install —
Pro/RSC sections should not appear
- [ ] Manual: run on a Pro install — Pro section should appear with
initializer/renderer/import checks


🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enhanced diagnostics for Pro and React Server Components (RSC):
validates renderer mode, bundler config presence, payload route,
Procfile watcher, React version compatibility, and flags incorrect
base-package imports. Detects Rails environment load issues and reports
clear warnings with guidance. Scans customizable JS source paths when
checking imports.

* **Tests**
* Expanded test coverage for the new diagnostics, edge cases, and
error/warning reporting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RSC support: add rsc:doctor preflight diagnostics

2 participants