diff --git a/.lychee.toml b/.lychee.toml index 83d224caef..4c69aa3e7e 100644 --- a/.lychee.toml +++ b/.lychee.toml @@ -116,9 +116,12 @@ exclude = [ # return 5xx responses to lychee in CI, even when the pages are live. '^https://github\.com/shakacode/react_on_rails/compare/', '^https://github\.com/shakacode/react_on_rails/pull/[0-9]+$', # Intermittent 5xx from GitHub PR pages in CI + '^https://github\.com/shakacode/react_on_rails/actions/workflows/pro-test-package-and-gem\.yml$', # Intermittent 502 from GitHub Actions workflow pages in CI '^https://github\.com/shakacode/react_on_rails/issues/2766$', # Intermittent 502 from GitHub issue pages in CI '^https://github\.com/rack/rack/blob/main/SPEC\.rdoc$', # Intermittent 502 from GitHub blob pages in CI '^https://github\.com/shakacode/shakapacker/blob/cdf32835d3e0949952b8b4b53063807f714f9b24/package/environments/base\.js(#.*)?$', # Intermittent 502 from GitHub blob view in CI + '^https://github\.com/npm/cli/issues/6253$', # Intermittent 502 from GitHub issue pages in CI + '^https://github\.com/search\?q=gem\+react_on_rails&ref=advsearch&type=repositories&utf8=%E2%9C%93$', # Intermittent 502 from GitHub search in CI # ============================================================================ # EXTERNAL DOC PAGES WITH CI CONNECTIVITY FAILURES diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c26fcb7dd..d7f21fa94b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ #### Fixed +- **[Pro]** **Pro migration generator rewrites all base-package references and preserves Gemfile pins**: `rails generate react_on_rails:pro` now rewrites Jest/Vitest mock helpers (`jest.mock`, `vi.mock`, `requireActual`/`importActual`, and the rest) and TypeScript `declare module 'react-on-rails'` blocks alongside its existing `import`/`require`/dynamic-import handling, and the Gemfile swap now preserves the user's existing version pin (and other gem options) instead of overwriting them with the running gem's version. `react_on_rails:doctor` is widened to match: it also flags stale side-effect imports (`import 'react-on-rails';`), Jest/Vitest mock helpers, and `declare module` blocks, and the new side-effect-import pattern keeps the doctor a superset of the rewriter so anything the rewriter doesn't reach gets surfaced. Closes [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). - **Doctor now honors nested JavaScript package roots**: `react_on_rails:doctor` now checks package-manager lockfiles, `package.json`, and installed React from the configured `node_modules_location`, reducing false diagnostics for legacy apps that keep dependencies under `client/`. The Vite migration guide now documents the supported thin-wrapper pattern for those layouts. Note: a missing `package.json` at the configured `node_modules_location` now emits a warning instead of being silently skipped, so apps misconfigured against a nonexistent path will see new diagnostics on upgrade. Fixes [Issue 3205](https://github.com/shakacode/react_on_rails/issues/3205). [PR 3220](https://github.com/shakacode/react_on_rails/pull/3220) by [justin808](https://github.com/justin808). - **Generated pack regeneration is now serialized**: `generate_packs_if_stale` now uses a Rails `tmp/` lock file, re-checks staleness after waiting, and avoids concurrent cleanup/regeneration races when multiple processes trigger auto-bundling at the same time. Fixes [Issue 1627](https://github.com/shakacode/react_on_rails/issues/1627). [PR 3231](https://github.com/shakacode/react_on_rails/pull/3231) by [justin808](https://github.com/justin808). - **Install generator validates the selected JavaScript package manager**: The install generator now checks the manager selected from `REACT_ON_RAILS_PACKAGE_MANAGER`, the `packageManager` field in `package.json`, or a lockfile on disk — instead of passing when any JavaScript package manager is installed. When the selected command is missing, the error names the selected manager, the source that selected it, and the available alternatives. The generator also warns when `REACT_ON_RAILS_PACKAGE_MANAGER` is set to a value outside the supported set (`npm`, `pnpm`, `yarn`, `bun`). Addresses package manager validation from [Issue 1958](https://github.com/shakacode/react_on_rails/issues/1958). [PR 3229](https://github.com/shakacode/react_on_rails/pull/3229) by [justin808](https://github.com/justin808). 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 7a12130de7..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,6 +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! makes pro_gem_installed? return true before install. See #3303. def mark_pro_gem_installed! @pro_gem_installed = true end 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 8f0f61f1a4..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 @@ -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,43 +104,21 @@ 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 base_gem_entry_found = false + base_gem_entries_removed = false line_index = 0 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 + base_gem_declaration = ReactOnRails::ProMigration.base_gem_declaration_at(gemfile_lines, line_index) - line_index = multiline_parenthesized_match[:next_index] - next - end - - match = line.match(base_gem_pattern) - - unless match + unless base_gem_declaration updated_lines << line line_index += 1 next @@ -147,25 +126,18 @@ 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] + if has_pro_gem_entry + base_gem_entries_removed = true + else 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 @@ -183,10 +155,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 @@ -194,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 @@ -329,8 +305,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) @@ -339,66 +315,84 @@ def js_files_for_import_update end.uniq end - def rewrite_react_on_rails_module_specifiers(content) - static_import_specifier_pattern = %r{ - (? - \A\s*(?:/\*.*?\*/\s*)?(?:import|export)(?:\s+type)?\s+.*?\s+from\s+| - \A\s*[\w\}\],\*\$\s]+\s+from\s+ - ) - (?["']) - react-on-rails(?!-pro) - (?=(?:["']|/)) - }x - - dynamic_or_require_specifier_pattern = %r{ - (? - (? + \A\s*(?:/\*.*?\*/\s*)?(?:import|export)(?:\s+type)?\s+.*?\s+from\s+| + \A\s*[\w\}\],\*\$\s]+\s+from\s+ + ) + (?["']) + react-on-rails(?!-pro) + (?=(?:["']|/)) + }x + + DYNAMIC_OR_REQUIRE_SPECIFIER_PATTERN = %r{ + (? + (?["']) + react-on-rails(?!-pro) + (?=(?:["']|/)) + }x + + SIDE_EFFECT_IMPORT_PATTERN = %r{ + \A(?\s*(?:/\*.*?\*/\s*)*import\s+) + (?["']) + react-on-rails(?!-pro) + (?=(?:["']|/)) + }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_PATTERN = ReactOnRails::ProMigration::JEST_MODULE_SPECIFIER_METHOD_PATTERN + VITEST_MODULE_SPECIFIER_METHOD_PATTERN = ReactOnRails::ProMigration::VITEST_MODULE_SPECIFIER_METHOD_PATTERN + + MOCK_CALL_PATTERN = %r{ + (? + (?["']) - react-on-rails(?!-pro) - (?=(?:["']|/)) - }x - - side_effect_import_pattern = %r{ - \A(?\s*(?:/\*.*?\*/\s*)*import\s+) - (?["']) - react-on-rails(?!-pro) - (?=(?:["']|/)) - }x + \s* + (?:<[^;\n]*>\s*)? + \s*\(\s* + ) + (?["']) + react-on-rails(?!-pro) + (?=(?:["']|/)) + }x + + DECLARE_MODULE_PATTERN = %r{ + \A(?\s*(?:export\s+)?declare\s+module\s+) + (?["']) + react-on-rails(?!-pro) + (?=(?:["']|/)) + }x + + BASE_PACKAGE_REWRITE_PATTERNS = [ + STATIC_IMPORT_SPECIFIER_PATTERN, + DYNAMIC_OR_REQUIRE_SPECIFIER_PATTERN, + SIDE_EFFECT_IMPORT_PATTERN, + 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| rewrite_outside_inline_template_literals(line) do |line_without_templates| - rewritten_line = line_without_templates.gsub(static_import_specifier_pattern) do - "#{Regexp.last_match[:prefix]}#{Regexp.last_match[:quote]}react-on-rails-pro" - end - - rewritten_line = rewritten_line.gsub(dynamic_or_require_specifier_pattern) do - "#{Regexp.last_match[:prefix]}#{Regexp.last_match[:quote]}react-on-rails-pro" - end - - rewritten_line.gsub(side_effect_import_pattern) do - "#{Regexp.last_match[:prefix]}#{Regexp.last_match[:quote]}react-on-rails-pro" - 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) @@ -562,23 +556,72 @@ 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)(?=(?:["']|/))} + def rewrite_pending_module_specifier(line) - line.gsub(%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) 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) @@ -617,7 +660,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*,(?:\s*#.*\n|\s++)*)["'][^"']*["'](?\s*,)?/ - loop do - updated_suffix = normalized_suffix.sub(version_arg_pattern) do - if Regexp.last_match[:trailing_comma] - Regexp.last_match[:prefix].sub(/\n[ \t]*\z/, "") - else - "" - 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 - break if updated_suffix == normalized_suffix - normalized_suffix = updated_suffix + scan_index += 1 end - normalized_suffix = normalized_suffix.sub(/\A,\s*(?:#[^\n]*)?\n\z/, "\n") - normalized_suffix = normalized_suffix.sub(/\A,[ \t]{2,}/, ", ") - normalized_suffix = normalized_suffix.sub(/\)(\s*(?:#.*)?\n)\z/, '\1') if parenthesized_gem_call - "#{indentation}gem #{quote}react_on_rails_pro#{quote}, " \ - "#{quote}#{pro_gem_version_requirement}#{quote}#{normalized_suffix}" + 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 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 2aefac417d..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 @@ -52,12 +53,18 @@ def setup_pro say set_color("=" * 80, :cyan) end - # Check if Pro gem is missing. Attempts auto-install via bundle add. + # Check if the Pro gem is missing. When the base react_on_rails gem is in + # the Gemfile, installation is deferred to the later Gemfile swap (which + # preserves the user's version pin); otherwise auto-install via `bundle + # add` is attempted. # @param force [Boolean] When true, always checks (default: only if use_pro?). - # @return [Boolean] true if Pro gem is missing and could not be installed + # @return [Boolean] true only if the Pro gem is missing and could not be + # installed; false if it is present, was auto-installed, or the install + # is deferred to the Gemfile swap. def missing_pro_gem?(force: false) return false unless force || use_pro? return false if pro_gem_installed? + return false if defer_pro_gem_install_to_gemfile_swap return false if attempt_pro_gem_auto_install optional_prerelease_line = prerelease_note.empty? ? "" : "\n#{prerelease_note}" @@ -532,6 +539,22 @@ def pro_gem_auto_install_command "bundle add #{PRO_GEM_NAME} --version='#{pro_gem_version_requirement}' --strict" end + def defer_pro_gem_install_to_gemfile_swap + return false unless base_react_on_rails_gem_in_gemfile? + + mark_pro_gem_installed! + true + end + + def base_react_on_rails_gem_in_gemfile? + gemfile_path = File.join(destination_root, "Gemfile") + return false unless File.exist?(gemfile_path) + + ReactOnRails::ProMigration.base_gem_entry?(File.read(gemfile_path)) + rescue SystemCallError, IOError + false + end + def pro_gem_version_requirement # Prerelease gem versions need an exact pin: Bundler's pessimistic operator # (~>) does not match prerelease versions, so a stable range would fail to diff --git a/react_on_rails/lib/react_on_rails/doctor.rb b/react_on_rails/lib/react_on_rails/doctor.rb index 4b15d91f17..c8d7e64a25 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" @@ -2748,7 +2749,7 @@ def check_pro_setup check_pro_initializer_existence ensure_rails_environment_loaded check_pro_renderer_mode - check_base_package_imports + check_base_package_references check_deprecated_renderer_cache_task end @@ -2875,44 +2876,121 @@ def renderer_cache_migration_suggestion(path) 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_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 = 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. + BASE_PACKAGE_JEST_MODULE_SPECIFIER_METHOD_PATTERN = + ReactOnRails::ProMigration::JEST_MODULE_SPECIFIER_METHOD_PATTERN + BASE_PACKAGE_VITEST_MODULE_SPECIFIER_METHOD_PATTERN = + 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. + # + # importActual/importMock exist only as vi.* methods; there is no + # `import { importActual } from 'vitest'` form. The bare branch below is a + # deliberately broad detector heuristic (the rewriter omits it because + # rewriting is destructive while detection is advisory) and accepts that a + # user-defined helper of that name taking a 'react-on-rails' string matches. + BASE_PACKAGE_MOCK_PATTERN = %r{ + \b(?: + (?: + jest\.(?:#{BASE_PACKAGE_JEST_MODULE_SPECIFIER_METHOD_PATTERN}) + | + vi\.(?:#{BASE_PACKAGE_VITEST_MODULE_SPECIFIER_METHOD_PATTERN}) + ) + \s* + (?:<[^;\n]*>\s*)? + | + (?:importActual|importMock) + ) + \s*\(\s*['"]react-on-rails(?:/[^'"]*)?['"] + }x + # In Ruby, ^ matches the start of any line, so this catches declarations anywhere in the file. + BASE_PACKAGE_DECLARE_MODULE_PATTERN = %r{^\s*(?:export\s+)?declare\s+module\s+['"]react-on-rails(?:/[^'"]*)?['"]} + BASE_PACKAGE_REFERENCE_PATTERNS = [ + BASE_PACKAGE_IMPORT_PATTERN, + BASE_PACKAGE_REQUIRE_PATTERN, + BASE_PACKAGE_DYNAMIC_IMPORT_PATTERN, + BASE_PACKAGE_SIDE_EFFECT_IMPORT_PATTERN, + BASE_PACKAGE_MOCK_PATTERN, + BASE_PACKAGE_DECLARE_MODULE_PATTERN + ].freeze - def check_base_package_imports # rubocop:disable Metrics/CyclomaticComplexity - 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 = [] - - 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) - - files_with_base_import << file - end - end + def check_base_package_references + files_with_base_reference = files_with_base_package_references(resolve_js_source_path) - 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 + Look for static imports, side-effect imports, CommonJS requires, dynamic imports, + Jest/Vitest mock helpers, or TypeScript module augmentations. + Note: this includes commented-out references; review each file before updating. + + 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) + import 'react-on-rails-pro'; // Side-effect import + const ReactOnRails = require('react-on-rails-pro'); // CommonJS require + const ReactOnRails = await import('react-on-rails-pro'); // Dynamic import + jest.mock('react-on-rails-pro', ...); // Jest mock helper + vi.mock('react-on-rails-pro', ...); // Vitest mock helper + 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 + + def files_with_base_package_references(source_path) + # 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}" } + end + + js_patterns.flat_map do |pattern| + Dir.glob(pattern) + .reject { |file| file.include?("/node_modules/") } + .select { |file| base_package_reference_file?(file) } + 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) + content = File.binread(file).force_encoding("UTF-8") + return false unless content.valid_encoding? + + base_package_reference?(content) + rescue SystemCallError, IOError + false + end + + def base_package_reference?(content) + # Content-based matching intentionally catches comments and string literals + # so stale migration references stay visible. + BASE_PACKAGE_REFERENCE_PATTERNS.any? { |reference_pattern| content.match?(reference_pattern) } end # ── React Server Components ──────────────────────────────────── 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..55f5df5c48 --- /dev/null +++ b/react_on_rails/lib/react_on_rails/pro_migration.rb @@ -0,0 +1,175 @@ +# 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*(?:,|\)|#|$))/ + STRING_LITERAL_PATTERN = /"(?:\\.|[^"\\])*+"|'(?:\\.|[^'\\])*+'|`(?:\\.|[^`\\])*+`/ + + 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 strip_ruby_inline_comment(gem_name_fragment).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 = strip_ruby_inline_comment(line).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(STRING_LITERAL_PATTERN, "") + return line_without_strings unless strip_ruby_comments + + strip_ruby_inline_comment(line_without_strings) + end + + def strip_ruby_inline_comment(line) + hash_index = line.index("#") + return line unless hash_index + + prefix = line[0, hash_index].rstrip + newline_index = line.index("\n", hash_index) + newline_index ? prefix + line[newline_index..] : prefix + end + end +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 33e4a6b645..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 @@ -2136,7 +2136,7 @@ def write_project_file(path, content) let(:doctor) { described_class.new(verbose: false, fix: true) } let(:checker) { doctor.instance_variable_get(:@checker) } - it "adds explicit guidance that Gemfile constraints are not auto-fixed" do + it "does not add Gemfile guidance when no package versions changed" do result = ReactOnRails::VersionSynchronizer::Result.new( changes: [ { section: "dependencies", package: "react-on-rails", from: "^16.0.0", to: "16.5.0" } @@ -2154,6 +2154,35 @@ def write_project_file(path, content) doctor.send(:auto_fix_versions) + info_messages = checker.messages.select { |msg| msg[:type] == :info }.map { |msg| msg[:content] } + expect(info_messages).not_to include( + a_string_including("FIX=true only updates package.json; update Gemfile constraints manually if needed.") + ) + end + + it "adds explicit guidance that Gemfile constraints are not auto-fixed when package versions changed" do + result = ReactOnRails::VersionSynchronizer::Result.new( + changes: [ + { + section: "dependencies", + package: "react-on-rails", + from: "16.0.0", + to: "16.1.0" + } + ], + changed_files: ["package.json"], + unsupported_specs: [], + missing_source_specs: [] + ) + synchronizer = instance_double(ReactOnRails::VersionSynchronizer, sync: result) + + allow(doctor).to receive(:resolved_package_json_path).and_return("package.json") + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with("package.json").and_return(true) + allow(ReactOnRails::VersionSynchronizer).to receive(:new).and_return(synchronizer) + + doctor.send(:auto_fix_versions) + info_messages = checker.messages.select { |msg| msg[:type] == :info }.map { |msg| msg[:content] } expect(info_messages).to include( a_string_including("FIX=true only updates package.json; update Gemfile constraints manually if needed.") @@ -2965,7 +2994,7 @@ class << self end end - describe "check_base_package_imports" do + describe "check_base_package_references" do let(:doctor) { described_class.new(verbose: false, fix: false) } let(:checker) { doctor.instance_variable_get(:@checker) } @@ -2982,10 +3011,11 @@ class << self end it "reports warning with file paths" do - doctor.send(:check_base_package_imports) + doctor.send(:check_base_package_references) warning_msgs = checker.messages.select { |m| m[:type] == :warning } expect(warning_msgs.any? { |m| m[:content].include?("react-on-rails") }).to be true expect(warning_msgs.any? { |m| m[:content].include?("custom-bundle.js") }).to be true + expect(warning_msgs.any? { |m| m[:content].include?("dynamic imports") }).to be true end end @@ -3002,7 +3032,7 @@ class << self end it "reports warning" do - doctor.send(:check_base_package_imports) + doctor.send(:check_base_package_references) warning_msgs = checker.messages.select { |m| m[:type] == :warning } expect(warning_msgs.any? { |m| m[:content].include?("react-on-rails") }).to be true end @@ -3021,12 +3051,408 @@ class << self end it "reports warning" do - doctor.send(:check_base_package_imports) + doctor.send(:check_base_package_references) warning_msgs = checker.messages.select { |m| m[:type] == :warning } expect(warning_msgs.any? { |m| m[:content].include?("react-on-rails") }).to be true end end + context "when JS files dynamically import 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/lazy.js", + "const ReactOnRails = await import('react-on-rails/client');\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?("lazy.js") }).to be true + end + end + + context "when JS files use a side-effect import of 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/side-effect.js", + "import 'react-on-rails';\nimport 'react-on-rails/client';\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?("side-effect.js") }).to be true + end + end + + context "when JS files use a side-effect import of the Pro package" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/side-effect.js", + "import 'react-on-rails-pro';\n") + example.run + end + end + end + + it "does not warn" do + doctor.send(:check_base_package_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("side-effect.js") }).to be false + end + end + + context "when Vue or Svelte components reference 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/Widget.vue", + "\n\n" \ + "\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| + 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_references) + 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 + 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_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 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| + 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 "does not warn" 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 false + end + end + + context "when Vitest tests import the actual 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", + "const mod = await vi.importActual('react-on-rails/client');\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 Vitest tests import the base package mock without a receiver" 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", + "const mod = await importMock('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 non-test code has a bare mock helper that references the base package string" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/factory.ts", + "mock('react-on-rails', factory);\n") + example.run + end + end + end + + it "does not warn" do + doctor.send(:check_base_package_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("factory.ts") }).to be false + end + end + + context "when non-test code has a mock-like receiver that references the base package string" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/factory.ts", + "server.mock('react-on-rails', factory);\n") + example.run + end + end + end + + it "does not warn" do + doctor.send(:check_base_package_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("factory.ts") }).to be false + end + end + + context "when JS tests use additional mock helpers after a Pro migration" do + around do |example| + Dir.mktmpdir do |tmpdir| + 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" + }.each do |filename, content| + File.write("app/javascript/packs/#{filename}", content) + end + example.run + end + end + end + + 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") + end + end + + context "when JS tests use a nonstandard vitest namespace object" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/vitest-namespace.test.ts", + "vitest.mock('react-on-rails', () => ({}));\n") + example.run + end + end + end + + it "does not report a warning for the nonstandard helper form" do + doctor.send(:check_base_package_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("vitest-namespace.test.ts") }).to be false + 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_references) + 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 TypeScript declaration files augment 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/types") + File.write("app/javascript/types/react-on-rails-client.d.ts", + "declare module 'react-on-rails/client' {\n export function register(): void;\n}\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?("react-on-rails-client.d.ts") }).to be true + end + end + + context "when TypeScript declarations export augmentations 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/types") + File.write("app/javascript/types/react-on-rails-export.d.ts", + "export 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_references) + warning_msgs = checker.messages.select { |m| m[:type] == :warning } + expect(warning_msgs.any? { |m| m[:content].include?("react-on-rails-export.d.ts") }).to be true + end + end + + context "when module files use ESM or CommonJS extensions 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/esm-entry.mjs", + "import ReactOnRails from 'react-on-rails';\n") + File.write("app/javascript/packs/cjs-entry.cjs", + "const ReactOnRails = require('react-on-rails/client');\n") + example.run + end + end + end + + it "reports warning for each module file" 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("esm-entry.mjs") + expect(warning_content).to include("cjs-entry.cjs") + end + end + + context "when one scanned file has invalid UTF-8 content" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + File.write("app/javascript/packs/app.js", + "import ReactOnRails from 'react-on-rails';\n") + File.binwrite("app/javascript/packs/binary.js", "\xFF") + example.run + end + end + end + + it "skips the unreadable file and keeps scanning the rest" 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("app.js") + expect(warning_content).not_to include("Could not scan") + end + end + context "when JS files correctly import from 'react-on-rails-pro'" do around do |example| Dir.mktmpdir do |tmpdir| @@ -3040,12 +3466,117 @@ class << self end it "reports success" do - doctor.send(:check_base_package_imports) + doctor.send(:check_base_package_references) success_msgs = checker.messages.select { |m| m[:type] == :success } expect(success_msgs.any? { |m| m[:content].include?("Pro package used correctly") }).to be true end end + context "when JS files correctly dynamically import from '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/lazy.js", + "const ReactOnRails = await import('react-on-rails-pro/client');\n") + example.run + end + end + end + + it "reports success" do + doctor.send(:check_base_package_references) + 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 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_references) + 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 Vitest tests correctly import actual '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", + "const mod = await vi.importActual('react-on-rails-pro');\n") + example.run + end + end + end + + it "reports success" do + doctor.send(:check_base_package_references) + 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_references) + 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 a Pro package subpath" 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-client.d.ts", + "declare module 'react-on-rails-pro/client' {\n export function register(): void;\n}\n") + example.run + end + end + end + + it "reports success" do + doctor.send(:check_base_package_references) + 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| @@ -3054,12 +3585,31 @@ class << self end it "reports success (no files to scan)" do - doctor.send(:check_base_package_imports) + doctor.send(:check_base_package_references) success_msgs = checker.messages.select { |m| m[:type] == :success } expect(success_msgs.any? { |m| m[:content].include?("Pro package used correctly") }).to be true 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| @@ -3079,12 +3629,37 @@ class << self end it "scans the custom source_path and reports warning" do - doctor.send(:check_base_package_imports) + doctor.send(:check_base_package_references) warning_msgs = checker.messages.select { |m| m[:type] == :warning } expect(warning_msgs.any? { |m| m[:content].include?("react-on-rails") }).to be true expect(warning_msgs.any? { |m| m[:content].include?("client/app/packs/app.js") }).to be true end end + + context "when a file disappears during the base package scan" do + around do |example| + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + FileUtils.mkdir_p("app/javascript/packs") + example.run + end + end + end + + it "skips the unreadable file and keeps scanning the rest" do + missing_file = "app/javascript/packs/deleted.js" + stale_file = "app/javascript/packs/stale.js" + File.write(stale_file, "import ReactOnRails from 'react-on-rails';\n") + + allow(Dir).to receive(:glob).and_call_original + allow(Dir).to receive(:glob).with("app/javascript/**/*.js").and_return([missing_file, stale_file]) + + 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(stale_file) + expect(warning_content).not_to include("Could not scan for base package references") + end + end end # ── RSC Setup Checks ───────────────────────────────────────────── 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 765118e8e2..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 @@ -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 @@ -55,6 +60,47 @@ end end + context "when Pro gem is not installed but base react_on_rails is in the Gemfile" do + let(:generator) { described_class.new } + let(:gemfile_path) { File.join(destination_root, "Gemfile") } + + before do + prepare_destination + allow(generator).to receive(:destination_root).and_return(destination_root) + allow(Gem).to receive(:loaded_specs).and_return({}) + allow(generator).to receive(:gem_in_lockfile?).with("react_on_rails_pro").and_return(false) + allow(Bundler).to receive(:with_unbundled_env).and_yield + allow(Process).to receive(:spawn) + + File.write(gemfile_path, <<~RUBY) + source "https://rubygems.org" + gem "react_on_rails", "~> 16.0" + RUBY + end + + specify "missing_pro_gem? returns false and bypasses bundle add to let swap handle the Gemfile" do + 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 + + 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 let(:generator) { described_class.new } let(:gemfile_path) { File.join(destination_root, "Gemfile") } @@ -132,7 +178,7 @@ allow(generator).to receive(:destination_root).and_return(destination_root) end - it "replaces react_on_rails with react_on_rails_pro and runs bundle install" do + it "replaces react_on_rails with react_on_rails_pro and preserves the user's version pin" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" gem "react_on_rails", "~> 16.0" @@ -142,8 +188,7 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") + expect(gemfile_content).to include('gem "react_on_rails_pro", "~> 16.0"') expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) expect(generator).to have_received(:bundle_install_after_gem_swap) end @@ -158,13 +203,12 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\", require: false") + expect(gemfile_content).to include('gem "react_on_rails_pro", "~> 16.0", require: false') expect(gemfile_content).to include("if: ENV[\"ENABLE_ROR\"]") expect(gemfile_content).not_to include("gem \"react_on_rails\",") end - it "strips all version constraints from multi-constraint declarations" do + it "preserves multi-constraint version declarations" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" gem "react_on_rails", ">= 15.0", "< 16.0", require: false @@ -174,10 +218,8 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\", require: false") - expect(gemfile_content).not_to include(">= 15.0") - expect(gemfile_content).not_to include("< 16.0") + expect(gemfile_content).to include('gem "react_on_rails_pro", ">= 15.0", "< 16.0", require: false') + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) end it "preserves indentation when replacing a grouped Gemfile entry" do @@ -193,12 +235,11 @@ 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 include(" gem \"react_on_rails_pro\", \"#{expected_version}\"") + expect(gemfile_content).to include(' gem "react_on_rails_pro", "~> 16.0"') expect(generator).to have_received(:bundle_install_after_gem_swap) end - it "replaces multiline react_on_rails declaration without leaving orphan lines" do + it "preserves the user's multiline non-parenthesized declaration verbatim" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" gem "react_on_rails", @@ -209,10 +250,8 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") - expect(gemfile_content).not_to include("gem \"react_on_rails\",") - expect(gemfile_content).not_to include(" \"~> 16.0\"") + expect(gemfile_content).to include("gem \"react_on_rails_pro\",\n \"~> 16.0\"") + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) expect(generator).to have_received(:bundle_install_after_gem_swap) end @@ -228,11 +267,10 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\",") - expect(gemfile_content).not_to include(",\n \n require: false") + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0",') expect(gemfile_content).to include("require: false") - expect(gemfile_content).not_to include("\"~> 16.0\"") + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) end it "does not leave a trailing comma when replacing multiline declarations before another gem" do @@ -247,13 +285,12 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") + expect(gemfile_content).to include("gem \"react_on_rails_pro\",\n \"~> 16.0\"") expect(gemfile_content).to include('gem "rails"') expect(gemfile_content).not_to match(/react_on_rails_pro".*,\s*\n\s*gem "rails"/) end - it "replaces multiline declarations that have an inline comment after the trailing comma" do + it "preserves a multiline declaration whose comma is followed by an inline comment" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" gem "react_on_rails", # pinned for compatibility @@ -264,13 +301,12 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") - expect(gemfile_content).not_to include("gem \"react_on_rails\", # pinned for compatibility") - expect(gemfile_content).not_to include(" \"~> 16.0\"") + expect(gemfile_content).to include('gem "react_on_rails_pro", # pinned for compatibility') + expect(gemfile_content).to include(' "~> 16.0"') + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) end - it "replaces multiline declarations when trailing comma is followed by a tight inline comment" do + it "preserves a multiline declaration whose comma is followed by a tight inline comment" do simulate_existing_file("Gemfile", <<~RUBY) source "https://rubygems.org" gem "react_on_rails",#pinned for compatibility @@ -281,10 +317,9 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") - expect(gemfile_content).not_to include("gem \"react_on_rails\",#pinned for compatibility") - expect(gemfile_content).not_to include(" \"~> 16.0\"") + expect(gemfile_content).to include('gem "react_on_rails_pro",#pinned for compatibility') + expect(gemfile_content).to include(' "~> 16.0"') + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) end it "consumes multiline declarations when comment-only lines appear before continuation lines" do @@ -300,10 +335,10 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0"') expect(gemfile_content).to include("gem \"rails\"") - expect(gemfile_content).not_to include("~> 16.0") + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) end it "consumes multiline declarations when blank lines appear before continuation lines" do @@ -319,10 +354,10 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0"') expect(gemfile_content).to include("gem \"rails\"") - expect(gemfile_content).not_to include("~> 16.0") + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) end it "does not consume the next gem line when base declaration ends with a trailing comma" do @@ -351,8 +386,7 @@ 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 include("gem 'react_on_rails_pro', '#{expected_version}'") + expect(gemfile_content).to include("gem 'react_on_rails_pro', '~> 16.0'") expect(generator).to have_received(:bundle_install_after_gem_swap) end @@ -366,9 +400,8 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") - expect(gemfile_content).not_to include("gem \"react_on_rails_pro\", \"#{expected_version}\")") + expect(gemfile_content).to include('gem "react_on_rails_pro", "~> 16.0"') + expect(gemfile_content).not_to include('"~> 16.0")') expect(gemfile_content).not_to include('gem("react_on_rails"') expect(generator).to have_received(:bundle_install_after_gem_swap) end @@ -383,12 +416,53 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\", require: false") + expect(gemfile_content).to include('gem "react_on_rails_pro", "~> 16.0", require: false') expect(gemfile_content).to include('if: ENV.fetch("ENABLE_ROR", false)') 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" @@ -402,10 +476,33 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0"') expect(gemfile_content).not_to include('"react_on_rails"') - expect(gemfile_content).not_to include('"~> 16.0"') + 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 @@ -422,8 +519,8 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\",") + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0",') expect(gemfile_content).to include("require: false") expect(gemfile_content).not_to include('gem("react_on_rails"') expect(gemfile_content.lines.any? { |line| line.strip == ")" }).to be false @@ -444,12 +541,11 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\",") + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0",') expect(gemfile_content).to include("require: false,") expect(gemfile_content).to include('if: ENV["ENABLE_ROR"]') expect(gemfile_content).not_to include('"react_on_rails"') - expect(gemfile_content).not_to include('"~> 16.0"') end it "handles comments containing closing parentheses inside multiline parenthesized declarations" do @@ -466,9 +562,8 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") - expect(gemfile_content).not_to include('"~> 16.0"') + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0"') expect(gemfile_content).not_to include("gem(") end @@ -486,11 +581,10 @@ expect { generator.send(:swap_base_gem_for_pro_in_gemfile) }.not_to raise_error gemfile_content = File.read(gemfile_path) - expected_version = generator.send(:pro_gem_version_requirement) - expect(gemfile_content).to include("gem \"react_on_rails_pro\", \"#{expected_version}\",") + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0",') expect(gemfile_content).to include("require: false") expect(gemfile_content).not_to include("gem(") - expect(gemfile_content).not_to include('"~> 16.0"') end it "handles nested parentheses in multiline parenthesized Gemfile options" do @@ -507,15 +601,14 @@ 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") - expect(gemfile_content).not_to include('"react_on_rails"') + expect(gemfile_content).to include('gem "react_on_rails_pro",') + expect(gemfile_content).to include('"~> 16.0",') expect(gemfile_content).to include('if: ENV.fetch("ENABLE_ROR") { true }') + expect(gemfile_content).not_to include('"react_on_rails"') expect(gemfile_content).not_to include("gem(") - expect(gemfile_content).not_to include('"~> 16.0"') end - it "removes base gem without adding duplicate react_on_rails_pro entries" do + 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" @@ -527,14 +620,21 @@ 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(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; preserving current version constraint", :yellow) + .with( + "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ + "removed the now-stale react_on_rails entries", + :yellow + ) expect(generator).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 + 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" @@ -550,10 +650,95 @@ 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(/["']react_on_rails_pro["']/).size).to eq(1) + 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; preserving current version constraint", :yellow) + .with( + "ℹ️ Existing react_on_rails_pro Gemfile entry detected; " \ + "removed the now-stale react_on_rails entries", + :yellow + ) + expect(generator).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", "~> 16.0", 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) + 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", "~> 16.0", 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(generator).to have_received(:bundle_install_after_gem_swap) end @@ -571,6 +756,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) @@ -684,6 +884,79 @@ expect(File.stat(gemfile_path).mode & 0o777).to eq(0o644) end + + it "preserves a path: argument alongside the version pin" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem "react_on_rails", "~> 16.0", path: "../react_on_rails" + 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", 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" + gem "react_on_rails", path: "../react_on_rails" + 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 include( + "gem \"react_on_rails_pro\", \"#{expected_version}\", path: \"../react_on_rails\"" + ) + end + + it "adds the default version when the user has only a git: argument" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem "react_on_rails", 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) + expected_version = generator.send(:pro_gem_version_requirement) + expect(gemfile_content).to include( + "gem \"react_on_rails_pro\", \"#{expected_version}\", git: \"https://example.invalid/repo.git\"" + ) + end + + it "adds the default version when the user has only the bare gem name" do + simulate_existing_file("Gemfile", <<~RUBY) + source "https://rubygems.org" + gem "react_on_rails" + 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 include("gem \"react_on_rails_pro\", \"#{expected_version}\"") + expect(gemfile_content).not_to match(/gem\s+["']react_on_rails["']/) + end end describe "#bundle_install_after_gem_swap" do @@ -1125,6 +1398,191 @@ expect(generator.send(:unclosed_block_comment_starts?, source_line)).to be true end + + it "rewrites jest.mock calls targeting the base package" do + source = <<~JS + jest.mock('react-on-rails', () => ({ authenticityHeaders: jest.fn() })); + jest.mock("react-on-rails/client", () => ({})); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to include("jest.mock('react-on-rails-pro',") + expect(rewritten).to include('jest.mock("react-on-rails-pro/client",') + end + + it "rewrites vi.mock and vi.importActual calls targeting the base package" do + source = <<~JS + vi.mock('react-on-rails', () => ({})); + const mod = await vi.importActual('react-on-rails/client'); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to include("vi.mock('react-on-rails-pro',") + expect(rewritten).to include("vi.importActual('react-on-rails-pro/client')") + end + + 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'); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + 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 + # call is therefore not a module specifier and must not be mutated. + source = <<~JS + const real = await importActual('react-on-rails'); + const fake = importMock('react-on-rails/client'); + const srv = server.importActual('react-on-rails'); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to eq(source) + end + + it "does not rewrite mock helpers on alien receivers" do + source = <<~JS + myJest.mock('react-on-rails', factory); + server.mock('react-on-rails', factory); + vitest.mock('react-on-rails'); + mock('react-on-rails', factory); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to eq(source) + end + + it "does not rewrite mock helpers already targeting the Pro package" do + source = <<~JS + jest.mock('react-on-rails-pro', () => ({})); + vi.mock('react-on-rails-pro/client', () => ({})); + const m = vi.importActual('react-on-rails-pro'); + JS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to eq(source) + end + + it "rewrites TypeScript declare module declarations targeting the base package" do + source = <<~TS + declare module 'react-on-rails' { + export function register(c: Record): void; + } + + declare module 'react-on-rails/client' { + export * from 'react-on-rails-pro'; + } + TS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to include("declare module 'react-on-rails-pro' {") + expect(rewritten).to include("declare module 'react-on-rails-pro/client' {") + expect(rewritten).not_to match(/declare module ['"]react-on-rails['"]/) + end + + it "rewrites export-prefixed declare module declarations" do + source = <<~TS + export declare module 'react-on-rails' { + export function register(c: Record): void; + } + TS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to include("export declare module 'react-on-rails-pro' {") + end + + it "rewrites indented declare module declarations and nested base-package exports together" do + source = <<~TS + declare module 'react-on-rails/client' { + export * from 'react-on-rails'; + export { default as ReactOnRails } from 'react-on-rails'; + } + TS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to include("declare module 'react-on-rails-pro/client' {") + expect(rewritten).to include("export * from 'react-on-rails-pro';") + expect(rewritten).to include("export { default as ReactOnRails } from 'react-on-rails-pro';") + expect(rewritten).not_to match(/['"]react-on-rails['"]/) + end + + it "does not rewrite declare module declarations already targeting the Pro package" do + source = <<~TS + declare module 'react-on-rails-pro' { } + declare module 'react-on-rails-pro/client' { } + TS + + rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source) + + expect(rewritten).to eq(source) + end end describe "#pro_flag_specified_for_context?" do