Engine: Ignoring vendor directory considered harmful - #1508
Open
joelhawksley wants to merge 1 commit into
Open
Conversation
By default, Herb currently ignores the `vendor` directly. This is potentially quite harmful, as it could expose consuming applications to runtime exceptions. For example. We vendor https://github.com/primer/view_components in our application. The dependency includes ViewComponents with ERB files, including a few that will not compile with Herb. If we don't compile the templates in our test environment, those Herb compilation errors will not be caught. I see similar risks for other gems that provide UI, such as as dashboards mounted via an engine. We thankfully discovered this issue and manually added `vendor/**/*.erb` to our `include` configuration, but I'm wary of blankly excluding the directory silently by default, especially when we describe the behavior of `herb analyze` as `Running herb analyze without arguments now defaults to the current directory`. This could make one believe that all subdirectories are being analyzed based on the config in the user's `.herb.yml` file, but in fact it is the combination of `defaults.yml` and the user's configuration. Signed-off-by: Joel Hawksley <joelhawksley@github.com>
joelhawksley
marked this pull request as ready for review
March 27, 2026 16:05
marcoroth
changed the title
marcoroth
added a commit
to marcoroth/reactionview
that referenced
this pull request
Aug 8, 2026
Follow up on #94. This pull request adds `config.external_template_mode` to control what happens to templates that come from gems rather than from the application itself. #### Motivation With `intercept_erb` enabled, ReActionView sees every `.html.erb` template Rails renders, including ones shipped inside gems. #94 stopped compiling those, which fixed #91, but it did so silently. If a gem's templates cannot be compiled by Herb, you never find out. That silence is the objection @joelhawksley raises in marcoroth/herb#1508, where a vendored copy of `primer/view_components` contains ERB that Herb cannot compile: So rather than a binary "skip or don't", the mode says how loudly to handle the failure: | Mode | Behavior | | --- | --- | | `:fallback` (default) | Compile with Herb. If that fails, log a warning and fall back to `ActionView::Template::Handlers::ERB`, so the template renders exactly as it would without ReActionView installed. | | `:skip` | Never compile templates that come from gems. This is what #94 shipped. | | `:compile` | No special treatment. Your `validation_mode` applies to them just as it does to your own templates, and nothing is rescued. | ```ruby ReActionView.configure do |config| config.external_template_mode = :fallback end ``` A gem template Herb cannot handle now renders as before, and says so: ``` [ReActionView] /app/vendor/bundle/ruby/3.4.0/gems/actionpack-8.1.2/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb could not be compiled by Herb, falling back to ActionView::Template::Handlers::ERB: InvalidNestingError: Block element <h2> cannot be nested inside <p> at line 9 ``` Related marcoroth/herb#1508 Related marcoroth/herb#1362 Related #91
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
1 participant
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.
By default, Herb currently ignores the
vendordirectly. This is potentially quite harmful, as it could expose consuming applications to runtime exceptions.For example. We vendor https://github.com/primer/view_components in our application. The dependency includes ViewComponents with ERB files, including a few that will not compile with Herb. If we don't compile the templates in our test environment, those Herb compilation errors will not be caught and would raise in production.
I see similar risks for other gems that provide UI, such as as dashboards mounted via an engine.
I'm wary of blankly excluding the directory silently by default, especially when we describe the behavior of
herb analyzeasRunning herb analyze without arguments now defaults to the current directory(https://herb-tools.dev/blog/whats-new-in-herb-v0-9#improved-herb-analyze-command). This could make one believe that all subdirectories are being analyzed based on the config in the user's.herb.ymlfile, but in fact it is the combination ofdefaults.ymland the user's configuration.It also appears that adding
'vendor/**/*.erb'toincludestill results in the directory being excluded. I had to explicitly callbundle exec herb analyze vendorto see errors from the vendor folder!Of course, this does nothing to avoid similar issues for gems that provide UI that are not vendored. I believe we should include a boot-time safe-guard for this issue in ReActionView at the very least. Or, we could look into ways of not using Herb to compile ERB from gems, but that sounds like a mess to me.