Skip to content

Commit f36c4ff

Browse files
justin808claude
andcommitted
Address PR review nitpicks and style suggestions
1. Refactor apply_minimum_versions into focused helper methods: - update_react_dependencies: Updates React and React-DOM versions - update_shakapacker_dependency: Updates Shakapacker in deps or devDeps - update_package_json_versions: Parses JSON and coordinates updates - update_gemfile_versions: Updates Gemfile with pinned version - apply_minimum_versions: Orchestrates both file updates This eliminates all RuboCop metric disables and improves testability. 2. Improve error message to include full package.json path instead of just the directory, making debugging easier. 3. Fix task description symmetry - both latest and minimum version task descriptions now specify exact versions for clarity: - latest: "React 19, Shakapacker 9.4.0" - minimum: "React 18, Shakapacker 8.2.0" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 12fc9d2 commit f36c4ff

File tree

2 files changed

+60
-49
lines changed

2 files changed

+60
-49
lines changed

react_on_rails/rakelib/run_rspec.rake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ namespace :run_rspec do
103103
ExampleType.all[:shakapacker_examples].select(&:minimum_versions?)
104104
end
105105

106-
desc "Runs Rspec for latest version example apps only (excludes minimum version tests)"
106+
desc "Runs Rspec for latest version example apps only (React 19, Shakapacker 9.4.0)"
107107
task shakapacker_examples_latest: latest_examples.map(&:gen_task_name) do
108108
latest_examples.each { |example_type| Rake::Task[example_type.rspec_task_name].invoke }
109109
end

react_on_rails/rakelib/shakapacker_examples.rake

Lines changed: 59 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,65 +16,74 @@ require_relative "task_helpers"
1616
namespace :shakapacker_examples do # rubocop:disable Metrics/BlockLength
1717
include ReactOnRails::TaskHelpers
1818

19-
# Updates package.json and Gemfile to use minimum supported versions for compatibility testing
20-
# rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
21-
def apply_minimum_versions(dir)
22-
# Update package.json
23-
package_json_path = File.join(dir, "package.json")
24-
if File.exist?(package_json_path)
25-
begin
26-
package_json = JSON.parse(File.read(package_json_path))
27-
rescue JSON::ParserError => e
28-
puts " ERROR: Failed to parse package.json in #{dir}: #{e.message}"
29-
raise
30-
end
19+
# Updates React-related dependencies to minimum supported versions
20+
def update_react_dependencies(deps)
21+
return unless deps
22+
23+
deps["react"] = ExampleType::MINIMUM_REACT_VERSION
24+
deps["react-dom"] = ExampleType::MINIMUM_REACT_VERSION
25+
# Shakapacker 8.2.0 requires webpack-assets-manifest ^5.x
26+
deps["webpack-assets-manifest"] = "^5.0.6" if deps.key?("webpack-assets-manifest")
27+
end
3128

32-
deps = package_json["dependencies"]
33-
dev_deps = package_json["devDependencies"]
29+
# Updates Shakapacker to minimum supported version in either dependencies or devDependencies
30+
def update_shakapacker_dependency(deps, dev_deps)
31+
if dev_deps&.key?("shakapacker")
32+
dev_deps["shakapacker"] = ExampleType::MINIMUM_SHAKAPACKER_VERSION
33+
elsif deps&.key?("shakapacker")
34+
deps["shakapacker"] = ExampleType::MINIMUM_SHAKAPACKER_VERSION
35+
end
36+
end
3437

35-
# Update React versions to minimum supported
36-
if deps
37-
deps["react"] = ExampleType::MINIMUM_REACT_VERSION
38-
deps["react-dom"] = ExampleType::MINIMUM_REACT_VERSION
39-
# Shakapacker 8.2.0 requires webpack-assets-manifest ^5.x
40-
deps["webpack-assets-manifest"] = "^5.0.6" if deps.key?("webpack-assets-manifest")
41-
end
38+
# Updates dependencies in package.json to use minimum supported versions
39+
def update_package_json_versions(package_json_path)
40+
return unless File.exist?(package_json_path)
4241

43-
# Shakapacker 8.2.0 requires webpack-assets-manifest ^5.x (check devDependencies too)
44-
dev_deps["webpack-assets-manifest"] = "^5.0.6" if dev_deps&.key?("webpack-assets-manifest")
42+
begin
43+
package_json = JSON.parse(File.read(package_json_path))
44+
rescue JSON::ParserError => e
45+
puts " ERROR: Failed to parse #{package_json_path}: #{e.message}"
46+
raise
47+
end
4548

46-
# Update Shakapacker to minimum supported version in package.json
47-
if dev_deps&.key?("shakapacker")
48-
dev_deps["shakapacker"] = ExampleType::MINIMUM_SHAKAPACKER_VERSION
49-
elsif deps&.key?("shakapacker")
50-
deps["shakapacker"] = ExampleType::MINIMUM_SHAKAPACKER_VERSION
51-
end
49+
deps = package_json["dependencies"]
50+
dev_deps = package_json["devDependencies"]
5251

53-
File.write(package_json_path, "#{JSON.pretty_generate(package_json)}\n")
54-
end
52+
update_react_dependencies(deps)
53+
# Shakapacker 8.2.0 requires webpack-assets-manifest ^5.x (check devDependencies too)
54+
dev_deps["webpack-assets-manifest"] = "^5.0.6" if dev_deps&.key?("webpack-assets-manifest")
55+
update_shakapacker_dependency(deps, dev_deps)
5556

56-
# Update Gemfile to pin shakapacker to minimum version
57-
# (must match the npm package version exactly)
58-
gemfile_path = File.join(dir, "Gemfile")
59-
if File.exist?(gemfile_path)
60-
gemfile_content = File.read(gemfile_path)
61-
# Replace any shakapacker gem line with exact version pin
62-
# Handle both single-line: gem 'shakapacker', '>= 8.2.0'
63-
# And multi-line declarations:
64-
# gem 'shakapacker',
65-
# '>= 8.2.0'
66-
gemfile_content = gemfile_content.gsub(
67-
/gem ['"]shakapacker['"][^\n]*(?:\n\s+[^g\n][^\n]*)*$/m,
68-
"gem 'shakapacker', '#{ExampleType::MINIMUM_SHAKAPACKER_VERSION}'"
69-
)
70-
File.write(gemfile_path, gemfile_content)
71-
end
57+
File.write(package_json_path, "#{JSON.pretty_generate(package_json)}\n")
58+
end
59+
60+
# Updates Gemfile to pin shakapacker to minimum version
61+
# (must match the npm package version exactly)
62+
def update_gemfile_versions(gemfile_path)
63+
return unless File.exist?(gemfile_path)
64+
65+
gemfile_content = File.read(gemfile_path)
66+
# Replace any shakapacker gem line with exact version pin
67+
# Handle both single-line: gem 'shakapacker', '>= 8.2.0'
68+
# And multi-line declarations:
69+
# gem 'shakapacker',
70+
# '>= 8.2.0'
71+
gemfile_content = gemfile_content.gsub(
72+
/gem ['"]shakapacker['"][^\n]*(?:\n\s+[^g\n][^\n]*)*$/m,
73+
"gem 'shakapacker', '#{ExampleType::MINIMUM_SHAKAPACKER_VERSION}'"
74+
)
75+
File.write(gemfile_path, gemfile_content)
76+
end
77+
78+
# Updates package.json and Gemfile to use minimum supported versions for compatibility testing
79+
def apply_minimum_versions(dir)
80+
update_package_json_versions(File.join(dir, "package.json"))
81+
update_gemfile_versions(File.join(dir, "Gemfile"))
7282

7383
puts " Updated package.json with minimum versions:"
7484
puts " React: #{ExampleType::MINIMUM_REACT_VERSION}"
7585
puts " Shakapacker: #{ExampleType::MINIMUM_SHAKAPACKER_VERSION}"
7686
end
77-
# rubocop:enable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
7887

7988
# Define tasks for each example type
8089
ExampleType.all[:shakapacker_examples].each do |example_type| # rubocop:disable Metrics/BlockLength
@@ -107,6 +116,8 @@ namespace :shakapacker_examples do # rubocop:disable Metrics/BlockLength
107116
"REACT_ON_RAILS_SKIP_VALIDATION=true #{cmd}"
108117
end
109118
sh_in_dir(example_type.dir, generator_commands)
119+
# Re-run bundle install since dev_tests generator adds rspec-rails and coveralls to Gemfile
120+
bundle_install_in(example_type.dir)
110121

111122
# Apply minimum versions for compatibility testing examples
112123
if example_type.minimum_versions

0 commit comments

Comments
 (0)