Skip to content

Commit f90da27

Browse files
justin808claude
andcommitted
Improve doctor output with contextual and actionable recommendations
- Fix absolute path concatenation bug in server bundle detection - Make Next Steps section contextual based on project setup: * Different suggestions based on errors vs warnings vs healthy setup * Detects Procfile.dev for ./bin/dev vs manual server commands * Suggests appropriate test commands based on available test suites * Recommends asset building only when relevant - Hide Recommendations section when no errors/warnings exist - Remove generic, unhelpful advice in favor of specific, actionable steps Doctor output is now much more useful and tailored to each project's actual state. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 9ce3af8 commit f90da27

File tree

2 files changed

+99
-8
lines changed

2 files changed

+99
-8
lines changed

lib/react_on_rails/doctor.rb

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def run_diagnosis
5656
print_header
5757
run_all_checks
5858
print_summary
59-
print_recommendations if @checker.errors? || @checker.warnings?
59+
print_recommendations if should_show_recommendations?
6060

6161
exit_with_status
6262
end
@@ -239,21 +239,92 @@ def print_recommendations
239239
puts
240240
end
241241

242+
print_next_steps
243+
end
244+
245+
def should_show_recommendations?
246+
# Only show recommendations if there are actual issues or actionable improvements
247+
checker.errors? || checker.warnings?
248+
end
249+
250+
def print_next_steps
242251
puts Rainbow("Next Steps:").blue.bold
243-
puts "• Run tests to verify everything works: bundle exec rspec"
244-
puts "• Start development server: ./bin/dev (if using Procfile.dev)"
245-
puts "• Check React on Rails documentation: https://github.com/shakacode/react_on_rails"
252+
253+
if checker.errors?
254+
puts "• Fix critical errors above before proceeding"
255+
puts "• Run doctor again to verify fixes: rake react_on_rails:doctor"
256+
elsif checker.warnings?
257+
puts "• Address warnings above for optimal setup"
258+
puts "• Run doctor again to verify improvements: rake react_on_rails:doctor"
259+
else
260+
puts "• Your setup is healthy! Consider these development workflow steps:"
261+
end
262+
263+
# Contextual suggestions based on what exists
264+
if File.exist?("Procfile.dev")
265+
puts "• Start development with: ./bin/dev"
266+
else
267+
puts "• Start Rails server: bin/rails server"
268+
puts "• Start webpack dev server: bin/shakapacker-dev-server (in separate terminal)"
269+
end
270+
271+
# Test suggestions based on what's available
272+
test_suggestions = []
273+
test_suggestions << "bundle exec rspec" if File.exist?("spec")
274+
test_suggestions << "npm test" if has_npm_test_script?
275+
test_suggestions << "yarn test" if has_yarn_test_script?
276+
277+
if test_suggestions.any?
278+
puts "• Run tests: #{test_suggestions.join(' or ')}"
279+
end
280+
281+
# Build suggestions
282+
if checker.messages.any? { |msg| msg[:content].include?("server bundle") }
283+
puts "• Build assets: bin/shakapacker or npm run build"
284+
end
285+
286+
puts "• Documentation: https://github.com/shakacode/react_on_rails"
246287
puts
247288
end
248289

290+
def has_npm_test_script?
291+
return false unless File.exist?("package.json")
292+
293+
begin
294+
package_json = JSON.parse(File.read("package.json"))
295+
test_script = package_json.dig("scripts", "test")
296+
test_script && !test_script.empty?
297+
rescue StandardError
298+
false
299+
end
300+
end
301+
302+
def has_yarn_test_script?
303+
has_npm_test_script? && system("which yarn > /dev/null 2>&1")
304+
end
305+
249306
def determine_server_bundle_path
250307
# Try to use Shakapacker gem API to get configuration
251308
begin
252309
require "shakapacker"
253-
source_path = Shakapacker.config.source_path
254-
source_entry_path = Shakapacker.config.source_entry_path
310+
311+
# Get the source path relative to Rails root
312+
source_path = Shakapacker.config.source_path.to_s
313+
source_entry_path = Shakapacker.config.source_entry_path.to_s
255314
server_bundle_filename = get_server_bundle_filename
256315

316+
# If source_path is absolute, make it relative to current directory
317+
if source_path.start_with?("/")
318+
# Convert absolute path to relative by removing the Rails root
319+
rails_root = Dir.pwd
320+
if source_path.start_with?(rails_root)
321+
source_path = source_path.sub("#{rails_root}/", "")
322+
else
323+
# If it's not under Rails root, just use the basename
324+
source_path = File.basename(source_path)
325+
end
326+
end
327+
257328
File.join(source_path, source_entry_path, server_bundle_filename)
258329
rescue LoadError, NameError, StandardError
259330
# Fallback to default paths if Shakapacker is not available or configured

spec/lib/react_on_rails/doctor_spec.rb

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
# Mock the new server bundle path methods
3131
allow(doctor).to receive(:determine_server_bundle_path).and_return("app/javascript/packs/server-bundle.js")
3232
allow(doctor).to receive(:get_server_bundle_filename).and_return("server-bundle.js")
33+
allow(doctor).to receive(:has_npm_test_script?).and_return(false)
34+
allow(doctor).to receive(:has_yarn_test_script?).and_return(false)
3335

3436
# Mock the checker to avoid actual system calls
3537
checker = instance_double(ReactOnRails::SystemChecker)
@@ -82,7 +84,7 @@
8284
let(:doctor) { described_class.new }
8385

8486
describe "#determine_server_bundle_path" do
85-
context "when Shakapacker gem is available" do
87+
context "when Shakapacker gem is available with relative paths" do
8688
let(:shakapacker_config) { double(source_path: "client/app", source_entry_path: "packs") }
8789

8890
before do
@@ -92,7 +94,25 @@
9294
allow(doctor).to receive(:get_server_bundle_filename).and_return("server-bundle.js")
9395
end
9496

95-
it "uses Shakapacker API configuration" do
97+
it "uses Shakapacker API configuration with relative paths" do
98+
path = doctor.send(:determine_server_bundle_path)
99+
expect(path).to eq("client/app/packs/server-bundle.js")
100+
end
101+
end
102+
103+
context "when Shakapacker gem is available with absolute paths" do
104+
let(:rails_root) { "/Users/test/myapp" }
105+
let(:shakapacker_config) { double(source_path: "#{rails_root}/client/app", source_entry_path: "packs") }
106+
107+
before do
108+
shakapacker_module = double("Shakapacker", config: shakapacker_config)
109+
stub_const("Shakapacker", shakapacker_module)
110+
allow(doctor).to receive(:require).with("shakapacker").and_return(true)
111+
allow(doctor).to receive(:get_server_bundle_filename).and_return("server-bundle.js")
112+
allow(Dir).to receive(:pwd).and_return(rails_root)
113+
end
114+
115+
it "converts absolute paths to relative paths" do
96116
path = doctor.send(:determine_server_bundle_path)
97117
expect(path).to eq("client/app/packs/server-bundle.js")
98118
end

0 commit comments

Comments
 (0)