From a1aab6f3d0d40bf56c99ba3d0a30d5f35c1d81ea Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 12 Nov 2025 17:16:58 -1000 Subject: [PATCH 1/6] Add doctor checks to detect :async usage without React on Rails Pro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds proactive detection of :async loading strategy usage in projects without React on Rails Pro, which can cause component registration race conditions. The doctor now checks for: 1. javascript_pack_tag with :async in view files 2. config.generated_component_packs_loading_strategy = :async in initializer When detected without Pro, provides clear guidance: - Upgrade to React on Rails Pro (recommended) - Change to :defer or :sync loading strategy This complements PR #1993's configuration validation by adding runtime doctor checks that help users identify and fix async usage issues during development. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- lib/react_on_rails/doctor.rb | 67 +++++++++++ spec/lib/react_on_rails/doctor_spec.rb | 156 +++++++++++++++++++++++++ 2 files changed, 223 insertions(+) diff --git a/lib/react_on_rails/doctor.rb b/lib/react_on_rails/doctor.rb index efe5f80006..98eadfc6af 100644 --- a/lib/react_on_rails/doctor.rb +++ b/lib/react_on_rails/doctor.rb @@ -173,6 +173,7 @@ def check_development check_procfile_dev check_bin_dev_script check_gitignore + check_async_usage end def check_javascript_bundles @@ -1146,6 +1147,72 @@ def safe_display_config_value(label, config, method_name) checker.add_info(" #{label}: ") end end + + def check_async_usage + # When Pro is installed, async is fully supported and is the default behavior + # No need to check for async usage in this case + return if ReactOnRails::Utils.react_on_rails_pro? + + async_issues = [] + + # Check 1: javascript_pack_tag with :async in view files + view_files_with_async = scan_view_files_for_async_pack_tag + unless view_files_with_async.empty? + async_issues << "javascript_pack_tag with :async found in view files:" + view_files_with_async.each do |file| + async_issues << " • #{file}" + end + end + + # Check 2: generated_component_packs_loading_strategy = :async + if config_has_async_loading_strategy? + async_issues << "config.generated_component_packs_loading_strategy = :async in initializer" + end + + return if async_issues.empty? + + # Report errors if async usage is found without Pro + checker.add_error("🚫 :async usage detected without React on Rails Pro") + async_issues.each { |issue| checker.add_error(" #{issue}") } + checker.add_info(" 💡 :async can cause race conditions. Options:") + checker.add_info(" 1. Upgrade to React on Rails Pro (recommended for :async support)") + checker.add_info(" 2. Change to :defer or :sync loading strategy") + checker.add_info(" 📖 https://www.shakacode.com/react-on-rails/docs/guides/configuration/") + end + + def scan_view_files_for_async_pack_tag + files_with_async = [] + + # Scan app/views for .erb and .haml files + view_patterns = ["app/views/**/*.erb", "app/views/**/*.haml"] + + view_patterns.each do |pattern| + Dir.glob(pattern).each do |file| + next unless File.exist?(file) + + content = File.read(file) + # Look for javascript_pack_tag with :async or "async" + if content.match?(/javascript_pack_tag.*:async/) || content.match?(/javascript_pack_tag.*["']async["']/) + files_with_async << relativize_path(file) + end + end + end + + files_with_async + rescue StandardError + [] + end + + def config_has_async_loading_strategy? + config_path = "config/initializers/react_on_rails.rb" + return false unless File.exist?(config_path) + + content = File.read(config_path) + # Check if generated_component_packs_loading_strategy is set to :async + content.match?(/config\.generated_component_packs_loading_strategy\s*=\s*:async/) + rescue StandardError + false + end end # rubocop:enable Metrics/ClassLength end diff --git a/spec/lib/react_on_rails/doctor_spec.rb b/spec/lib/react_on_rails/doctor_spec.rb index beab9b83dd..002eefd2d1 100644 --- a/spec/lib/react_on_rails/doctor_spec.rb +++ b/spec/lib/react_on_rails/doctor_spec.rb @@ -188,4 +188,160 @@ end end end + + describe "#check_async_usage" do + let(:checker) { instance_double(ReactOnRails::SystemChecker) } + + before do + allow(doctor).to receive(:checker).and_return(checker) + allow(checker).to receive_messages(add_error: true, add_warning: true, add_info: true) + allow(File).to receive(:exist?).and_call_original + allow(Dir).to receive(:glob).and_return([]) + end + + context "when Pro gem is installed" do + before do + allow(ReactOnRails::Utils).to receive(:react_on_rails_pro?).and_return(true) + end + + it "skips the check" do + doctor.send(:check_async_usage) + expect(checker).not_to have_received(:add_error) + expect(checker).not_to have_received(:add_warning) + end + end + + context "when Pro gem is not installed" do + before do + allow(ReactOnRails::Utils).to receive(:react_on_rails_pro?).and_return(false) + end + + context "when async is used in view files" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb").and_return(["app/views/layouts/application.html.erb"]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.erb") + .and_return('<%= javascript_pack_tag "application", :async %>') + allow(File).to receive(:exist?).with("config/initializers/react_on_rails.rb").and_return(false) + allow(doctor).to receive(:relativize_path).with("app/views/layouts/application.html.erb") + .and_return("app/views/layouts/application.html.erb") + end + + it "reports an error" do + doctor.send(:check_async_usage) + expect(checker).to have_received(:add_error).with("🚫 :async usage detected without React on Rails Pro") + expect(checker).to have_received(:add_error) + .with(" javascript_pack_tag with :async found in view files:") + end + end + + context "when generated_component_packs_loading_strategy is :async" do + before do + allow(File).to receive(:exist?).with("config/initializers/react_on_rails.rb").and_return(true) + allow(File).to receive(:read).with("config/initializers/react_on_rails.rb") + .and_return("config.generated_component_packs_loading_strategy = :async") + end + + it "reports an error" do + doctor.send(:check_async_usage) + expect(checker).to have_received(:add_error).with("🚫 :async usage detected without React on Rails Pro") + expect(checker).to have_received(:add_error) + .with(" config.generated_component_packs_loading_strategy = :async in initializer") + end + end + + context "when no async usage is detected" do + before do + allow(File).to receive(:exist?).with("config/initializers/react_on_rails.rb").and_return(true) + allow(File).to receive(:read).with("config/initializers/react_on_rails.rb") + .and_return("config.generated_component_packs_loading_strategy = :defer") + end + + it "does not report any issues" do + doctor.send(:check_async_usage) + expect(checker).not_to have_received(:add_error) + expect(checker).not_to have_received(:add_warning) + end + end + end + end + + describe "#scan_view_files_for_async_pack_tag" do + before do + allow(Dir).to receive(:glob).and_call_original + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:read).and_call_original + end + + context "when view files contain javascript_pack_tag with :async" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb") + .and_return(["app/views/layouts/application.html.erb"]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.erb") + .and_return('<%= javascript_pack_tag "app", :async %>') + allow(doctor).to receive(:relativize_path).with("app/views/layouts/application.html.erb") + .and_return("app/views/layouts/application.html.erb") + end + + it "returns files with async" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to include("app/views/layouts/application.html.erb") + end + end + + context "when view files do not contain async" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb") + .and_return(["app/views/layouts/application.html.erb"]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.erb") + .and_return('<%= javascript_pack_tag "app" %>') + end + + it "returns empty array" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to be_empty + end + end + end + + describe "#config_has_async_loading_strategy?" do + context "when config file has :async strategy" do + before do + allow(File).to receive(:exist?).with("config/initializers/react_on_rails.rb").and_return(true) + allow(File).to receive(:read).with("config/initializers/react_on_rails.rb") + .and_return("config.generated_component_packs_loading_strategy = :async") + end + + it "returns true" do + expect(doctor.send(:config_has_async_loading_strategy?)).to be true + end + end + + context "when config file has different strategy" do + before do + allow(File).to receive(:exist?).with("config/initializers/react_on_rails.rb").and_return(true) + allow(File).to receive(:read).with("config/initializers/react_on_rails.rb") + .and_return("config.generated_component_packs_loading_strategy = :defer") + end + + it "returns false" do + expect(doctor.send(:config_has_async_loading_strategy?)).to be false + end + end + + context "when config file does not exist" do + before do + allow(File).to receive(:exist?).with("config/initializers/react_on_rails.rb").and_return(false) + end + + it "returns false" do + expect(doctor.send(:config_has_async_loading_strategy?)).to be false + end + end + end end From 036aaa649b5cfc3a5437812315ea9a256b916336 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Wed, 12 Nov 2025 20:34:59 -1000 Subject: [PATCH 2/6] Improve doctor async checks: better error handling and comment filtering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addressed code review feedback to make the async usage detection more robust and accurate: 1. Enhanced error handling: - Replace broad StandardError catches with specific exceptions (Errno::ENOENT, Encoding::InvalidByteSequenceError, Encoding::UndefinedConversionError) - Add debug logging when errors occur (if Rails logger available) 2. Add comment filtering to reduce false positives: - Filter out commented Ruby lines in config files (lines starting with #) - Skip ERB comments (<%# ... %>), HAML comments (-# ...), and HTML comments () in view files - Prevent false alarms from commented-out code 3. Refactor for code quality: - Extract scan_pattern_for_async and file_has_async_pack_tag? helpers - Reduce cyclomatic complexity in scan_view_files_for_async_pack_tag - All RuboCop checks pass 4. Expand test coverage: - Add tests for ERB, HAML, and HTML comment scenarios - Add test for commented config with async strategy - All 13 async-related tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- lib/react_on_rails/doctor.rb | 59 +++++++++++++++++-------- spec/lib/react_on_rails/doctor_spec.rb | 60 ++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 17 deletions(-) diff --git a/lib/react_on_rails/doctor.rb b/lib/react_on_rails/doctor.rb index 98eadfc6af..6b1147dc0a 100644 --- a/lib/react_on_rails/doctor.rb +++ b/lib/react_on_rails/doctor.rb @@ -1181,26 +1181,46 @@ def check_async_usage end def scan_view_files_for_async_pack_tag - files_with_async = [] - - # Scan app/views for .erb and .haml files view_patterns = ["app/views/**/*.erb", "app/views/**/*.haml"] + files_with_async = view_patterns.flat_map { |pattern| scan_pattern_for_async(pattern) } + files_with_async.compact + rescue Errno::ENOENT, Encoding::InvalidByteSequenceError, Encoding::UndefinedConversionError => e + # Log the error if Rails logger is available + Rails.logger.debug("Error scanning view files for async: #{e.message}") if defined?(Rails) && Rails.logger + [] + end - view_patterns.each do |pattern| - Dir.glob(pattern).each do |file| - next unless File.exist?(file) + def scan_pattern_for_async(pattern) + Dir.glob(pattern).filter_map do |file| + next unless File.exist?(file) - content = File.read(file) - # Look for javascript_pack_tag with :async or "async" - if content.match?(/javascript_pack_tag.*:async/) || content.match?(/javascript_pack_tag.*["']async["']/) - files_with_async << relativize_path(file) - end - end + content = File.read(file) + next if content_has_only_commented_async?(content) + next unless file_has_async_pack_tag?(content) + + relativize_path(file) + end + end + + def file_has_async_pack_tag?(content) + content.match?(/javascript_pack_tag.*:async/) || content.match?(/javascript_pack_tag.*["']async["']/) + end + + def content_has_only_commented_async?(content) + # Check if all occurrences of javascript_pack_tag with :async are in comments + has_uncommented_async = content.each_line.any? do |line| + # Skip ERB comments (<%# ... %>) + next if line.match?(/<%\s*#.*javascript_pack_tag.*:async/) + # Skip HAML comments (-# ...) + next if line.match?(/^\s*-#.*javascript_pack_tag.*:async/) + # Skip HTML comments () + next if line.match?(//) + + # Check if line has javascript_pack_tag with :async + line.match?(/javascript_pack_tag.*:async/) end - files_with_async - rescue StandardError - [] + !has_uncommented_async end def config_has_async_loading_strategy? @@ -1209,8 +1229,13 @@ def config_has_async_loading_strategy? content = File.read(config_path) # Check if generated_component_packs_loading_strategy is set to :async - content.match?(/config\.generated_component_packs_loading_strategy\s*=\s*:async/) - rescue StandardError + # Filter out commented lines to avoid false positives + content.each_line.any? do |line| + line !~ /^\s*#/ && line.match?(/config\.generated_component_packs_loading_strategy\s*=\s*:async/) + end + rescue Errno::ENOENT, Encoding::InvalidByteSequenceError, Encoding::UndefinedConversionError => e + # Log the error if Rails logger is available + Rails.logger.debug("Error checking async loading strategy: #{e.message}") if defined?(Rails) && Rails.logger false end end diff --git a/spec/lib/react_on_rails/doctor_spec.rb b/spec/lib/react_on_rails/doctor_spec.rb index 002eefd2d1..fa555eae12 100644 --- a/spec/lib/react_on_rails/doctor_spec.rb +++ b/spec/lib/react_on_rails/doctor_spec.rb @@ -307,6 +307,54 @@ expect(files).to be_empty end end + + context "when async is only in ERB comments" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb") + .and_return(["app/views/layouts/application.html.erb"]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.erb") + .and_return('<%# javascript_pack_tag "app", :async %>') + end + + it "returns empty array" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to be_empty + end + end + + context "when async is only in HAML comments" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml") + .and_return(["app/views/layouts/application.html.haml"]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.haml").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.haml") + .and_return('-# javascript_pack_tag "app", :async') + end + + it "returns empty array" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to be_empty + end + end + + context "when async is only in HTML comments" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb") + .and_return(["app/views/layouts/application.html.erb"]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.erb") + .and_return('') + end + + it "returns empty array" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to be_empty + end + end end describe "#config_has_async_loading_strategy?" do @@ -343,5 +391,17 @@ expect(doctor.send(:config_has_async_loading_strategy?)).to be false end end + + context "when :async strategy is commented out" do + before do + allow(File).to receive(:exist?).with("config/initializers/react_on_rails.rb").and_return(true) + allow(File).to receive(:read).with("config/initializers/react_on_rails.rb") + .and_return("# config.generated_component_packs_loading_strategy = :async") + end + + it "returns false" do + expect(doctor.send(:config_has_async_loading_strategy?)).to be false + end + end end end From 4cdff17bb127de6a7f74d492dab9b4f1b99a03c0 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Thu, 13 Nov 2025 12:16:28 -1000 Subject: [PATCH 3/6] Improve doctor async checks: enhance regex precision and add comprehensive tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove overly broad string form regex that could cause false positives - Add support for both :async symbol and async: true hash syntax - Add word boundary \b to prevent matching :async_mode or similar - Improve multi-line javascript_pack_tag detection - Add comprehensive inline documentation with examples - Add test for async: true pattern detection - Add test to confirm defer: "async" does not trigger false positive - Add test for multi-line javascript_pack_tag calls - Improve comment filtering logic to handle multi-line tags correctly - Use String#include? for better performance (RuboCop compliance) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- lib/react_on_rails/doctor.rb | 50 +++++++++++++++++++------ spec/lib/react_on_rails/doctor_spec.rb | 52 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 11 deletions(-) diff --git a/lib/react_on_rails/doctor.rb b/lib/react_on_rails/doctor.rb index 6b1147dc0a..6de38c6a9b 100644 --- a/lib/react_on_rails/doctor.rb +++ b/lib/react_on_rails/doctor.rb @@ -1203,21 +1203,44 @@ def scan_pattern_for_async(pattern) end def file_has_async_pack_tag?(content) - content.match?(/javascript_pack_tag.*:async/) || content.match?(/javascript_pack_tag.*["']async["']/) + # Match javascript_pack_tag with :async symbol or async: true hash syntax + # Examples that should match: + # - javascript_pack_tag "app", :async + # - javascript_pack_tag "app", async: true + # - javascript_pack_tag "app", :async, other_option: value + # Examples that should NOT match: + # - javascript_pack_tag "app", defer: "async" (async is a string value, not the option) + # - javascript_pack_tag "app", :defer + # Note: Theoretical edge case `data: { async: true }` would match but is extremely unlikely + # in real code and represents a harmless false positive (showing a warning when not needed) + # Use word boundary \b to ensure :async is not part of a longer symbol like :async_mode + # [^<]* allows matching across newlines within ERB tags but stops at closing ERB tag + content.match?(/javascript_pack_tag[^<]*(?::async\b|async:\s*true)/) end def content_has_only_commented_async?(content) # Check if all occurrences of javascript_pack_tag with :async are in comments + # Returns true if ONLY commented async usage exists (no active async usage) + # Note: We need to check the full content first (for multi-line tags) before line-by-line filtering + + # First check if there's any javascript_pack_tag with :async in the full content + return true unless file_has_async_pack_tag?(content) + + # Now check line-by-line to see if all occurrences are commented + # For multi-line tags, we check if the starting line is commented has_uncommented_async = content.each_line.any? do |line| - # Skip ERB comments (<%# ... %>) - next if line.match?(/<%\s*#.*javascript_pack_tag.*:async/) - # Skip HAML comments (-# ...) - next if line.match?(/^\s*-#.*javascript_pack_tag.*:async/) - # Skip HTML comments () - next if line.match?(//) + # Skip lines that don't contain javascript_pack_tag + next unless line.include?("javascript_pack_tag") - # Check if line has javascript_pack_tag with :async - line.match?(/javascript_pack_tag.*:async/) + # Skip ERB comments (<%# ... %>) - matches ERB comment opening with optional whitespace + next if line.match?(/<%\s*#.*javascript_pack_tag/) + # Skip HAML comments (-# ...) - matches line-starting HAML comments + next if line.match?(/^\s*-#.*javascript_pack_tag/) + # Skip HTML comments () - matches complete HTML comment blocks + next if line.match?(/) - matches complete HTML comment blocks - next if line.match?(/) + next if line.match?(HTML_COMMENT_PATTERN) # If we reach here, this line has an uncommented javascript_pack_tag true @@ -1263,9 +1268,15 @@ def config_has_async_loading_strategy? end rescue Errno::ENOENT, Encoding::InvalidByteSequenceError, Encoding::UndefinedConversionError => e # Log the error if Rails logger is available - Rails.logger.debug("Error checking async loading strategy: #{e.message}") if defined?(Rails) && Rails.logger + log_debug("Error checking async loading strategy: #{e.message}") false end + + def log_debug(message) + return unless defined?(Rails.logger) && Rails.logger + + Rails.logger.debug(message) + end end # rubocop:enable Metrics/ClassLength end From eb0b3536e44651a14eb96640689bebdea6d05fec Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 15 Nov 2025 17:24:14 -1000 Subject: [PATCH 5/6] Fix false positive in async detection for mixed commented/uncommented tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical bug fix: The content_has_only_commented_async? method was incorrectly flagging files that had: - Uncommented javascript_pack_tag WITHOUT :async - Commented javascript_pack_tag WITH :async The previous implementation checked if ANY uncommented javascript_pack_tag existed, not specifically ones with :async. This caused false positives. Solution: Refactored to remove all commented lines first, then check if :async exists in the remaining content. This handles both single-line and multi-line tags correctly and is more robust. Added comprehensive test case to prevent regression. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- lib/react_on_rails/doctor.rb | 26 +++++++++----------------- spec/lib/react_on_rails/doctor_spec.rb | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/lib/react_on_rails/doctor.rb b/lib/react_on_rails/doctor.rb index dd3cdbac48..244bdb0e57 100644 --- a/lib/react_on_rails/doctor.rb +++ b/lib/react_on_rails/doctor.rb @@ -1226,29 +1226,21 @@ def file_has_async_pack_tag?(content) def content_has_only_commented_async?(content) # Check if all occurrences of javascript_pack_tag with :async are in comments # Returns true if ONLY commented async usage exists (no active async usage) - # Note: We need to check the full content first (for multi-line tags) before line-by-line filtering # First check if there's any javascript_pack_tag with :async in the full content return true unless file_has_async_pack_tag?(content) - # Now check line-by-line to see if all occurrences are commented - # For multi-line tags, we check if the starting line is commented - has_uncommented_async = content.each_line.any? do |line| - # Skip lines that don't contain javascript_pack_tag - next unless line.include?("javascript_pack_tag") - - # Skip ERB comments (<%# ... %>) - next if line.match?(ERB_COMMENT_PATTERN) - # Skip HAML comments (-# ...) - next if line.match?(HAML_COMMENT_PATTERN) - # Skip HTML comments () - next if line.match?(HTML_COMMENT_PATTERN) - - # If we reach here, this line has an uncommented javascript_pack_tag - true + # Strategy: Remove all commented lines, then check if any :async remains + # This handles both single-line and multi-line tags correctly + uncommented_lines = content.each_line.reject do |line| + line.match?(ERB_COMMENT_PATTERN) || + line.match?(HAML_COMMENT_PATTERN) || + line.match?(HTML_COMMENT_PATTERN) end - !has_uncommented_async + uncommented_content = uncommented_lines.join + # If no async found in uncommented content, all async usage was commented + !file_has_async_pack_tag?(uncommented_content) end def config_has_async_loading_strategy? diff --git a/spec/lib/react_on_rails/doctor_spec.rb b/spec/lib/react_on_rails/doctor_spec.rb index c7005e701f..f483b5eb6b 100644 --- a/spec/lib/react_on_rails/doctor_spec.rb +++ b/spec/lib/react_on_rails/doctor_spec.rb @@ -342,6 +342,23 @@ end end + context "when file has uncommented pack tag without :async and commented one with :async" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb") + .and_return(["app/views/layouts/application.html.erb"]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.erb") + .and_return('<%= javascript_pack_tag "app", defer: true %> +<%# javascript_pack_tag "other", :async %>') + end + + it "does not return files (only commented line has :async)" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to be_empty + end + end + context "when async is only in ERB comments" do before do allow(Dir).to receive(:glob).with("app/views/**/*.erb") From 1f61137168ac75eb272ea328f6b82497983ec593 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sat, 15 Nov 2025 17:38:36 -1000 Subject: [PATCH 6/6] Add Slim template support to async detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extended the async detection functionality to support Slim templates in addition to ERB and HAML: - Added SLIM_COMMENT_PATTERN constant for Slim comment syntax (/) - Added app/views/**/*.slim to view file scanning patterns - Included Slim comment filtering in content_has_only_commented_async? - Added comprehensive test cases for Slim files: * Slim files with uncommented :async (should detect) * Slim files with commented :async (should not detect) - Added slim stub mocks to all existing test cases for consistency This ensures users of Slim templates get the same async detection warnings as ERB and HAML users. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- lib/react_on_rails/doctor.rb | 4 ++- spec/lib/react_on_rails/doctor_spec.rb | 49 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/lib/react_on_rails/doctor.rb b/lib/react_on_rails/doctor.rb index 244bdb0e57..c49eca4591 100644 --- a/lib/react_on_rails/doctor.rb +++ b/lib/react_on_rails/doctor.rb @@ -1151,6 +1151,7 @@ def safe_display_config_value(label, config, method_name) # Comment patterns used for filtering out commented async usage ERB_COMMENT_PATTERN = /<%\s*#.*javascript_pack_tag/ HAML_COMMENT_PATTERN = /^\s*-#.*javascript_pack_tag/ + SLIM_COMMENT_PATTERN = %r{^\s*/.*javascript_pack_tag} HTML_COMMENT_PATTERN = /') @@ -407,11 +417,50 @@ end end + context "when async is only in Slim comments" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.slim").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.slim") + .and_return(["app/views/layouts/application.html.slim"]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.slim").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.slim") + .and_return('/ = javascript_pack_tag "app", :async') + end + + it "returns empty array" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to be_empty + end + end + + context "when view files contain Slim javascript_pack_tag with :async" do + before do + allow(Dir).to receive(:glob).with("app/views/**/*.erb").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.slim").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.slim") + .and_return(["app/views/layouts/application.html.slim"]) + allow(File).to receive(:exist?).with("app/views/layouts/application.html.slim").and_return(true) + allow(File).to receive(:read).with("app/views/layouts/application.html.slim") + .and_return('= javascript_pack_tag "app", :async') + allow(doctor).to receive(:relativize_path).with("app/views/layouts/application.html.slim") + .and_return("app/views/layouts/application.html.slim") + end + + it "returns files with async" do + files = doctor.send(:scan_view_files_for_async_pack_tag) + expect(files).to include("app/views/layouts/application.html.slim") + end + end + context "when javascript_pack_tag spans multiple lines" do before do allow(Dir).to receive(:glob).with("app/views/**/*.erb") .and_return(["app/views/layouts/application.html.erb"]) allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([]) + allow(Dir).to receive(:glob).with("app/views/**/*.slim").and_return([]) allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true) allow(File).to receive(:read).with("app/views/layouts/application.html.erb") .and_return("<%= javascript_pack_tag \"app\",\n :async %>")