Enable configuring Action View's render tracker #55194
Merged
Conversation
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
skipkayhil
force-pushed
the
hm-configurable-render-tracker
branch
from
7ea055a to
f520bc7
Compare
|
This makes sense to me. Is the plan that we would make the Ruby parser implementation default? I think that we should. Also what about #55165? It seems like we should merge it as well. |
🤦 I misread the PR. I see that indeed this makes the Ruby parser implementation default. I think that's good, lets ship it. |
In Rails 7, a new render tracking implementation was [added][1] to Rails: the RipperTracker. From that commit message: > Using a parser finds dependencies that would otherwise be difficult to > find with the regular expressions. It should also theoretically work > with other template systems since it operates on the compiled template > instead of the contents of the file. It was initially made the default render tracker, but that change was later [backed out][2]. Since then, the Prism parser was released and made the default Ruby parser. A Prism render tracker implementation was also [added][3] to Rails in 7.2. Until now, there hasn't been a "blessed" way for applications to try out the new tracker implementations (you would have to go through private API). This commit adds configuration to not only make it easy for applications to use the new Ruby-parser-based trackers, but it also makes these new trackers the default for new applications on 8.1. The new trackers are not only better at finding `render` calls, they are also much easier to maintain. When I [added][4] trailing interpolation tracking to the three trackers, the Prism tracker was by far the easiest to work with. [1]: 897b9bf [2]: 41c0681 [3]: d743a2b [4]: 13df150
skipkayhil
force-pushed
the
hm-configurable-render-tracker
branch
from
f520bc7 to
89fddd3
Compare
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.
Motivation / Background
Ref #55161
In Rails 7, a new render tracking implementation was added to Rails: the RipperTracker. From that commit message:
It was initially made the default render tracker, but that change was later backed out.
Since then, the Prism parser was released and made the default Ruby parser. A Prism render tracker implementation was also added to Rails in 7.2.
Detail
Until now, there hasn't been a "blessed" way for applications to try out the new tracker implementations (you would have to go through private API). This commit adds configuration to not only make it easy for applications to use the new Ruby-parser-based trackers, but it also makes these new trackers the default for new applications on 8.1.
The new trackers are not only better at finding
rendercalls, they are also much easier to maintain. When I added trailing interpolation tracking to the three trackers, the Prism tracker was by far the easiest to work with.Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]