Proposal: partials should not create a local variable with the name of the partial when no :local or :object option was passed

rubyonrails-core
1

From this stack overflow question.

Suppose I have a partial _flash.html.erb for rendering the flash in a consistent way. The natural thing to do would be to assume I can call ActionView::Base#flash from within this partial to retrieve the flash, and render the partial withrender partial: "flash". But in fact, this doesn’t work. Even though I haven’t passed object: flash orlocals: {flash: flash} in the call to render partial, PartialRenderer oh-so-helpfully defines a flashlocal variable, whose value is nil, shadowing ActionView::Base#flash. My alternatives are to rename the partial or pass a value for the flash local explicitly using one of those more verbose call to render.

I suggest that if no object option or local with the same name as the partial was passed to the render call, thatPartialRenderer not define a local variable of the same name as the partial. This would change the behavior for anyone who is relying on the current behavior to get a nil value for the local when they don’t pass an explicit value, but that seems like an unlikely case and easily fixed by passing nil explicitly, and it would be much more useful to do things like call ActionView::Base#flash from within a _flash.html.erb partial.

I’m happy to create a pull request if others agree.

2

I’m -1 on ‘not creating local variable’, but will +1 if you’re going to make it check first if there’s a method with the same name defined or not.’

I can see that people will go outrage if we remove this feature …

Thanks!

  • Prem
3

I’m -1 on ‘not creating local variable’, but will +1 if you’re going to make it check first if there’s a method with the same name defined or not.’

Check first and then do what? Not create the local variable in that case?

I can see that people will go outrage if we remove this feature …

Are you aware of uses of relying on an implicitly nil local variable?

4

Do you know where the code is that does this? I'd like to understand why we do it before agreeing / disagreeing.

5

Right, I think I understand the problem incorrectly the first time.

If we do render 'flash', then there should be no local variable name flash that is nil. Does Rails currently create a nil variable name flash?

  • Prem
6

Yes, similarly if you do render @customer you get a variable called ‘customer’ when you render that template.

7

Around here, I guess https://github.com/rails/rails/blob/780ecb2b/actionpack/lib/action_view/renderer/partial_renderer.rb#L300

I remember I once actually hit the same problem and wrote a patch like this (but I forgot why I didn't finish the patch and send a PR). https://github.com/amatsuda/rails/commit/5b1c542ccb5a5aed6a2a9764c43523e103057336