Skip to content

Fix ERB expression compilation when code contains a heredoc#78

Open
pinzonjulian wants to merge 1 commit intomarcoroth:mainfrom
buildkite:fix/heredoc-expression-wrapping
Open

Fix ERB expression compilation when code contains a heredoc#78
pinzonjulian wants to merge 1 commit intomarcoroth:mainfrom
buildkite:fix/heredoc-expression-wrapping

Conversation

@pinzonjulian
Copy link

@pinzonjulian pinzonjulian commented Feb 19, 2026

I attempted to use Reactionview in Buildkite's Rails monolith and found a blocking issue related to Heredocs.

When an ERB output tag wraps code in parentheses and that code contains a heredoc:

some_method, <<~A_HEREDOC, some_methods_attributes
  contents
  of
  the heredoc
A HEREDOC

the closing parenthesis must appear on its own line after the heredoc terminator. Without this, the compiled Ruby is syntactically invalid.

This is my first time diving into herb and reactionview so I relied quite a bit on AI for this as I get more comfortable. However, I reviewed the code thoroughly and tested it in CI in Buildkite where I was able to reduce failed specs from ~700 to ~60.

This is a sibling commit of marcoroth/herb#1206

Comment on lines +69 to +71
@src << "(" << code
@src << "\n" if code.match?(/<<[~-]?\s*['"`]?\w/)
@src << ")"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could just assume to always add a newline, no matter what:

Suggested change
@src << "(" << code
@src << "\n" if code.match?(/<<[~-]?\s*['"`]?\w/)
@src << ")"
@src << "(" << code << "\n)"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me try it in CI in our own patch; I'll report back

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked well

Copy link
Owner

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pinzonjulian for opening this!

Once we have this merged here, I'd also love to bring over the same document/test to the Herb test suite, just to make sure we also handle that correctly in Herb itself and "lock in" the behavior using a snapshot test.

@pinzonjulian pinzonjulian force-pushed the fix/heredoc-expression-wrapping branch from 1276bad to fb4e8cd Compare February 19, 2026 02:23
marcoroth pushed a commit to marcoroth/herb that referenced this pull request Feb 19, 2026
…1206)

When an ERB output tag contains a heredoc (e.g. `<%= method_call
<<~GRAPHQL, variables %>`), the closing parenthesis in the compiled Ruby
must appear on its own line after the heredoc terminator. Without this,
the compiled Ruby is syntactically invalid.

This is a sibling commit to the one introduced in
marcoroth/reactionview#78
When an ERB output tag wraps code in parentheses and that code contains
a heredoc (e.g. `method_call <<~GRAPHQL, variables`), the closing
parenthesis must appear on its own line after the heredoc terminator.
Without this, the compiled Ruby is syntactically invalid.

Always insert a newline before the closing parenthesis unconditionally,
since Ruby ignores trailing newlines in expressions.

Includes a snapshot test for the compiled output.

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019c7333-8beb-715a-9183-8434cd5fb15a
@pinzonjulian pinzonjulian force-pushed the fix/heredoc-expression-wrapping branch from fb4e8cd to ad04e49 Compare February 19, 2026 02:23
marcoroth added a commit to marcoroth/herb that referenced this pull request Feb 19, 2026
Inspired by
marcoroth/reactionview#78 (comment)

This pull request updates our `assert_compiled_snapshot` assertion
helper to make sure the compiled snapshot is valid Ruby by passing the
engine output to Prism and checking for syntax errors.

Co-authored-by: Julián Pinzón Eslava <julian@buildkite.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants