Repository navigation
Fix compensation of ERB line numbers on the next major Rails version - #2780
Merged
Merged
Conversation
Rails 8.1 added a newline to compiled ERB output, which ViewComponent compensates for when it compiles a template. The check required `MAJOR >= 8 && MINOR > 0`, which is false on the `.0` release of any later major version, so backtraces from annotated HTML+ERB templates would point one line off there. Nothing is broken on 8.x today. Compare `Rails.gem_version` instead, here and in the tests that skipped on the same condition or compared version strings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VLwjZw7FEdBpwALzxXxXnJ
It lived in a class that skips before Rails 8.1, although the version check it covers doesn't depend on the installed Rails. Also compare the major version directly in the backtrace index tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VLwjZw7FEdBpwALzxXxXnJ
Contributor
Author
|
Thanks ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
Rails 8.1 added a newline to compiled ERB output (rails/rails#53731), and
Template#erb_newline_compensation_needed?corrects line numbers for it. The check is:Nothing is broken today: it's correct for 8.0 and every 8.x after it. But it will be false on the
.0release of the next major version, sinceMINORis 0 there. Line numbers would stop being corrected, and backtraces from annotated HTML+ERB templates would point one line off. The gemspec allows>= 7.1.0with no upper bound, so apps can install that release on day one. This is a small change now to avoid a hard-to-trace bug later.What approach did you choose and why?
The check now compares
Rails.gem_versionwith8.1.0.alpha, so Rails main prereleases of 8.1 still count. The testskips that used the same condition now use the same comparison.rendering_test.rbcomparedRails::VERSION::STRING < "8.0"as strings, which would break at 10.0, so I fixed that too.Anything you want to highlight for special attention from reviewers?
The regression test swaps
Rails::VERSION::MAJOR/MINOR/STRINGinside a block, restores them afterwards, and checks that a hypothetical 9.0.0 compensates and 8.0.3 doesn't. Onmain, it fails for 9.0.0. I couldn't run the suite locally.