From 1502b93931b4740dcad7497110a2a27146326073 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 29 Apr 2026 23:42:55 -1000 Subject: [PATCH 01/39] Detect stale Pro migration module specifiers --- react_on_rails/lib/react_on_rails/doctor.rb | 9 ++++- .../spec/lib/react_on_rails/doctor_spec.rb | 38 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index b9d90d1b2c..e94f06d966 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2752,8 +2752,10 @@ def check_pro_renderer_mode # caching, RSC), and may cause "component not registered" errors at runtime. BASE_PACKAGE_IMPORT_PATTERN = %r{\bfrom\s+['"]react-on-rails(?:/[^'"]*)?['"]} BASE_PACKAGE_REQUIRE_PATTERN = %r{\brequire\s*\(\s*['"]react-on-rails(?:/[^'"]*)?['"]\s*\)} + BASE_PACKAGE_MOCK_PATTERN = %r{\b\w+\.(?:mock|unmock|doMock|dontMock|requireActual|requireMock)\s*\(\s*['"]react-on-rails(?:/[^'"]*)?['"]} # rubocop:disable Layout/LineLength + BASE_PACKAGE_DECLARE_MODULE_PATTERN = %r{^\s*declare\s+module\s+['"]react-on-rails(?:/[^'"]*)?['"]} - def check_base_package_imports # rubocop:disable Metrics/CyclomaticComplexity + def check_base_package_imports # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity source_path = resolve_js_source_path js_extensions = %w[js jsx ts tsx] js_patterns = js_extensions.map { |ext| "#{source_path}/**/*.#{ext}" } @@ -2762,7 +2764,10 @@ def check_base_package_imports # rubocop:disable Metrics/CyclomaticComplexity js_patterns.each do |pattern| Dir.glob(pattern).each do |file| content = File.read(file) - next unless content.match?(BASE_PACKAGE_IMPORT_PATTERN) || content.match?(BASE_PACKAGE_REQUIRE_PATTERN) + next unless content.match?(BASE_PACKAGE_IMPORT_PATTERN) || + content.match?(BASE_PACKAGE_REQUIRE_PATTERN) || + content.match?(BASE_PACKAGE_MOCK_PATTERN) || + content.match?(BASE_PACKAGE_DECLARE_MODULE_PATTERN) files_with_base_import << file end diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 36c85be15f..4875db60a8 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2530,6 +2530,44 @@ class << self end end + context "when JS tests mock the base package after a Pro migration" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/app.test.ts", + "jest.mock('react-on-rails', () => ({ authenticityHeaders: jest.fn() }));\n") + example.run + end + end + end + + it "reports warning" do + doctor.send(:check_base_package_imports) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("app.test.ts") }).to be true + end + end + + context "when TypeScript declaration files augment the base package after a Pro migration" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/types") + File.write("app/javascript/types/react-on-rails.d.ts", + "declare module 'react-on-rails' {\n export function register(): void;\n}\n") + example.run + end + end + end + + it "reports warning" do + doctor.send(:check_base_package_imports) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("react-on-rails.d.ts") }).to be true + end + end + context "when JS files correctly import from 'react-on-rails-pro'" do around do |example| Dir.mktmpdir do |tmpdir| From fa3c40728c8b8c5c5533d3ce2cbb775ee21aa19d Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 29 Apr 2026 23:44:03 -1000 Subject: [PATCH 02/39] Add changelog for Pro doctor specifier scan --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86b96ba30b..d657d1eefb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ #### Fixed +- **[Pro]** **Doctor detects more stale Pro migration imports**: `react_on_rails:doctor` now warns when Pro apps still reference the base package through Jest/Vitest mock helpers or TypeScript `declare module` blocks, catching migration leftovers that import/require scanning missed. See [Issue 3104](https://github.com/shakacode/react_on_rails/issues/3104). [PR 3232](https://github.com/shakacode/react_on_rails/pull/3232) by [justin808](https://github.com/justin808). - **[Pro]** **Node renderer now exposes `performance` when `supportModules: true`**: React 19's development build of `React.lazy` calls `performance.now()`, which previously threw `ReferenceError: performance is not defined` inside the node renderer's VM context unless users manually added `performance` via `additionalContext`. `performance` is now included in the default globals alongside `Buffer`, `process`, etc. Fixes [Issue 3154](https://github.com/shakacode/react_on_rails/issues/3154). [PR 3158](https://github.com/shakacode/react_on_rails/pull/3158) by [justin808](https://github.com/justin808). - **Client startup now recovers if initialization begins during `interactive` after `DOMContentLoaded` already fired**: React on Rails now still initializes the page when the client bundle starts in the browser timing window after `DOMContentLoaded` but before the document reaches `complete`. Fixes [Issue 3150](https://github.com/shakacode/react_on_rails/issues/3150). [PR 3151](https://github.com/shakacode/react_on_rails/pull/3151) by [ihabadham](https://github.com/ihabadham). - **Doctor accepts TypeScript server bundle entrypoints**: `react_on_rails:doctor` now resolves common source entrypoint suffixes (`.js`, `.jsx`, `.ts`, `.tsx`, `.mjs`, `.cjs`) before warning that the server bundle is missing, preventing false positives when apps use `server-bundle.ts`. [PR 3111](https://github.com/shakacode/react_on_rails/pull/3111) by [justin808](https://github.com/justin808). From fec385eff05c84f8edcadee8ac16e8c876ff843c Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 3 May 2026 21:27:41 -1000 Subject: [PATCH 03/39] Clarify Pro doctor base package reference warning --- react_on_rails/lib/react_on_rails/doctor.rb | 30 +++++---- .../spec/lib/react_on_rails/doctor_spec.rb | 64 +++++++++++++++++++ 2 files changed, 80 insertions(+), 14 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index e94f06d966..2105db169c 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2747,9 +2747,9 @@ def check_pro_renderer_mode end # The base 'react-on-rails' npm package is a transitive dependency of 'react-on-rails-pro', - # so `import ... from 'react-on-rails'` resolves silently — loading the base package instead - # of Pro. Components registered through the base package won't have Pro features (streaming, - # caching, RSC), and may cause "component not registered" errors at runtime. + # so references to 'react-on-rails' resolve silently, loading the base package instead of Pro. + # Components registered through the base package won't have Pro features (streaming, caching, + # RSC), and may cause "component not registered" errors at runtime. BASE_PACKAGE_IMPORT_PATTERN = %r{\bfrom\s+['"]react-on-rails(?:/[^'"]*)?['"]} BASE_PACKAGE_REQUIRE_PATTERN = %r{\brequire\s*\(\s*['"]react-on-rails(?:/[^'"]*)?['"]\s*\)} BASE_PACKAGE_MOCK_PATTERN = %r{\b\w+\.(?:mock|unmock|doMock|dontMock|requireActual|requireMock)\s*\(\s*['"]react-on-rails(?:/[^'"]*)?['"]} # rubocop:disable Layout/LineLength @@ -2759,7 +2759,7 @@ def check_base_package_imports # rubocop:disable Metrics/CyclomaticComplexity, M source_path = resolve_js_source_path js_extensions = %w[js jsx ts tsx] js_patterns = js_extensions.map { |ext| "#{source_path}/**/*.#{ext}" } - files_with_base_import = [] + files_with_base_reference = [] js_patterns.each do |pattern| Dir.glob(pattern).each do |file| @@ -2769,27 +2769,29 @@ def check_base_package_imports # rubocop:disable Metrics/CyclomaticComplexity, M content.match?(BASE_PACKAGE_MOCK_PATTERN) || content.match?(BASE_PACKAGE_DECLARE_MODULE_PATTERN) - files_with_base_import << file + files_with_base_reference << file end end - if files_with_base_import.empty? - checker.add_success("✅ No base 'react-on-rails' imports found (Pro package used correctly)") + if files_with_base_reference.empty? + checker.add_success("✅ No base 'react-on-rails' references found (Pro package used correctly)") else checker.add_warning(<<~MSG.strip) - ⚠️ Found imports from 'react-on-rails' instead of 'react-on-rails-pro': - #{files_with_base_import.map { |f| " • #{f}" }.join("\n")} + ⚠️ Found references to 'react-on-rails' instead of 'react-on-rails-pro': + #{files_with_base_reference.map { |f| " • #{f}" }.join("\n")} - The base package is a transitive dependency of Pro, so these imports resolve + The base package is a transitive dependency of Pro, so these references resolve silently but load the base version without Pro features. - Fix: Update imports to use 'react-on-rails-pro': - import ReactOnRails from 'react-on-rails-pro'; // server - import ReactOnRails from 'react-on-rails-pro/client'; // client + Fix: Replace base-package references with their Pro equivalents: + import ReactOnRails from 'react-on-rails-pro'; // ES import (server) + import ReactOnRails from 'react-on-rails-pro/client'; // ES import (client) + jest.mock('react-on-rails-pro', ...); // Jest/Vitest mock + declare module 'react-on-rails-pro' { ... } // TypeScript augmentation MSG end rescue StandardError => e - checker.add_warning("⚠️ Could not scan for base package imports: #{e.message}") + checker.add_warning("⚠️ Could not scan for base package references: #{e.message}") end # ── React Server Components ──────────────────────────────────── diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 4875db60a8..34a84abdb9 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -2542,6 +2542,27 @@ class << self end end + it "reports warning" do + doctor.send(:check_base_package_imports) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("app.test.ts") }).to be true + expect(warning_msgs.any? { |m| m[:content].include?("Found references to 'react-on-rails'") }).to be true + expect(warning_msgs.any? { |m| m[:content].include?("Jest/Vitest mock") }).to be true + end + end + + context "when JS tests mock a base package subpath after a Pro migration" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/app.test.ts", + "jest.mock('react-on-rails/client', () => ({ authenticityHeaders: jest.fn() }));\n") + example.run + end + end + end + it "reports warning" do doctor.send(:check_base_package_imports) warning_msgs = checker.messages.select { |m| m[:type] == :warning } @@ -2565,6 +2586,7 @@ class << self doctor.send(:check_base_package_imports) warning_msgs = checker.messages.select { |m| m[:type] == :warning } expect(warning_msgs.any? { |m| m[:content].include?("react-on-rails.d.ts") }).to be true + expect(warning_msgs.any? { |m| m[:content].include?("TypeScript augmentation") }).to be true end end @@ -2587,6 +2609,48 @@ class << self end end + context "when JS tests correctly mock 'react-on-rails-pro'" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/app.test.ts", + "jest.mock('react-on-rails-pro', () => ({ authenticityHeaders: jest.fn() }));\n") + example.run + end + end + end + + it "reports success" do + doctor.send(:check_base_package_imports) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + success_msgs = checker.messages.select { |m| m[:type] == :success } + expect(warning_msgs).to be_empty + expect(success_msgs.any? { |m| m[:content].include?("Pro package used correctly") }).to be true + end + end + + context "when TypeScript declarations correctly augment 'react-on-rails-pro'" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/types") + File.write("app/javascript/types/react-on-rails-pro.d.ts", + "declare module 'react-on-rails-pro' {\n export function register(): void;\n}\n") + example.run + end + end + end + + it "reports success" do + doctor.send(:check_base_package_imports) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + success_msgs = checker.messages.select { |m| m[:type] == :success } + expect(warning_msgs).to be_empty + expect(success_msgs.any? { |m| m[:content].include?("Pro package used correctly") }).to be true + end + end + context "when no JS files exist" do around do |example| Dir.mktmpdir do |tmpdir| From 43aa8c22736fc599a1c3e15d4b74e42d7681c6c1 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 3 May 2026 21:40:48 -1000 Subject: [PATCH 04/39] Cover Vitest base package references --- .claude/commands/stress-test.md | 207 +++++++++--------- react_on_rails/lib/react_on_rails/doctor.rb | 4 +- .../spec/lib/react_on_rails/doctor_spec.rb | 59 +++++ 3 files changed, 167 insertions(+), 103 deletions(-) diff --git a/.claude/commands/stress-test.md b/.claude/commands/stress-test.md index 0ca0583ddc..72ca58e981 100644 --- a/.claude/commands/stress-test.md +++ b/.claude/commands/stress-test.md @@ -69,7 +69,7 @@ How to measure: - Use `oha` (preferred) or `ab` to issue concurrent requests at multiple concurrency levels (1, 4, 16, 64 — capped by tier). Record p50/p95/p99/throughput for each demo's hot routes. - Compare: same component with `prerender: true` vs `prerender: false`. Same RSC route with cache hit vs cold. Same streaming response with Suspense boundaries vs flat. - Watch CPU and renderer worker saturation during the load (`top -pid `). -- Treat any case where a documented optimization (caching, streaming, immediate hydration, async loading) does *not* improve over its alternative as a finding. +- Treat any case where a documented optimization (caching, streaming, immediate hydration, async loading) does _not_ improve over its alternative as a finding. A vector or persona that does not produce explicit measurements in these three concerns has not done its job. @@ -77,30 +77,30 @@ A vector or persona that does not produce explicit measurements in these three c ## Argument parsing -| Form | Meaning | -|---|---| -| *(empty)* | Stress test the **whole framework** at the current `HEAD` of `main` | -| `` | Focus only on features/code paths touched by that commit | -| `` or PR URL (`https://github.com/.../pull/N`) | Focus only on changes in that PR | -| `--from ` | All commits from ``..default-branch (resolved via `git symbolic-ref --short refs/remotes/origin/HEAD`; falls back to `main`/`master` lookup; aborts if neither exists) | -| `--from --to ` | All commits from `--from` to `--to` (branch or commit, not necessarily the default branch) | -| `--features ` | Comma-separated feature scope filter (see "Feature scopes" below) | -| `--tier quick\|standard\|deep\|exhaustive` | Time/coverage tier. Default: `standard` (or `quick` if scope is a single small commit/PR) | -| `--max-hours N` | Override tier's wallclock ceiling. Hard cap; agents stop when reached | -| `--no-network-fault` | Skip toxiproxy / network simulation phase | -| `--skip-pro` | Skip Pro tier (RSC / streaming / node renderer) phases | -| `--repo ` | Override framework repo location (default: autodetect from `git rev-parse --show-toplevel`) | +| Form | Meaning | +| --------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| _(empty)_ | Stress test the **whole framework** at the current `HEAD` of `main` | +| `` | Focus only on features/code paths touched by that commit | +| `` or PR URL (`https://github.com/.../pull/N`) | Focus only on changes in that PR | +| `--from ` | All commits from ``..default-branch (resolved via `git symbolic-ref --short refs/remotes/origin/HEAD`; falls back to `main`/`master` lookup; aborts if neither exists) | +| `--from --to ` | All commits from `--from` to `--to` (branch or commit, not necessarily the default branch) | +| `--features ` | Comma-separated feature scope filter (see "Feature scopes" below) | +| `--tier quick\|standard\|deep\|exhaustive` | Time/coverage tier. Default: `standard` (or `quick` if scope is a single small commit/PR) | +| `--max-hours N` | Override tier's wallclock ceiling. Hard cap; agents stop when reached | +| `--no-network-fault` | Skip toxiproxy / network simulation phase | +| `--skip-pro` | Skip Pro tier (RSC / streaming / node renderer) phases | +| `--repo ` | Override framework repo location (default: autodetect from `git rev-parse --show-toplevel`) | Multiple flags can combine: ` --tier deep --features rsc,streaming --no-network-fault`. ### Tier defaults -| Tier | Wallclock | Demos | Personas | Pentest | Leak/perf load | Max parallel agents | -|---|---|---|---|---|---|---| -| quick | 30–60 min | 1–2 | 2 (extreme user, novice) | smoke | N=200 reqs | 4 | -| standard | 2–4 hr | 5 | 4 (extreme, novice, distracted senior, attacker) | 1 pass | N=2000 reqs, conc 1/4/16 | 8 | -| deep | 8–16 hr | 5 + variants | 6 (+ ops engineer, malicious) | full pass with prop-fuzzing | N=20000 reqs, conc 1/4/16/64, heap snapshots | 12 | -| exhaustive | 24–48 hr ⚠️ **very high API cost** | full feature matrix | all + multiple seeds | + regression replays | + 24h soak per demo | 12 | +| Tier | Wallclock | Demos | Personas | Pentest | Leak/perf load | Max parallel agents | +| ---------- | ---------------------------------- | ------------------- | ------------------------------------------------ | --------------------------- | -------------------------------------------- | ------------------- | +| quick | 30–60 min | 1–2 | 2 (extreme user, novice) | smoke | N=200 reqs | 4 | +| standard | 2–4 hr | 5 | 4 (extreme, novice, distracted senior, attacker) | 1 pass | N=2000 reqs, conc 1/4/16 | 8 | +| deep | 8–16 hr | 5 + variants | 6 (+ ops engineer, malicious) | full pass with prop-fuzzing | N=20000 reqs, conc 1/4/16/64, heap snapshots | 12 | +| exhaustive | 24–48 hr ⚠️ **very high API cost** | full feature matrix | all + multiple seeds | + regression replays | + 24h soak per demo | 12 | The `Max parallel agents` value is the **hard ceiling on concurrent sub-agents at any moment, across all phases** — when the cap is hit, queue further spawns and wait for an in-flight agent to finish. Without this cap, an exhaustive run could reasonably want 6 personas × 7 demos × 13 feature areas of agents simultaneously, which would exhaust the host machine and the API quota. @@ -110,39 +110,39 @@ If no `--tier` and no scope given → `standard`. If a single small commit/PR ( Comma-separated list. Unknown values abort with the list of valid values. If both `--features` and a commit/PR/range scope are given, the intersection wins (only features touched by the diff AND in the list are stressed). -| Value | Stress focus | -|---|---| -| `ssr` | All server rendering (covers `ssr-execjs` + `ssr-node` + streaming + non-streaming) | -| `ssr-execjs` | ExecJS-backed SSR only — context reuse, console polyfill, async/await unsupported paths | -| `ssr-node` | Pro Node renderer SSR — HTTP transport, fallback, retries, keep-alive | -| `ssr-no-streaming` | Traditional non-streaming SSR — `prerender: true`, `react_component_hash` | -| `streaming` | Pro streaming SSR — `stream_react_component`, Suspense boundaries, abort propagation, `Rack::Deflater` interaction | -| `rsc` | React Server Components — `'use client'`, server/client boundary, three-bundle build | -| `rsc-payload` | RSC payload generation specifically — `rsc_payload_react_component`, NDJSON framing, `injectRSCPayload`, manifest mapping | -| `hydration` | Client hydration — `ClientRenderer`, hydrate vs render decision, mismatch warnings, immediate_hydration | -| `immediate-hydration` | Pro immediate hydration only | -| `auto-bundling` | `auto_load_bundle`, `nested_entries`, `ror_components/`, `generate_packs`, file-system-based registration | -| `registration` | `ReactOnRails.register`, `registerStore`, registry races, double-register, HMR re-register | -| `redux` | Redux store registration, `redux_store`, store generators, hydration of stores | -| `router` | React Router SSR (`StaticRouter`), wildcard route, location from railsContext | -| `turbo` | Turbo / Turbolinks integration — `setOptions({ turbo: true })`, page lifecycle, frame swap, stream targets | -| `hotwire` | Hotwire-broader (Turbo + Stimulus + morphing) | -| `i18n` | I18n — locale generator, server/client divergence, fallback chain, cache key inclusion | -| `caching` | All caching: `cached_react_component`, `prerender_caching`, fragment_cache wrapping, dependency_globs | -| `prerender-cache` | `prerender_caching` only — key derivation, locale, bundle digest, collisions | -| `props` | Props serialization — JSON-unsafe values, U+2028/2029, escaping, large payloads, circular refs | -| `rails-context` | `railsContext` content, mutation, RSC boundary crossing, mailer mode | -| `config` | Configuration loading, idempotency, Shakapacker auto-detect, env-var divergence | -| `generators` | `react_on_rails:install`, `generate_packs`, locale generator, partial-state recovery | -| `doctor` | `react_on_rails:doctor` task, `FIX=true` mode, false positives/negatives | -| `node-renderer` | Pro node renderer process — workers, restart intervals, OOM, file watcher, sandbox | -| `assets` | Manifest, Shakapacker integration, `private_output_path`, `assets_to_copy`, CDN/asset_host | -| `csp` | CSP nonce threading, inline `")` server-side. `console.log()` server-side then verify canary not echoed to a *different* user's response. -- *(caching)* `fragment_cache` wrapping `react_component` with cache key omitting `current_user.id`. `cached_react_component` with non-deterministic prop ordering. Two components sharing a prerender cache key. Locale missing from key. -- *(turbo/hotwire)* Turbo Frame swap mid-hydration, then again, then `turbo:before-cache`. bfcache: navigate away → back → observe React state. Idiomorph patching nodes React owns. Turbo Stream targeting a React root without `immediate_hydration`. **Memory:** navigate 100x and watch detached DOM count. -- *(assets)* Service Worker stub serving stale chunk after a "deploy" (rebuild assets in place). Manifest digest mismatch. `public/packs` purged mid-request. Grep client bundle for canary env vars (data leak). -- *(auto-bundling)* `make_generated_server_bundle_the_entrypoint = false` + add new `ror_components/` file. `auto_load_bundle: true` without re-running `generate_packs` after a rename. macOS dev casing → Linux container case-sensitive break. -- *(ssr-execjs)* `prerender: true` on async (React 19) component with ExecJS only. **Memory:** plant a module-level Map that grows per render; observe across N renders. -- *(streaming)* Suspense fallback that never resolves. Error boundary missing during streaming; child throws after first chunk flushed. `` wrapper on every leaf — pathological streaming chunk count. Rack::Deflater on the streaming response. Client closes tab mid-stream — verify server-side fetch is aborted (memory + perf). -- *(rsc)* `'use client'` missing on legacy entry pack after enabling RSC. Server-only library imported into a client component (data leak surface). Pass full `railsContext` (with functions) into a Client Component prop. -- *(rsc-payload)* Pollute the NDJSON line framing. Inject HTML error page mid-stream from a misconfigured proxy. Verify payload does not leak server-only props. -- *(node-renderer)* Slowloris against the renderer (perf degradation). SIGTERM during `allWorkersRestartInterval`. Mid-stream worker kill. Long soak with rolling-restart off vs on (memory leak signal). -- *(error-handling)* `raise_on_prerender_error` flipped between dev and prod. Error boundary missing during streaming. Throw an error containing a canary; check whether canary appears in user-visible response. -- *(csp)* Strict CSP with per-request nonces; observe whether RoR-generated `")` server-side. `console.log()` server-side then verify canary not echoed to a _different_ user's response. +- _(caching)_ `fragment_cache` wrapping `react_component` with cache key omitting `current_user.id`. `cached_react_component` with non-deterministic prop ordering. Two components sharing a prerender cache key. Locale missing from key. +- _(turbo/hotwire)_ Turbo Frame swap mid-hydration, then again, then `turbo:before-cache`. bfcache: navigate away → back → observe React state. Idiomorph patching nodes React owns. Turbo Stream targeting a React root without `immediate_hydration`. **Memory:** navigate 100x and watch detached DOM count. +- _(assets)_ Service Worker stub serving stale chunk after a "deploy" (rebuild assets in place). Manifest digest mismatch. `public/packs` purged mid-request. Grep client bundle for canary env vars (data leak). +- _(auto-bundling)_ `make_generated_server_bundle_the_entrypoint = false` + add new `ror_components/` file. `auto_load_bundle: true` without re-running `generate_packs` after a rename. macOS dev casing → Linux container case-sensitive break. +- _(ssr-execjs)_ `prerender: true` on async (React 19) component with ExecJS only. **Memory:** plant a module-level Map that grows per render; observe across N renders. +- _(streaming)_ Suspense fallback that never resolves. Error boundary missing during streaming; child throws after first chunk flushed. `` wrapper on every leaf — pathological streaming chunk count. Rack::Deflater on the streaming response. Client closes tab mid-stream — verify server-side fetch is aborted (memory + perf). +- _(rsc)_ `'use client'` missing on legacy entry pack after enabling RSC. Server-only library imported into a client component (data leak surface). Pass full `railsContext` (with functions) into a Client Component prop. +- _(rsc-payload)_ Pollute the NDJSON line framing. Inject HTML error page mid-stream from a misconfigured proxy. Verify payload does not leak server-only props. +- _(node-renderer)_ Slowloris against the renderer (perf degradation). SIGTERM during `allWorkersRestartInterval`. Mid-stream worker kill. Long soak with rolling-restart off vs on (memory leak signal). +- _(error-handling)_ `raise_on_prerender_error` flipped between dev and prod. Error boundary missing during streaming. Throw an error containing a canary; check whether canary appears in user-visible response. +- _(csp)_ Strict CSP with per-request nonces; observe whether RoR-generated `\n") + File.write("app/javascript/packs/Widget.svelte", + "\n\n
\n") + example.run + end + end + end + + it "reports warning for both .vue and .svelte files" do + doctor.send(:check_base_package_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("Widget.vue") }).to be true + expect(warning_msgs.any? { |m| m[:content].include?("Widget.svelte") }).to be true + end + end + context "when JS tests mock the base package after a Pro migration" do around do |example| Dir.mktmpdir do |tmpdir| From 53bb0d2c431ac3e3199c9b8d40746ae606aeb095 Mon Sep 17 00:00:00 2001 From: ihabadham Date: Fri, 15 May 2026 21:27:07 +0300 Subject: [PATCH 29/39] fix(doctor): skip node_modules in base package reference scan js_files_for_import_update in the Pro generator already rejects node_modules paths; files_with_base_package_references did not. For client-layout or Vite apps whose Shakapacker source_path contains a nested node_modules subtree, the recursive glob descended into transitive dependencies. The base package is a transitive dependency of Pro and its own files reference 'react-on-rails', so the scan produced guaranteed false-positive warnings listing dependency files. Add the same node_modules reject so the doctor scan stays consistent with the rewriter, as the "Keep in sync" comment requires. Co-Authored-By: Claude Opus 4.7 (1M context) --- react_on_rails/lib/react_on_rails/doctor.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 96272a6328..85c7fa162c 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2951,7 +2951,9 @@ def files_with_base_package_references(source_path) js_patterns = js_extensions.map { |ext| "#{source_path}/**/*.#{ext}" } js_patterns.flat_map do |pattern| - Dir.glob(pattern).select { |file| base_package_reference_file?(file) } + Dir.glob(pattern) + .reject { |file| file.include?("/node_modules/") } + .select { |file| base_package_reference_file?(file) } end.sort end From f117480d96981970118d337e03e73b86686f7cbb Mon Sep 17 00:00:00 2001 From: ihabadham Date: Fri, 15 May 2026 21:27:11 +0300 Subject: [PATCH 30/39] test(pro_generator): cover git: argument alongside the version pin The suffix-preservation regex is option-agnostic, so a version pin combined with git: is handled identically to the already-tested path: case. Add an explicit regression spec mirroring the path:+pin test so the git:+pin combination is locked in against future changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../react_on_rails/generators/pro_generator_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb index 1092ab257a..d0ea2c3806 100644 --- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb +++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb @@ -704,6 +704,19 @@ expect(gemfile_content).to include('gem "react_on_rails_pro", "~> 16.0", path: "../react_on_rails"') end + it "preserves a git: argument alongside the version pin" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem "react_on_rails", "~> 16.0", git: "https://example.invalid/repo.git" + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + generator.send(:swap_base_gem_for_pro_in_gemfile) + + gemfile_content = File.read(gemfile_path) + expect(gemfile_content).to include('gem "react_on_rails_pro", "~> 16.0", git: "https://example.invalid/repo.git"') + end + it "adds the default version when the user has no version pin" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" From 6523a33916126d2052e93b96968310a36c8d487d Mon Sep 17 00:00:00 2001 From: ihabadham Date: Fri, 15 May 2026 23:58:32 +0300 Subject: [PATCH 31/39] fix pro migration module specifier rewrites --- .../react_on_rails/pro_generator.rb | 61 +++++++++++++++-- .../generators/react_on_rails/pro_setup.rb | 4 +- react_on_rails/lib/react_on_rails/doctor.rb | 47 +++++++++++--- .../spec/lib/react_on_rails/doctor_spec.rb | 48 ++++++++++++++ .../generators/pro_generator_spec.rb | 65 ++++++++++++++++++- 5 files changed, 209 insertions(+), 16 deletions(-) diff --git a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb index 2a6d5bd405..09a7c611c6 100644 --- a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb +++ b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb @@ -366,10 +366,29 @@ def js_files_for_import_update (?=(?:["']|/)) }x + # Explicit allowlist of documented Jest/Vitest APIs whose first argument is a module specifier. + # Keep destructive rewrites narrow; the doctor can warn more broadly if needed. + JEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ + createMockFromModule + mock unmock deepUnmock doMock dontMock setMock + requireActual requireMock unstable_mockModule unstable_unmockModule + ].freeze + VITEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ + mock unmock doMock doUnmock + importActual importMock + ].freeze + JEST_MODULE_SPECIFIER_METHOD_PATTERN = Regexp.union(JEST_MODULE_SPECIFIER_METHOD_NAMES) + VITEST_MODULE_SPECIFIER_METHOD_PATTERN = Regexp.union(VITEST_MODULE_SPECIFIER_METHOD_NAMES) + MOCK_CALL_PATTERN = %r{ (? - (?\s*)? \s*\(\s* ) (?["']) @@ -582,15 +601,43 @@ def unclosed_block_comment_starts?(line) comment_balance.positive? end + MODULE_SPECIFIER_CALL_START_PATTERN = / + (?\s*)? + \s*\( + /x + + MODULE_SPECIFIER_CALL_WITH_STRING_PATTERN = %r{ + (?\s*)? + \s*\(\s* + (?:/\*[^*]*\*+(?:[^/*][^*]*\*+)*/\s*)* + ["'] + }x + def starts_pending_multiline_module_call?(line) line_without_literals = line_without_string_literals_and_inline_comments(line) - return false unless line_without_literals.match?(/(?["'])react-on-rails(?!-pro)(?=(?:["']|/))}) do + line.sub(%r{(?["'])react-on-rails(?!-pro)(?=(?:["']|/))}) do "#{Regexp.last_match[:quote]}react-on-rails-pro" end end @@ -599,6 +646,7 @@ def update_pending_multiline_module_call_tracking(line, pending_depth) if pending_depth.positive? rewritten_line = rewrite_pending_module_specifier(line) updated_depth = pending_depth + module_call_parenthesis_delta(rewritten_line) + updated_depth = 0 if rewritten_line != line updated_depth = 0 if updated_depth <= 0 [rewritten_line, updated_depth] elsif starts_pending_multiline_module_call?(line) @@ -637,7 +685,8 @@ def starts_pending_multiline_static_import_specifier?(line) def module_call_parenthesis_delta(line, from_module_call_start: false) line_without_literals = line_without_string_literals_and_inline_comments(line) line_to_measure = if from_module_call_start - line_without_literals.sub(/\A.*?(?\s*)? | (?:importActual|importMock) ) @@ -2946,15 +2967,25 @@ def check_base_package_references def files_with_base_package_references(source_path) # Keep in sync with js_files_for_import_update in the Pro generator: the # doctor must scan every file type the migration rewriter can modify. - js_extensions = %w[js jsx ts tsx mjs cjs vue svelte] # **/*.ts naturally matches *.d.ts declaration files because they end in .ts. - js_patterns = js_extensions.map { |ext| "#{source_path}/**/*.#{ext}" } + js_patterns = base_package_reference_source_paths(source_path).flat_map do |source_root| + BASE_PACKAGE_REFERENCE_EXTENSIONS.map { |ext| "#{source_root}/**/*.#{ext}" } + end js_patterns.flat_map do |pattern| Dir.glob(pattern) .reject { |file| file.include?("/node_modules/") } .select { |file| base_package_reference_file?(file) } - end.sort + end.uniq.sort + end + + def base_package_reference_source_paths(source_path) + ([source_path] + BASE_PACKAGE_REFERENCE_SOURCE_ROOTS) + .compact + .map(&:to_s) + .reject(&:empty?) + .uniq + .select { |path| Dir.exist?(path) } end def base_package_reference_file?(file) diff --git a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb index 64f96cfd60..0b24f7be92 100644 --- a/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb +++ b/react_on_rails/spec/lib/react_on_rails/doctor_spec.rb @@ -3176,6 +3176,25 @@ class << self end end + context "when JS tests use a typed Jest mock for the base package after a Pro migration" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/app.test.ts", + "jest.mock('react-on-rails', () => ({}));\n") + example.run + end + end + end + + it "reports warning" do + doctor.send(:check_base_package_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("app.test.ts") }).to be true + end + end + context "when JS tests mock the Pro package after a Pro migration" do around do |example| Dir.mktmpdir do |tmpdir| @@ -3277,10 +3296,15 @@ class << self Dir.chdir(tmpdir) do FileUtils.mkdir_p("app/javascript/packs") { + "create-mock.test.ts" => "jest.createMockFromModule('react-on-rails');\n", "unmock.test.ts" => "jest.unmock('react-on-rails');\n", + "deep-unmock.test.ts" => "jest.deepUnmock('react-on-rails');\n", "do-mock.test.ts" => "jest.doMock('react-on-rails/client', () => ({}));\n", "do-unmock.test.ts" => "vi.doUnmock('react-on-rails');\n", "dont-mock.test.ts" => "jest.dontMock('react-on-rails');\n", + "set-mock.test.ts" => "jest.setMock('react-on-rails', {});\n", + "unstable-mock-module.test.ts" => "jest.unstable_mockModule('react-on-rails', () => ({}));\n", + "unstable-unmock-module.test.ts" => "jest.unstable_unmockModule('react-on-rails');\n", "require-actual.test.ts" => "const mod = jest.requireActual('react-on-rails');\n", "require-mock.test.ts" => "const mod = jest.requireMock('react-on-rails/client');\n", "vitest-mock.test.ts" => "vi.mock('react-on-rails', () => ({ authenticityHeaders: vi.fn() }));\n" @@ -3295,10 +3319,15 @@ class << self it "reports warning for each stale helper reference" do doctor.send(:check_base_package_references) warning_content = checker.messages.select { |m| m[:type] == :warning }.map { |m| m[:content] }.join("\n") + expect(warning_content).to include("create-mock.test.ts") expect(warning_content).to include("unmock.test.ts") + expect(warning_content).to include("deep-unmock.test.ts") expect(warning_content).to include("do-mock.test.ts") expect(warning_content).to include("do-unmock.test.ts") expect(warning_content).to include("dont-mock.test.ts") + expect(warning_content).to include("set-mock.test.ts") + expect(warning_content).to include("unstable-mock-module.test.ts") + expect(warning_content).to include("unstable-unmock-module.test.ts") expect(warning_content).to include("require-actual.test.ts") expect(warning_content).to include("require-mock.test.ts") expect(warning_content).to include("vitest-mock.test.ts") @@ -3562,6 +3591,25 @@ class << self end end + context "when a generator-scanned client root contains stale base package references" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("client/app/packs") + File.write("client/app/packs/app.test.ts", + "jest.mock('react-on-rails', () => ({}));\n") + example.run + end + end + end + + it "reports warning for the same root the Pro generator rewrites" do + doctor.send(:check_base_package_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("client/app/packs/app.test.ts") }).to be true + end + end + context "when Shakapacker source_path is custom (e.g. client/app)" do around do |example| Dir.mktmpdir do |tmpdir| diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb index d0ea2c3806..a39982810e 100644 --- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb +++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb @@ -78,6 +78,22 @@ expect(Process).not_to have_received(:spawn) expect(generator.send(:pro_gem_installed?)).to be true end + + specify "missing_pro_gem? also defers for parenthesized declarations with leading comments" do + File.write(gemfile_path, <<~RUBY) + source "https://rubygems.org" + gem( + # pinned for compatibility + "react_on_rails", + "~> 16.0", + require: false + ) + RUBY + + expect(generator.send(:missing_pro_gem?, force: true)).to be false + expect(Process).not_to have_received(:spawn) + expect(generator.send(:pro_gem_installed?)).to be true + end end describe "#run_generator" do @@ -1231,10 +1247,15 @@ it "rewrites the full set of Jest mock helpers" do source = <<~JS + jest.createMockFromModule('react-on-rails'); jest.unmock('react-on-rails'); + jest.deepUnmock('react-on-rails'); jest.doMock('react-on-rails/client'); vi.doUnmock('react-on-rails'); jest.dontMock('react-on-rails'); + jest.setMock('react-on-rails', {}); + jest.unstable_mockModule('react-on-rails', () => ({})); + jest.unstable_unmockModule('react-on-rails'); const a = jest.requireActual('react-on-rails'); const b = jest.requireMock('react-on-rails/client'); vi.importMock('react-on-rails'); @@ -1242,10 +1263,52 @@ rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) - expect(rewritten.scan("react-on-rails-pro").size).to eq(7) + expect(rewritten.scan("react-on-rails-pro").size).to eq(12) expect(rewritten).not_to match(/['"]react-on-rails['"]/) end + it "rewrites multiline Jest and Vitest mock helper calls" do + source = <<~JS + jest.mock( + 'react-on-rails', + () => ({ authenticityHeaders: jest.fn() }) + ); + const actual = await vi.importActual( + 'react-on-rails/client' + ); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to include("'react-on-rails-pro',") + expect(rewritten).to include("'react-on-rails-pro/client'") + expect(rewritten).not_to match(/['"]react-on-rails['"]/) + end + + it "does not rewrite non-specifier strings inside multiline mock factories" do + source = <<~JS + jest.mock( + 'react-on-rails', + () => ({ packageName: 'react-on-rails' }) + ); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to include("'react-on-rails-pro',") + expect(rewritten).to include("packageName: 'react-on-rails'") + end + + it "rewrites typed Jest mock helper module specifiers" do + source = "jest.mock('react-on-rails', () => ({}));\n" + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to eq( + "jest.mock('react-on-rails-pro', () => ({}));\n" + ) + end + it "does not rewrite bare or non-jest/vi importActual/importMock calls" do # importActual/importMock are only `vi.importActual` / `vi.importMock` in Vitest; # there is no `import { importActual } from 'vitest'`. A bare or alien-receiver From f5ae39063b5e7f2edb0855dc01dfd8a5a1875153 Mon Sep 17 00:00:00 2001 From: ihabadham Date: Sat, 16 May 2026 00:40:11 +0300 Subject: [PATCH 32/39] refactor pro migration shared handling --- .../react_on_rails/pro_generator.rb | 185 ++++-------------- .../generators/react_on_rails/pro_setup.rb | 5 +- react_on_rails/lib/react_on_rails/doctor.rb | 22 +-- .../lib/react_on_rails/pro_migration.rb | 169 ++++++++++++++++ .../generators/pro_generator_spec.rb | 15 ++ 5 files changed, 235 insertions(+), 161 deletions(-) create mode 100644 react_on_rails/lib/react_on_rails/pro_migration.rb diff --git a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb index 09a7c611c6..2fb0b3831d 100644 --- a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb +++ b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb @@ -7,6 +7,7 @@ require_relative "generator_messages" require_relative "js_dependency_manager" require_relative "pro_setup" +require "react_on_rails/pro_migration" module ReactOnRails module Generators @@ -103,12 +104,10 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) end gemfile_content = File.read(gemfile_path) - pro_gem_pattern = /^\s*gem(?:\s+|\(\s*(?:#.*\n\s*)*)["']react_on_rails_pro["']/ - base_gem_pattern = /^(\s*)gem(?:\s+|\(\s*)(["'])react_on_rails\2(?=\s*(?:,|\)|#|$))/ - - has_pro_gem_entry = gemfile_content.match?(pro_gem_pattern) + has_pro_gem_entry = ReactOnRails::ProMigration.pro_gem_entry?(gemfile_content) had_pro_gem_entry_before_prerequisites = - original_gemfile_content_for_rollback&.match?(pro_gem_pattern) + original_gemfile_content_for_rollback && + ReactOnRails::ProMigration.pro_gem_entry?(original_gemfile_content_for_rollback) gemfile_lines = gemfile_content.lines updated_lines = [] pro_entry_added = has_pro_gem_entry @@ -117,29 +116,9 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) while line_index < gemfile_lines.length line = gemfile_lines[line_index] - multiline_parenthesized_match = match_multiline_parenthesized_base_gem(gemfile_lines, line_index) - - if multiline_parenthesized_match - base_gem_entry_found = true - unless pro_entry_added - indentation = multiline_parenthesized_match[:indentation] - quote = multiline_parenthesized_match[:quote] - updated_lines << build_pro_gem_replacement_line( - indentation: indentation, - quote: quote, - suffix: multiline_parenthesized_match[:trailing_suffix], - parenthesized_gem_call: true - ) - pro_entry_added = true - end - - line_index = multiline_parenthesized_match[:next_index] - next - end - - match = line.match(base_gem_pattern) + base_gem_declaration = ReactOnRails::ProMigration.base_gem_declaration_at(gemfile_lines, line_index) - unless match + unless base_gem_declaration updated_lines << line line_index += 1 next @@ -147,25 +126,17 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) base_gem_entry_found = true - declaration = consume_non_parenthesized_base_gem_declaration( - gemfile_lines, - line_index, - match.end(0) - ) - unless pro_entry_added - indentation = match[1] - quote = match[2] updated_lines << build_pro_gem_replacement_line( - indentation: indentation, - quote: quote, - suffix: declaration[:trailing_suffix], - parenthesized_gem_call: match[0].include?("(") + indentation: base_gem_declaration[:indentation], + quote: base_gem_declaration[:quote], + suffix: base_gem_declaration[:trailing_suffix], + parenthesized_gem_call: base_gem_declaration[:parenthesized_gem_call] ) pro_entry_added = true end - line_index = declaration[:next_index] + line_index = base_gem_declaration[:next_index] end updated_content = updated_lines.join @@ -329,8 +300,8 @@ def update_imports_to_pro_package end def js_files_for_import_update - js_extensions = %w[js jsx ts tsx mjs cjs vue svelte].join(",") - %w[app/javascript app/frontend frontend javascript client].flat_map do |root| + js_extensions = ReactOnRails::ProMigration::JS_SOURCE_EXTENSIONS.join(",") + ReactOnRails::ProMigration::JS_SOURCE_ROOTS.flat_map do |root| root_path = File.join(destination_root, root) next [] unless Dir.exist?(root_path) @@ -368,17 +339,8 @@ def js_files_for_import_update # Explicit allowlist of documented Jest/Vitest APIs whose first argument is a module specifier. # Keep destructive rewrites narrow; the doctor can warn more broadly if needed. - JEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ - createMockFromModule - mock unmock deepUnmock doMock dontMock setMock - requireActual requireMock unstable_mockModule unstable_unmockModule - ].freeze - VITEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ - mock unmock doMock doUnmock - importActual importMock - ].freeze - JEST_MODULE_SPECIFIER_METHOD_PATTERN = Regexp.union(JEST_MODULE_SPECIFIER_METHOD_NAMES) - VITEST_MODULE_SPECIFIER_METHOD_PATTERN = Regexp.union(VITEST_MODULE_SPECIFIER_METHOD_NAMES) + JEST_MODULE_SPECIFIER_METHOD_PATTERN = ReactOnRails::ProMigration::JEST_MODULE_SPECIFIER_METHOD_PATTERN + VITEST_MODULE_SPECIFIER_METHOD_PATTERN = ReactOnRails::ProMigration::VITEST_MODULE_SPECIFIER_METHOD_PATTERN MOCK_CALL_PATTERN = %r{ (? @@ -414,30 +376,17 @@ def js_files_for_import_update def rewrite_react_on_rails_module_specifiers(content) rewrite_non_comment_lines(content) do |line| rewrite_outside_inline_template_literals(line) do |line_without_templates| - BASE_PACKAGE_REWRITE_PATTERNS.reduce(line_without_templates) do |result, pattern| - result.gsub(pattern) do - "#{Regexp.last_match[:prefix]}#{Regexp.last_match[:quote]}react-on-rails-pro" - end - end + rewrite_base_package_patterns(line_without_templates) end end end - def line_continues_with_comma?(line) - line_without_comment = line.sub(/\s*#.*$/, "").rstrip - line_without_comment.end_with?(",") - end - - def gem_declaration_continues_on_next_line?(line) - stripped = line.lstrip - return true if stripped.empty? - - !stripped.match?(/\Agem(?:\s|\()/) - end - - def comment_or_blank_line?(line) - stripped = line.lstrip - stripped.empty? || stripped.start_with?("#") + def rewrite_base_package_patterns(line) + BASE_PACKAGE_REWRITE_PATTERNS.reduce(line) do |result, pattern| + result.gsub(pattern) do + "#{Regexp.last_match[:prefix]}#{Regexp.last_match[:quote]}react-on-rails-pro" + end + end end def add_missing_gemfile_warning(gemfile_path) @@ -636,10 +585,30 @@ def starts_pending_multiline_module_call?(line) !line.match?(MODULE_SPECIFIER_CALL_WITH_STRING_PATTERN) end + PENDING_MODULE_SPECIFIER_PATTERN = %r{(?["'])react-on-rails(?!-pro)(?=(?:["']|/))} + def rewrite_pending_module_specifier(line) - line.sub(%r{(?["'])react-on-rails(?!-pro)(?=(?:["']|/))}) do + match = line.match(PENDING_MODULE_SPECIFIER_PATTERN) + return line unless match + + rewritten_line = line.sub(PENDING_MODULE_SPECIFIER_PATTERN) do "#{Regexp.last_match[:quote]}react-on-rails-pro" end + + rewrite_statement_suffix_after_pending_module_specifier(rewritten_line, match) + end + + def rewrite_statement_suffix_after_pending_module_specifier(line, pending_match) + closing_quote_index = line.index(pending_match[:quote], pending_match.end(0)) + return line unless closing_quote_index + + suffix = line[(closing_quote_index + 1)..].to_s + separator_match = suffix.match(/\A(?\s*;\s*)/) + return line unless separator_match + + suffix_code = suffix[separator_match.end(0)..].to_s + rewritten_suffix_code = rewrite_base_package_patterns(suffix_code) + "#{line[0..closing_quote_index]}#{separator_match[:separator]}#{rewritten_suffix_code}" end def update_pending_multiline_module_call_tracking(line, pending_depth) @@ -892,74 +861,6 @@ def next_quote_state(current_state, char, line, index) nil end - # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity - def match_multiline_parenthesized_base_gem(lines, start_index) - start_line = lines[start_index] - start_match = start_line.match(/^(\s*)gem\s*\(/) - return nil unless start_match - - line_index = start_index - found_base_gem_name = false - base_gem_quote = nil - gem_name_line_index = nil - gem_name_match_end = nil - paren_depth = 0 - - while line_index < lines.length - line = lines[line_index] - line_without_comment = line.sub(/\s*#.*$/, "") - line_without_literals = line_without_string_literals_and_inline_comments(line, strip_ruby_comments: true) - - if !found_base_gem_name && - (gem_name_match = line_without_comment.match(/(["'])react_on_rails\1(?=\s*(?:,|\)|#|$))/)) - found_base_gem_name = true - base_gem_quote = gem_name_match[1] - gem_name_line_index = line_index - gem_name_match_end = gem_name_match.end(0) - end - - paren_depth += line_without_literals.count("(") - line_without_literals.count(")") - - if paren_depth <= 0 - return nil unless found_base_gem_name - - declaration_fragment = lines[gem_name_line_index..line_index].join - suffix = declaration_fragment[gem_name_match_end..] - suffix = "\n" if suffix.nil? || suffix.empty? - return { - indentation: start_match[1], - quote: base_gem_quote, - next_index: line_index + 1, - trailing_suffix: suffix - } - end - - line_index += 1 - end - - nil - end - # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity - - def consume_non_parenthesized_base_gem_declaration(lines, start_index, match_end) - line_index = start_index - current_line = lines[line_index] - declaration_lines = [current_line] - line_index += 1 - - while line_index < lines.length && - line_continues_with_comma?(current_line) && - gem_declaration_continues_on_next_line?(lines[line_index]) - next_line = lines[line_index] - declaration_lines << next_line - current_line = next_line unless comment_or_blank_line?(next_line) - line_index += 1 - end - - trailing_suffix = lines[start_index][match_end..].to_s + declaration_lines.drop(1).join - { trailing_suffix: trailing_suffix, next_index: line_index } - end - def build_pro_gem_replacement_line(indentation:, quote:, suffix:, parenthesized_gem_call: false) normalized_suffix = suffix || "\n" normalized_suffix = "#{normalized_suffix}\n" unless normalized_suffix.end_with?("\n") diff --git a/react_on_rails/lib/generators/react_on_rails/pro_setup.rb b/react_on_rails/lib/generators/react_on_rails/pro_setup.rb index 7e9199cd77..df1c25e898 100644 --- a/react_on_rails/lib/generators/react_on_rails/pro_setup.rb +++ b/react_on_rails/lib/generators/react_on_rails/pro_setup.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative "generator_messages" +require "react_on_rails/pro_migration" module ReactOnRails module Generators @@ -549,9 +550,7 @@ def base_react_on_rails_gem_in_gemfile? gemfile_path = File.join(destination_root, "Gemfile") return false unless File.exist?(gemfile_path) - File.read(gemfile_path).match?( - /^\s*gem(?:\s+|\(\s*(?:#.*\n\s*)*)["']react_on_rails["'](?=\s*(?:,|\)|#|$))/ - ) + ReactOnRails::ProMigration.base_gem_entry?(File.read(gemfile_path)) rescue SystemCallError, IOError false end diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 391c680381..64ac26be6c 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -9,6 +9,7 @@ require_relative "version_syntax_converter" require_relative "version_synchronizer" require_relative "system_checker" +require_relative "pro_migration" begin require "rainbow" @@ -2882,23 +2883,13 @@ def renderer_cache_migration_suggestion(path) BASE_PACKAGE_REQUIRE_PATTERN = %r{\brequire\s*\(\s*['"]react-on-rails(?:/[^'"]*)?['"]\s*\)} BASE_PACKAGE_DYNAMIC_IMPORT_PATTERN = %r{\bimport\s*\(\s*['"]react-on-rails(?:/[^'"]*)?['"]\s*\)} BASE_PACKAGE_SIDE_EFFECT_IMPORT_PATTERN = %r{^\s*import\s+['"]react-on-rails(?:/[^'"]*)?['"]} - BASE_PACKAGE_REFERENCE_SOURCE_ROOTS = %w[app/javascript app/frontend frontend javascript client].freeze - BASE_PACKAGE_REFERENCE_EXTENSIONS = %w[js jsx ts tsx mjs cjs vue svelte].freeze + BASE_PACKAGE_REFERENCE_SOURCE_ROOTS = ReactOnRails::ProMigration::JS_SOURCE_ROOTS + BASE_PACKAGE_REFERENCE_EXTENSIONS = ReactOnRails::ProMigration::JS_SOURCE_EXTENSIONS # Explicit allowlist of documented Jest/Vitest APIs whose first argument is a module specifier. - # Keep in sync with the Pro generator's destructive rewrite allowlist. - BASE_PACKAGE_JEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ - createMockFromModule - mock unmock deepUnmock doMock dontMock setMock - requireActual requireMock unstable_mockModule unstable_unmockModule - ].freeze - BASE_PACKAGE_VITEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ - mock unmock doMock doUnmock - importActual importMock - ].freeze BASE_PACKAGE_JEST_MODULE_SPECIFIER_METHOD_PATTERN = - Regexp.union(BASE_PACKAGE_JEST_MODULE_SPECIFIER_METHOD_NAMES) + ReactOnRails::ProMigration::JEST_MODULE_SPECIFIER_METHOD_PATTERN BASE_PACKAGE_VITEST_MODULE_SPECIFIER_METHOD_PATTERN = - Regexp.union(BASE_PACKAGE_VITEST_MODULE_SPECIFIER_METHOD_NAMES) + ReactOnRails::ProMigration::VITEST_MODULE_SPECIFIER_METHOD_PATTERN # Match known Jest/Vitest module-specifier helpers. Aliased or nested receivers # are intentionally out of scope to avoid warning on arbitrary application methods. # @@ -2965,8 +2956,7 @@ def check_base_package_references end def files_with_base_package_references(source_path) - # Keep in sync with js_files_for_import_update in the Pro generator: the - # doctor must scan every file type the migration rewriter can modify. + # Scan every file type the Pro migration rewriter can modify. # **/*.ts naturally matches *.d.ts declaration files because they end in .ts. js_patterns = base_package_reference_source_paths(source_path).flat_map do |source_root| BASE_PACKAGE_REFERENCE_EXTENSIONS.map { |ext| "#{source_root}/**/*.#{ext}" } diff --git a/react_on_rails/lib/react_on_rails/pro_migration.rb b/react_on_rails/lib/react_on_rails/pro_migration.rb new file mode 100644 index 0000000000..6bddcaa90d --- /dev/null +++ b/react_on_rails/lib/react_on_rails/pro_migration.rb @@ -0,0 +1,169 @@ +# frozen_string_literal: true + +module ReactOnRails + # Shared Pro migration facts and Gemfile parsing used by the generator and doctor. + module ProMigration + JS_SOURCE_ROOTS = %w[app/javascript app/frontend frontend javascript client].freeze + JS_SOURCE_EXTENSIONS = %w[js jsx ts tsx mjs cjs vue svelte].freeze + + JEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ + createMockFromModule + mock unmock deepUnmock doMock dontMock setMock + requireActual requireMock unstable_mockModule unstable_unmockModule + ].freeze + VITEST_MODULE_SPECIFIER_METHOD_NAMES = %w[ + mock unmock doMock doUnmock + importActual importMock + ].freeze + JEST_MODULE_SPECIFIER_METHOD_PATTERN = Regexp.union(JEST_MODULE_SPECIFIER_METHOD_NAMES) + VITEST_MODULE_SPECIFIER_METHOD_PATTERN = Regexp.union(VITEST_MODULE_SPECIFIER_METHOD_NAMES) + + PRO_GEM_PATTERN = /^\s*gem(?:\s+|\(\s*(?:#.*\n\s*)*)["']react_on_rails_pro["']/ + BASE_GEM_PATTERN = /^(\s*)gem(?:\s+|\(\s*)(["'])react_on_rails\2(?=\s*(?:,|\)|#|$))/ + + module_function + + def pro_gem_entry?(gemfile_content) + gemfile_content.match?(PRO_GEM_PATTERN) + end + + def base_gem_entry?(gemfile_content) + gemfile_lines = gemfile_content.lines + line_index = 0 + + while line_index < gemfile_lines.length + return true if base_gem_declaration_at(gemfile_lines, line_index) + + line_index += 1 + end + + false + end + + def base_gem_declaration_at(lines, start_index) + match_multiline_parenthesized_base_gem(lines, start_index) || + match_non_parenthesized_base_gem(lines, start_index) + end + + def match_multiline_parenthesized_base_gem(lines, start_index) + start_line = lines[start_index] + start_match = start_line.match(/^(\s*)gem\s*\(/) + return nil unless start_match + + line_index = start_index + gem_name = nil + paren_depth = 0 + + while line_index < lines.length + line = lines[line_index] + gem_name_fragment_offset = line_index == start_index ? start_match.end(0) : 0 + gem_name ||= parenthesized_base_gem_name(line, gem_name_fragment_offset, line_index) + return nil if gem_name == false + + line_without_literals = line_without_string_literals_and_inline_comments(line, strip_ruby_comments: true) + paren_depth += parenthesis_delta(line_without_literals) + + return parenthesized_base_gem_declaration(lines, start_match, gem_name, line_index) if paren_depth <= 0 + + line_index += 1 + end + + nil + end + + def parenthesized_base_gem_name(line, fragment_offset, line_index) + gem_name_fragment = line[fragment_offset..].to_s + return nil if gem_name_fragment.sub(/\s*#.*$/, "").strip.empty? + + gem_name_match = gem_name_fragment.match(/\A\s*(["'])react_on_rails\1(?=\s*(?:,|\)|#|$))/) + return false unless gem_name_match + + { + quote: gem_name_match[1], + line_index: line_index, + match_end: fragment_offset + gem_name_match.end(0) + } + end + + def parenthesized_base_gem_declaration(lines, start_match, gem_name, end_line_index) + return nil unless gem_name + + declaration_fragment = lines[gem_name[:line_index]..end_line_index].join + suffix = declaration_fragment[gem_name[:match_end]..] + suffix = "\n" if suffix.nil? || suffix.empty? + { + indentation: start_match[1], + quote: gem_name[:quote], + next_index: end_line_index + 1, + trailing_suffix: suffix, + parenthesized_gem_call: true + } + end + + def parenthesis_delta(line) + line.count("(") - line.count(")") + end + + def match_non_parenthesized_base_gem(lines, start_index) + line = lines[start_index] + match = line.match(BASE_GEM_PATTERN) + return nil unless match + + declaration = consume_non_parenthesized_base_gem_declaration(lines, start_index, match.end(0)) + { + indentation: match[1], + quote: match[2], + next_index: declaration[:next_index], + trailing_suffix: declaration[:trailing_suffix], + parenthesized_gem_call: match[0].include?("(") + } + end + + def consume_non_parenthesized_base_gem_declaration(lines, start_index, match_end) + line_index = start_index + current_line = lines[line_index] + declaration_lines = [current_line] + line_index += 1 + + while line_index < lines.length && + line_continues_with_comma?(current_line) && + gem_declaration_continues_on_next_line?(lines[line_index]) + next_line = lines[line_index] + declaration_lines << next_line + current_line = next_line unless comment_or_blank_line?(next_line) + line_index += 1 + end + + trailing_suffix = lines[start_index][match_end..].to_s + declaration_lines.drop(1).join + { trailing_suffix: trailing_suffix, next_index: line_index } + end + + def line_continues_with_comma?(line) + line_without_comment = line.sub(/\s*#.*$/, "").rstrip + line_without_comment.end_with?(",") + end + + def gem_declaration_continues_on_next_line?(line) + stripped = line.lstrip + return true if stripped.empty? + + !stripped.match?(/\Agem(?:\s|\()/) + end + + def comment_or_blank_line?(line) + stripped = line.lstrip + stripped.empty? || stripped.start_with?("#") + end + + def line_without_string_literals_and_inline_comments(line, strip_ruby_comments: false) + line_without_strings = line.gsub( + /"(?:\\.|[^"\\])*"|'(?:\\.|[^'\\])*'|`(?:\\.|[^`\\])*`/, + "" + ) + line_without_comments = line_without_strings.sub(%r{//.*$}, "") + return line_without_comments unless strip_ruby_comments + + line_without_comments.sub(/\s*#.*$/, "") + end + end +end diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb index a39982810e..33e578b45f 100644 --- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb +++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb @@ -593,6 +593,21 @@ expect(generator).not_to have_received(:bundle_install_after_gem_swap) end + it "does not replace other parenthesized gem declarations that reference react_on_rails in options" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem("other_gem", require: "react_on_rails") + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + original_content = File.read(gemfile_path) + result = generator.send(:swap_base_gem_for_pro_in_gemfile) + + expect(result).to be(false) + expect(File.read(gemfile_path)).to eq(original_content) + expect(generator).not_to have_received(:bundle_install_after_gem_swap) + end + it "warns when Gemfile is missing" do allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with(gemfile_path).and_return(false) From 48815a34667c2abb9024b1238331dc665b380124 Mon Sep 17 00:00:00 2001 From: ihabadham Date: Sat, 16 May 2026 01:41:35 +0300 Subject: [PATCH 33/39] Avoid regex ambiguity in Pro migration parsing --- .../lib/react_on_rails/pro_migration.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/pro_migration.rb b/react_on_rails/lib/react_on_rails/pro_migration.rb index 6bddcaa90d..814390a66f 100644 --- a/react_on_rails/lib/react_on_rails/pro_migration.rb +++ b/react_on_rails/lib/react_on_rails/pro_migration.rb @@ -20,6 +20,8 @@ module ProMigration PRO_GEM_PATTERN = /^\s*gem(?:\s+|\(\s*(?:#.*\n\s*)*)["']react_on_rails_pro["']/ BASE_GEM_PATTERN = /^(\s*)gem(?:\s+|\(\s*)(["'])react_on_rails\2(?=\s*(?:,|\)|#|$))/ + RUBY_INLINE_COMMENT_PATTERN = /(? Date: Sat, 16 May 2026 02:07:48 +0300 Subject: [PATCH 34/39] fix(doctor): rescue IOError in base_package_reference_file? for sibling parity base_react_on_rails_gem_in_gemfile? already rescues SystemCallError, IOError; this method only rescued SystemCallError. File.binread can raise IOError on some platforms (interrupted/!broken reads), which would abort the whole base package scan instead of skipping the one unreadable file. Align the rescue so behavior is consistent with the sibling. Co-Authored-By: Claude Opus 4.7 (1M context) --- react_on_rails/lib/react_on_rails/doctor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 64ac26be6c..c8d7e64a25 100644 --- a/react_on_rails/lib/react_on_rails/doctor.rb +++ b/react_on_rails/lib/react_on_rails/doctor.rb @@ -2983,7 +2983,7 @@ def base_package_reference_file?(file) return false unless content.valid_encoding? base_package_reference?(content) - rescue SystemCallError + rescue SystemCallError, IOError false end From d34199a0623a24d3681114f4a612fb4ac1f83473 Mon Sep 17 00:00:00 2001 From: ihabadham Date: Sat, 16 May 2026 02:07:51 +0300 Subject: [PATCH 35/39] chore(generator): track the CQS-smell refactor in an issue, not an in-code note The nine-line design proposal in mark_pro_gem_installed!'s TODO ages poorly under git blame and drifts from the code. Move the detail to a tracked issue and trim the comment to a one-line reference. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../lib/generators/react_on_rails/generator_helper.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/react_on_rails/lib/generators/react_on_rails/generator_helper.rb b/react_on_rails/lib/generators/react_on_rails/generator_helper.rb index c8b498d4bf..b2ec5ddafe 100644 --- a/react_on_rails/lib/generators/react_on_rails/generator_helper.rb +++ b/react_on_rails/lib/generators/react_on_rails/generator_helper.rb @@ -147,15 +147,7 @@ def pro_gem_installed? @pro_gem_installed = Gem.loaded_specs.key?("react_on_rails_pro") || gem_in_lockfile?("react_on_rails_pro") end - # TODO: CQS smell — `mark_pro_gem_installed!` lets `pro_gem_installed?` return - # true before the gem is actually in the lockfile. Called from both - # `attempt_pro_gem_auto_install` (after `bundle add` succeeds but before bundle - # install) and `defer_pro_gem_install_to_gemfile_swap` (before the Gemfile is - # rewritten and bundle install runs). Callers can't distinguish "really - # installed" from "scheduled to be installed by this generator run". Refactor: - # keep `pro_gem_installed?` truthful (real lockfile/loaded check only) and add - # a separate `@pro_gem_install_deferred` flag that callers needing the broader - # meaning consult explicitly. + # TODO: CQS smell: mark_pro_gem_installed! makes pro_gem_installed? return true before install. See #3303. def mark_pro_gem_installed! @pro_gem_installed = true end From 7b8426b44ffc3dcdb7c59bbe65e0d20278abf889 Mon Sep 17 00:00:00 2001 From: ihabadham Date: Sat, 16 May 2026 02:37:18 +0300 Subject: [PATCH 36/39] fix(pro_migration): strip Ruby inline comments without a regex The negative-lookbehind /(? --- .../lib/react_on_rails/pro_migration.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/react_on_rails/lib/react_on_rails/pro_migration.rb b/react_on_rails/lib/react_on_rails/pro_migration.rb index 814390a66f..55f5df5c48 100644 --- a/react_on_rails/lib/react_on_rails/pro_migration.rb +++ b/react_on_rails/lib/react_on_rails/pro_migration.rb @@ -20,7 +20,6 @@ module ProMigration PRO_GEM_PATTERN = /^\s*gem(?:\s+|\(\s*(?:#.*\n\s*)*)["']react_on_rails_pro["']/ BASE_GEM_PATTERN = /^(\s*)gem(?:\s+|\(\s*)(["'])react_on_rails\2(?=\s*(?:,|\)|#|$))/ - RUBY_INLINE_COMMENT_PATTERN = /(? Date: Sat, 16 May 2026 19:24:36 +0300 Subject: [PATCH 37/39] fix pro gem swap for postfix guards --- .../react_on_rails/pro_generator.rb | 61 ++++++++++++++++++- .../generators/pro_generator_spec.rb | 47 ++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb index 2fb0b3831d..e1e103921b 100644 --- a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb +++ b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb @@ -372,6 +372,7 @@ def js_files_for_import_update MOCK_CALL_PATTERN, DECLARE_MODULE_PATTERN ].freeze + GEMFILE_STRING_DELIMITERS = ["'", '"', "`"].freeze def rewrite_react_on_rails_module_specifiers(content) rewrite_non_comment_lines(content) do |line| @@ -868,12 +869,70 @@ def build_pro_gem_replacement_line(indentation:, quote:, suffix:, parenthesized_ has_user_version_pin = normalized_suffix.match?(/\A\s*,\s*(?:#[^\n]*\n\s*)*["']/) version_arg = has_user_version_pin ? "" : ", #{quote}#{pro_gem_version_requirement}#{quote}" + if parenthesized_gem_call + normalized_suffix = remove_parenthesized_gem_call_closing_parenthesis(normalized_suffix) + end normalized_suffix = "\n" if normalized_suffix.match?(/\A,\s*\n\z/) - normalized_suffix = normalized_suffix.sub(/\)(\s*(?:#.*)?\n)\z/, '\1') if parenthesized_gem_call "#{indentation}gem #{quote}react_on_rails_pro#{quote}#{version_arg}#{normalized_suffix}" end + def remove_parenthesized_gem_call_closing_parenthesis(suffix) + closing_index = parenthesized_gem_call_closing_parenthesis_index(suffix) + return suffix unless closing_index + + prefix = suffix[0...closing_index] + rest = suffix[(closing_index + 1)..].to_s + return "#{prefix.rstrip} #{rest.lstrip}" if closing_parenthesis_line_has_postfix_code?(rest) + + "#{prefix}#{rest}" + end + + def closing_parenthesis_line_has_postfix_code?(rest) + stripped_rest = rest.lstrip + !stripped_rest.empty? && !stripped_rest.start_with?("#", "\n", "\r") + end + + # The suffix starts after the gem name but still inside the original `gem(...)` call, + # so the matching call-closing parenthesis is found by starting at depth 1. + def parenthesized_gem_call_closing_parenthesis_index(suffix) + depth = 1 + quote = nil + scan_index = 0 + + while scan_index < suffix.length + char = suffix[scan_index] + + if quote + quote = nil if char == quote && !character_escaped?(suffix, scan_index) + else + scan_index, depth, quote, closing_index = + next_parenthesized_gem_suffix_scan_state(suffix, scan_index, depth) + return closing_index if closing_index + return nil unless scan_index + end + + scan_index += 1 + end + + nil + end + + def next_parenthesized_gem_suffix_scan_state(suffix, scan_index, depth) + char = suffix[scan_index] + return [scan_index, depth, char, nil] if GEMFILE_STRING_DELIMITERS.include?(char) + return [suffix.index("\n", scan_index), depth, nil, nil] if char == "#" + return [scan_index, depth + 1, nil, nil] if char == "(" + return parenthesized_gem_suffix_closing_state(scan_index, depth) if char == ")" + + [scan_index, depth, nil, nil] + end + + def parenthesized_gem_suffix_closing_state(scan_index, depth) + next_depth = depth - 1 + [scan_index, next_depth, nil, next_depth.zero? ? scan_index : nil] + end + def print_success_message route = if File.exist?(File.join(destination_root, "app/controllers/hello_server_controller.rb")) "hello_server" diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb index 33e578b45f..fd7dd9bfdc 100644 --- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb +++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "ripper" require_relative "../support/generator_spec_helper" describe ProGenerator, type: :generator do @@ -7,6 +8,10 @@ destination File.expand_path("../dummy-for-generators", __dir__) + def expect_parseable_ruby(source) + expect(Ripper.sexp(source)).not_to be_nil + end + # Unit tests for prerequisite validation context "when base React on Rails is not installed" do @@ -416,6 +421,48 @@ expect(gemfile_content).not_to include("false))") end + it "replaces parenthesized Gemfile declarations with postfix if guards without leaving trailing parentheses" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem("react_on_rails", "~> 16.0") if ENV["ENABLE_ROR"] + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + generator.send(:swap_base_gem_for_pro_in_gemfile) + + gemfile_content = File.read(gemfile_path) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + gem "react_on_rails_pro", "~> 16.0" if ENV["ENABLE_ROR"] + RUBY + expect_parseable_ruby(gemfile_content) + expect(generator).to have_received(:bundle_install_after_gem_swap) + end + + it "replaces multiline parenthesized Gemfile declarations with postfix unless guards" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem( + "react_on_rails", + "~> 16.0", + require: false + ) unless ENV["SKIP_ROR"] + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + generator.send(:swap_base_gem_for_pro_in_gemfile) + + gemfile_content = File.read(gemfile_path) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + gem "react_on_rails_pro", + "~> 16.0", + require: false unless ENV["SKIP_ROR"] + RUBY + expect_parseable_ruby(gemfile_content) + expect(generator).to have_received(:bundle_install_after_gem_swap) + end + it "replaces multiline parenthesized Gemfile declarations" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" From a134cf505c49d13027838d444c1a3d47310f81ea Mon Sep 17 00:00:00 2001 From: ihabadham Date: Sat, 16 May 2026 23:27:25 +0300 Subject: [PATCH 38/39] fix pro gem swap duplicate declarations --- .../react_on_rails/pro_generator.rb | 20 ++-- .../generators/pro_generator_spec.rb | 105 ++++++++++++++++-- 2 files changed, 106 insertions(+), 19 deletions(-) diff --git a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb index e1e103921b..c7ca307181 100644 --- a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb +++ b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb @@ -110,8 +110,8 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) ReactOnRails::ProMigration.pro_gem_entry?(original_gemfile_content_for_rollback) gemfile_lines = gemfile_content.lines updated_lines = [] - pro_entry_added = has_pro_gem_entry base_gem_entry_found = false + base_gem_entries_preserved = false line_index = 0 while line_index < gemfile_lines.length @@ -126,14 +126,16 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) base_gem_entry_found = true - unless pro_entry_added + if has_pro_gem_entry + updated_lines.concat(gemfile_lines[line_index...base_gem_declaration[:next_index]]) + base_gem_entries_preserved = true + else updated_lines << build_pro_gem_replacement_line( indentation: base_gem_declaration[:indentation], quote: base_gem_declaration[:quote], suffix: base_gem_declaration[:trailing_suffix], parenthesized_gem_call: base_gem_declaration[:parenthesized_gem_call] ) - pro_entry_added = true end line_index = base_gem_declaration[:next_index] @@ -141,6 +143,14 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) updated_content = updated_lines.join if updated_content == gemfile_content + if base_gem_entries_preserved + say( + "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ + "leaving react_on_rails entries unchanged", + :yellow + ) + end + unless base_gem_entry_found || had_pro_gem_entry_before_prerequisites rollback_message = rollback_gemfile_after_failed_swap_precondition( gemfile_path: gemfile_path, @@ -154,10 +164,6 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) return true end - if has_pro_gem_entry - say "ℹ️ Existing react_on_rails_pro Gemfile entry detected; preserving current version constraint", :yellow - end - if options[:pretend] say_status :pretend, "Would replace react_on_rails with react_on_rails_pro in Gemfile", :yellow return true diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb index fd7dd9bfdc..81a091af03 100644 --- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb +++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb @@ -584,27 +584,30 @@ def expect_parseable_ruby(source) expect(gemfile_content).not_to include("gem(") end - it "removes base gem without adding duplicate react_on_rails_pro entries" do - simulate_existing_file("Gemfile", <<~RUBY) + it "leaves base gem entries unchanged when react_on_rails_pro already exists" do + original_gemfile = <<~RUBY source "https://rubygems.org" gem "react_on_rails", "~> 16.0" gem "react_on_rails_pro", "~> 16.0" RUBY + simulate_existing_file("Gemfile", original_gemfile) allow(generator).to receive(:bundle_install_after_gem_swap) allow(generator).to receive(:say) generator.send(:swap_base_gem_for_pro_in_gemfile) - gemfile_content = File.read(gemfile_path) - expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) - expect(gemfile_content.scan(/gem\s+["']react_on_rails_pro["']/).size).to eq(1) + expect(File.read(gemfile_path)).to eq(original_gemfile) expect(generator).to have_received(:say) - .with("ℹ️ Existing react_on_rails_pro Gemfile entry detected; preserving current version constraint", :yellow) - expect(generator).to have_received(:bundle_install_after_gem_swap) + .with( + "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ + "leaving react_on_rails entries unchanged", + :yellow + ) + expect(generator).not_to have_received(:bundle_install_after_gem_swap) end - it "does not add duplicate react_on_rails_pro entries when existing parenthesized declaration has comment lines" do - simulate_existing_file("Gemfile", <<~RUBY) + it "leaves base gem entries unchanged when existing parenthesized pro declaration has comment lines" do + original_gemfile = <<~RUBY source "https://rubygems.org" gem "react_on_rails", "~> 16.0" gem( @@ -613,16 +616,94 @@ def expect_parseable_ruby(source) "~> 16.0" ) RUBY + simulate_existing_file("Gemfile", original_gemfile) allow(generator).to receive(:bundle_install_after_gem_swap) allow(generator).to receive(:say) generator.send(:swap_base_gem_for_pro_in_gemfile) + expect(File.read(gemfile_path)).to eq(original_gemfile) + expect(generator).to have_received(:say) + .with( + "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ + "leaving react_on_rails entries unchanged", + :yellow + ) + expect(generator).not_to have_received(:bundle_install_after_gem_swap) + end + + it "replaces duplicate base gem declarations without dropping grouped declarations" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem "react_on_rails", "~> 16.0" + group :test do + gem "react_on_rails", require: false + end + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + generator.send(:swap_base_gem_for_pro_in_gemfile) + gemfile_content = File.read(gemfile_path) + expected_version = generator.send(:pro_gem_version_requirement) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + gem "react_on_rails_pro", "~> 16.0" + group :test do + gem "react_on_rails_pro", "#{expected_version}", require: false + end + RUBY + expect_parseable_ruby(gemfile_content) + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) + expect(generator).to have_received(:bundle_install_after_gem_swap) + end + + it "replaces base gem declarations in both conditional branches" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + if ENV["ROR_EDGE"] + gem "react_on_rails", github: "shakacode/react_on_rails", branch: "master" + else + gem "react_on_rails", "~> 16.0" + end + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + generator.send(:swap_base_gem_for_pro_in_gemfile) + + gemfile_content = File.read(gemfile_path) + expected_version = generator.send(:pro_gem_version_requirement) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + if ENV["ROR_EDGE"] + gem "react_on_rails_pro", "#{expected_version}", github: "shakacode/react_on_rails", branch: "master" + else + gem "react_on_rails_pro", "~> 16.0" + end + RUBY + expect_parseable_ruby(gemfile_content) + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) + expect(generator).to have_received(:bundle_install_after_gem_swap) + end + + it "replaces duplicate base gem declarations with platform-specific options" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem "react_on_rails", "~> 16.0" + gem "react_on_rails", "~> 16.0", platforms: :jruby + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + generator.send(:swap_base_gem_for_pro_in_gemfile) + + gemfile_content = File.read(gemfile_path) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + gem "react_on_rails_pro", "~> 16.0" + gem "react_on_rails_pro", "~> 16.0", platforms: :jruby + RUBY + expect_parseable_ruby(gemfile_content) expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) - expect(gemfile_content.scan(/["']react_on_rails_pro["']/).size).to eq(1) - expect(generator).to have_received(:say) - .with("ℹ️ Existing react_on_rails_pro Gemfile entry detected; preserving current version constraint", :yellow) expect(generator).to have_received(:bundle_install_after_gem_swap) end From fa798dc5422974b177cad2f944de60fddab8f68f Mon Sep 17 00:00:00 2001 From: ihabadham Date: Sun, 17 May 2026 20:02:27 +0300 Subject: [PATCH 39/39] fix pro gem swap to remove stale base gem when pro entry exists When a react_on_rails_pro entry was already present, the swap left the now-stale base react_on_rails declaration in the Gemfile and skipped bundle install -- re-introducing the exact stale-reference state issue #3104 set out to eliminate, and diverging from the pre-refactor behavior. The swap now drops the base declaration, runs bundle install, and reports the removal. Also chomp the trailing newline when stripping a parenthesized gem call's closing parenthesis so a multiline gem( ... ) declaration no longer leaves a blank line behind its replacement. Specs updated to assert the removed/bundled behavior and a byte-exact no-blank-line result; the duplicate-declaration spec now uses a Bundler-valid Gemfile so it no longer defends behavior with invalid input. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../react_on_rails/pro_generator.rb | 25 ++++--- .../generators/pro_generator_spec.rb | 65 ++++++++++++++----- 2 files changed, 62 insertions(+), 28 deletions(-) diff --git a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb index c7ca307181..a559be2f03 100644 --- a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb +++ b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb @@ -111,7 +111,7 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) gemfile_lines = gemfile_content.lines updated_lines = [] base_gem_entry_found = false - base_gem_entries_preserved = false + base_gem_entries_removed = false line_index = 0 while line_index < gemfile_lines.length @@ -127,8 +127,7 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) base_gem_entry_found = true if has_pro_gem_entry - updated_lines.concat(gemfile_lines[line_index...base_gem_declaration[:next_index]]) - base_gem_entries_preserved = true + base_gem_entries_removed = true else updated_lines << build_pro_gem_replacement_line( indentation: base_gem_declaration[:indentation], @@ -143,14 +142,6 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) updated_content = updated_lines.join if updated_content == gemfile_content - if base_gem_entries_preserved - say( - "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ - "leaving react_on_rails entries unchanged", - :yellow - ) - end - unless base_gem_entry_found || had_pro_gem_entry_before_prerequisites rollback_message = rollback_gemfile_after_failed_swap_precondition( gemfile_path: gemfile_path, @@ -171,7 +162,15 @@ def swap_base_gem_for_pro_in_gemfile(original_gemfile_content_for_rollback: nil) original_gemfile_content = original_gemfile_content_for_rollback || gemfile_content atomic_write_file(gemfile_path, updated_content) - say "✅ Replaced react_on_rails with react_on_rails_pro in Gemfile", :green + if base_gem_entries_removed + say( + "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ + "removed the now-stale react_on_rails entries", + :yellow + ) + else + say "✅ Replaced react_on_rails with react_on_rails_pro in Gemfile", :green + end bundle_install_after_gem_swap( gemfile_path: gemfile_path, original_gemfile_content: original_gemfile_content @@ -891,7 +890,7 @@ def remove_parenthesized_gem_call_closing_parenthesis(suffix) rest = suffix[(closing_index + 1)..].to_s return "#{prefix.rstrip} #{rest.lstrip}" if closing_parenthesis_line_has_postfix_code?(rest) - "#{prefix}#{rest}" + "#{prefix.chomp}#{rest}" end def closing_parenthesis_line_has_postfix_code?(rest) diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb index 81a091af03..4ee341168f 100644 --- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb +++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb @@ -482,6 +482,30 @@ def expect_parseable_ruby(source) expect(generator).to have_received(:bundle_install_after_gem_swap) end + it "does not introduce a blank line after a multiline parenthesized declaration" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem( + "react_on_rails", + "~> 16.0" + ) + gem "rails", "~> 7.1" + RUBY + allow(generator).to receive(:bundle_install_after_gem_swap) + + generator.send(:swap_base_gem_for_pro_in_gemfile) + + gemfile_content = File.read(gemfile_path) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + gem "react_on_rails_pro", + "~> 16.0" + gem "rails", "~> 7.1" + RUBY + expect_parseable_ruby(gemfile_content) + expect(generator).to have_received(:bundle_install_after_gem_swap) + end + it "replaces parenthesized declarations that start on the gem line and continue across lines" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" @@ -584,30 +608,34 @@ def expect_parseable_ruby(source) expect(gemfile_content).not_to include("gem(") end - it "leaves base gem entries unchanged when react_on_rails_pro already exists" do - original_gemfile = <<~RUBY + it "removes the stale base gem entry and runs bundle install when react_on_rails_pro already exists" do + simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" gem "react_on_rails", "~> 16.0" gem "react_on_rails_pro", "~> 16.0" RUBY - simulate_existing_file("Gemfile", original_gemfile) allow(generator).to receive(:bundle_install_after_gem_swap) allow(generator).to receive(:say) generator.send(:swap_base_gem_for_pro_in_gemfile) - expect(File.read(gemfile_path)).to eq(original_gemfile) + gemfile_content = File.read(gemfile_path) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + gem "react_on_rails_pro", "~> 16.0" + RUBY + expect_parseable_ruby(gemfile_content) expect(generator).to have_received(:say) .with( "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ - "leaving react_on_rails entries unchanged", + "removed the now-stale react_on_rails entries", :yellow ) - expect(generator).not_to have_received(:bundle_install_after_gem_swap) + expect(generator).to have_received(:bundle_install_after_gem_swap) end - it "leaves base gem entries unchanged when existing parenthesized pro declaration has comment lines" do - original_gemfile = <<~RUBY + it "removes the stale base gem and bundles when a parenthesized pro declaration has comment lines" do + simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" gem "react_on_rails", "~> 16.0" gem( @@ -616,20 +644,28 @@ def expect_parseable_ruby(source) "~> 16.0" ) RUBY - simulate_existing_file("Gemfile", original_gemfile) allow(generator).to receive(:bundle_install_after_gem_swap) allow(generator).to receive(:say) generator.send(:swap_base_gem_for_pro_in_gemfile) - expect(File.read(gemfile_path)).to eq(original_gemfile) + gemfile_content = File.read(gemfile_path) + expect(gemfile_content).to eq(<<~RUBY) + source "https://rubygems.org" + gem( + # pinned for compatibility + "react_on_rails_pro", + "~> 16.0" + ) + RUBY + expect_parseable_ruby(gemfile_content) expect(generator).to have_received(:say) .with( "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ - "leaving react_on_rails entries unchanged", + "removed the now-stale react_on_rails entries", :yellow ) - expect(generator).not_to have_received(:bundle_install_after_gem_swap) + expect(generator).to have_received(:bundle_install_after_gem_swap) end it "replaces duplicate base gem declarations without dropping grouped declarations" do @@ -637,7 +673,7 @@ def expect_parseable_ruby(source) source "https://rubygems.org" gem "react_on_rails", "~> 16.0" group :test do - gem "react_on_rails", require: false + gem "react_on_rails", "~> 16.0", require: false end RUBY allow(generator).to receive(:bundle_install_after_gem_swap) @@ -645,12 +681,11 @@ def expect_parseable_ruby(source) generator.send(:swap_base_gem_for_pro_in_gemfile) gemfile_content = File.read(gemfile_path) - expected_version = generator.send(:pro_gem_version_requirement) expect(gemfile_content).to eq(<<~RUBY) source "https://rubygems.org" gem "react_on_rails_pro", "~> 16.0" group :test do - gem "react_on_rails_pro", "#{expected_version}", require: false + gem "react_on_rails_pro", "~> 16.0", require: false end RUBY expect_parseable_ruby(gemfile_content)