Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix duplicate view template loading #51712

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

niklas-hasselmeyer
Copy link

Motivation / Background

Currently, when rendering an ActionView::Template, the template source is loaded from the disk twice.

During one rendering process, Template#source is called at least twice (once in encode! and once in strict_locals!), each of these calls runs @source.to_s. As @source is usually a Template::Sources::File, #to_s hits the disk to load the template.

It is unnecessary to load the file from disk more than once just to check whether the # locals: comment is present.

Detail

This Pull Request changes the Template#source so that it memoizes the loaded source.

Additional information

This pull request introduces a small behavior change: The # locals: (...) comment is now removed from the source before passing the source to the handler. It looks like this was the intended behavior all along. Previously the line

self.source.sub!(STRICT_LOCALS_REGEX, "")

had no effect, because the source was reloaded from disk afterwards and the magic comment was present again.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the actionview label May 2, 2024
Currently, when rendering an ActionView::Template, the template source
is loaded from the disk twice.

During one rendering process, Template#source is called at least twice
(once in encode! and once in strict_locals!), each of these calls runs
@source.to_s. As @source is usually a Template::Sources::File, #to_s
hits the disk to load the template.

It is unnecessary to load the file from disk more than once just to
check whether the "# locals:" comment is present.
@niklas-hasselmeyer niklas-hasselmeyer force-pushed the nha/fix-duplicate-template-loading branch from 97ec459 to 2ed2f23 Compare May 2, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant