Don't compile external templates

[jherdman]

User's should not be beholden to template errors outside of their application. When we detect a template not local to the project we use the fallback ERB implementation.

Resolves #91

[jherdman]

@marcoroth im seeing some test fails here, but I think they're on main. Anything I can do to help?

[marcoroth]

@jherdman thanks! I think you might have to run bundle exec appraisal bundle install since you added mocha

[marcoroth]

If people opt-in to use Herb (or have a .herb file) we should compile everything using Herb. So I think we want to move this condition to lib/reactionview/template/handlers/erb.rb:10:

reactionview/lib/reactionview/template/handlers/erb.rb

Line 10 in 5b7269c

if template.format == :html && ReActionView.config.intercept_erb

[jherdman]

OK, I think I understand your motivation, but I want to double check. Am I correct to understand that ReActionView::Template::Handlers::Herb is never used unless we intercept ERb files?

[marcoroth]

Correct yes, see here:

reactionview/lib/reactionview/railtie.rb

Lines 28 to 34 in 416995f

	ActionView::Template.register_template_handler :herb, ReActionView::Template::Handlers::Herb
	end
	end
	config.after_initialize do
	ActiveSupport.on_load(:action_view) do
	ActionView::Template.register_template_handler :erb, ReActionView::Template::Handlers::ERB if ReActionView.config.intercept_erb

So if you have .html.herb it uses ReActionView::Template::Handlers::Herb directly, and otherwise if you have intercept_erb = true it goes through ReActionView::Template::Handlers::ERB

[marcoroth]

Ideally, we would still try to compile everything through Herb::Engine and only fall back (and warn) if there are some parse errors. But we don't need to do this in this PR.

[jherdman]

Do you reckon the cost of a rescue, log, retry through the fallback, is acceptable? Implementation doesn't seem that difficult imho.

[marcoroth]

Since this is a compile-time concern I'm not too worried about the performance cost, that said, we could also just do it in development/test and not in production.

[jherdman]

This test pattern follows your others, but, much to my surprise, something in this setup doesn't raise an error when I encounter an erroneous template. This frustrated my abilities to implement logging.

We know real world usage raises per the original issue. Is there something askew with the test setup?

[jherdman]

Short of exposing this as a class method on the Herb handler, it felt like this duplication was either warranted, so we should consider a TemplateUtilities module that we can mix in. Thoughts?