Skip to content

Commit ad2a54c

Browse files
authored
Engine: Avoid applying debug spans inside javascript_tag blocks (#953)
This pull request fixes #952. `javascript_tag` blocks were still receiving debug spans when debug mode was enabled, even though raw `<script>` tags were already excluded. This caused JavaScript code inside the helper to break. To address this, I introduced an `@erb_block_stack` to track the current ERB block and extended `in_excluded_context?` to detect `javascript_tag` calls within ERB. When inside such a block, debug spans are skipped. A snapshot test has been added to ensure that ERB expressions inside `javascript_tag` do not receive debug spans. If you have any feedback on the approach or implementation, I’d be happy to adjust!
1 parent a43c7e6 commit ad2a54c

File tree

4 files changed

+49
-1
lines changed

4 files changed

+49
-1
lines changed

lib/herb/engine/debug_visitor.rb

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ def initialize(file_path: nil, project_path: nil)
2525
@relative_file_path = calculate_relative_path
2626
@top_level_elements = [] #: Array[Herb::AST::HTMLElementNode]
2727
@element_stack = [] #: Array[String]
28+
@erb_block_stack = [] #: Array[Herb::AST::ERBBlockNode]
2829
@debug_attributes_applied = false
2930
@in_attribute = false
3031
@in_html_comment = false
@@ -83,6 +84,12 @@ def visit_erb_yield_node(_node)
8384
nil
8485
end
8586

87+
def visit_erb_block_node(node)
88+
@erb_block_stack.push(node)
89+
super
90+
@erb_block_stack.pop
91+
end
92+
8693
private
8794

8895
def calculate_relative_path
@@ -295,7 +302,11 @@ def in_script_or_style_context?
295302

296303
def in_excluded_context?
297304
excluded_tags = ["script", "style", "head", "textarea", "pre"]
298-
excluded_tags.any? { |tag| @element_stack.include?(tag) }
305+
return true if excluded_tags.any? { |tag| @element_stack.include?(tag) }
306+
307+
return true if @erb_block_stack.any? { |node| javascript_tag?(node.content.value.strip) }
308+
309+
false
299310
end
300311

301312
def erb_output?(opening)
@@ -332,6 +343,18 @@ def complex_rails_helper?(code)
332343

333344
false
334345
end
346+
347+
# TODO: Rewrite using Prism Nodes once available
348+
def javascript_tag?(code)
349+
cleaned_code = code.strip.gsub(/\s+/, " ")
350+
351+
return true if cleaned_code.match?(/\bjavascript_tag\s.*do\s*$/) ||
352+
cleaned_code.match?(/\bjavascript_tag\s.*\{\s*$/) ||
353+
cleaned_code.match?(/\bjavascript_tag\(.*do\s*$/) ||
354+
cleaned_code.match?(/\bjavascript_tag\(.*\{\s*$/)
355+
356+
false
357+
end
335358
end
336359
end
337360
end

sig/herb/engine/debug_visitor.rbs

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/engine/debug_mode_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,5 +391,16 @@ class DebugModeTest < Minitest::Spec
391391
test "regular div content still gets debug spans after excluded context tests" do
392392
assert_compiled_snapshot("<div><%= @content %></div>", debug: true)
393393
end
394+
395+
test "javascript_tag content erb expressions do NOT get debug spans" do
396+
template = <<~ERB
397+
<%= javascript_tag do %>
398+
var userId = <%= @user.id %>;
399+
var name = "<%= @user.name %>";
400+
<% end %>
401+
ERB
402+
403+
assert_compiled_snapshot(template, debug: true)
404+
end
394405
end
395406
end

test/snapshots/engine/debug_mode_test/test_0046_javascript_tag_content_erb_expressions_do_NOT_get_debug_spans_a98f57bcf7299e63c0fed690e6457510.txt

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)