Significant change should be better explained in the Changelog regarding non ERB view partials

Here is the relevant section in the changelog, I think:

https://github.com/rails/rails/blob/v5.0.0.beta4/actionview/CHANGELOG.md

"Change the default template handler from ERB to Raw.

Files without a template handler in their extension will be rendered using the raw handler instead of ERB.

Rafael Mendonça França"

I always give it a glance to the changelogs before trying to upgrade but this documented change didn't grab my attention enough to think that it might impact on my application.

There are a few performance/analytics/monitoring related scripts that are loaded directly in an inline script in one of my view layouts.

They are loaded like this:

<script type="text/javascript">
   <% unless params[:monitoring] || Rails.env.development? -%>
     <%= render '/common/pingdomjs.js' %>
   <% end -%>
   ...

After this change I had to move it to:

     <%= render('/common/pingdomjs.js').html_safe %>

I think this is not an uncommon pattern and that maybe the need of such changes to non ERB files rendering should be highlighted in the Changelog to grab the reader's attention.

This is just a sugestion to improve the changelog before Rails 5 final is released in order to make the upgrade process smoother.

Best,
Rodrigo.

That makes sense. Thank you for pointing out. Do you have a suggestion how to improve it? Could you open a pull request?

Hi Rafael, thanks for your response. My wife went through a surgery on Friday and is under recovery for a few days and I'm alone with the babies during the weekend. I'll try to find some time to create such PR tomorrow.

But, before I do that, I'd like to ask: is this incompatible change really intentional?

I mean, I can see why Raw should be the default over ERB, but shouldn't the resulted string be flagged as html safe by default?

In other words, could you describe a real scenario where applying html escaping over a raw partial would make sense?

I'm just curious. If you decide this change indeed worths the breakage I can try to submit a PR with a better warning complementing the current explanation.

Best,
Rodrigo.

Hi, no worries. I can think in a better wording open a PR and ping you on it.

Yes, the change is intentional and it is not html safe by default by definition. The whole idea of this change is to avoid people to rendering unsafe responses as safe by mistake.

See https://github.com/rails/rails/issues/2394 and https://github.com/rails/rails/pull/6309 for background.

I understand the reasons why it’s usually not desired for strings to be html safe by default but I don’t see how useful this change is when applied to partials rendering, that’s why I asked for an example. I couldn’t find any real scenario where it would be desired when looking at the issues you mentioned.

I understand why we don’t want every file to be interpreted by ERB by default and why RAW is a better fit, I just fail to see how not being html safe would be useful for partial rendering…

The babysitter should arrive in a few minutes and I’ll try to open a PR to change the changelog anyway…

Or maybe we should automatically flag .js and .htm(l) extensions as html safe after using the raw handler, as they are common extensions expected to be used as partials…