Skip to content

Commit eb0b353

Browse files
justin808claude
andcommitted
Fix false positive in async detection for mixed commented/uncommented tags
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 <[email protected]>
1 parent cda9a31 commit eb0b353

File tree

2 files changed

+26
-17
lines changed

2 files changed

+26
-17
lines changed

lib/react_on_rails/doctor.rb

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,29 +1226,21 @@ def file_has_async_pack_tag?(content)
12261226
def content_has_only_commented_async?(content)
12271227
# Check if all occurrences of javascript_pack_tag with :async are in comments
12281228
# Returns true if ONLY commented async usage exists (no active async usage)
1229-
# Note: We need to check the full content first (for multi-line tags) before line-by-line filtering
12301229

12311230
# First check if there's any javascript_pack_tag with :async in the full content
12321231
return true unless file_has_async_pack_tag?(content)
12331232

1234-
# Now check line-by-line to see if all occurrences are commented
1235-
# For multi-line tags, we check if the starting line is commented
1236-
has_uncommented_async = content.each_line.any? do |line|
1237-
# Skip lines that don't contain javascript_pack_tag
1238-
next unless line.include?("javascript_pack_tag")
1239-
1240-
# Skip ERB comments (<%# ... %>)
1241-
next if line.match?(ERB_COMMENT_PATTERN)
1242-
# Skip HAML comments (-# ...)
1243-
next if line.match?(HAML_COMMENT_PATTERN)
1244-
# Skip HTML comments (<!-- ... -->)
1245-
next if line.match?(HTML_COMMENT_PATTERN)
1246-
1247-
# If we reach here, this line has an uncommented javascript_pack_tag
1248-
true
1233+
# Strategy: Remove all commented lines, then check if any :async remains
1234+
# This handles both single-line and multi-line tags correctly
1235+
uncommented_lines = content.each_line.reject do |line|
1236+
line.match?(ERB_COMMENT_PATTERN) ||
1237+
line.match?(HAML_COMMENT_PATTERN) ||
1238+
line.match?(HTML_COMMENT_PATTERN)
12491239
end
12501240

1251-
!has_uncommented_async
1241+
uncommented_content = uncommented_lines.join
1242+
# If no async found in uncommented content, all async usage was commented
1243+
!file_has_async_pack_tag?(uncommented_content)
12521244
end
12531245

12541246
def config_has_async_loading_strategy?

spec/lib/react_on_rails/doctor_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,23 @@
342342
end
343343
end
344344

345+
context "when file has uncommented pack tag without :async and commented one with :async" do
346+
before do
347+
allow(Dir).to receive(:glob).with("app/views/**/*.erb")
348+
.and_return(["app/views/layouts/application.html.erb"])
349+
allow(Dir).to receive(:glob).with("app/views/**/*.haml").and_return([])
350+
allow(File).to receive(:exist?).with("app/views/layouts/application.html.erb").and_return(true)
351+
allow(File).to receive(:read).with("app/views/layouts/application.html.erb")
352+
.and_return('<%= javascript_pack_tag "app", defer: true %>
353+
<%# javascript_pack_tag "other", :async %>')
354+
end
355+
356+
it "does not return files (only commented line has :async)" do
357+
files = doctor.send(:scan_view_files_for_async_pack_tag)
358+
expect(files).to be_empty
359+
end
360+
end
361+
345362
context "when async is only in ERB comments" do
346363
before do
347364
allow(Dir).to receive(:glob).with("app/views/**/*.erb")

0 commit comments

Comments
 (0)