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

[John Firebaugh]

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.

[Prem Sichanugrist]

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

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/sHiSiBDAdOkJ.
To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

[John Firebaugh]

On Tue, Sep 18, 2012 at 12:04 PM, Prem Sichanugrist <sika...@gmail.com> wrote:

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?

[Aaron Patterson]

On Tue, Sep 18, 2012 at 12:01:59PM -0700, John Firebaugh wrote:
>
>
> From this stack overflow question<http://stackoverflow.com/questions/3111844/flash-messages-in-partials-rails-3/12482879>
> .

>
> 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.

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

--
Aaron Patterson
http://tenderlovemaking.com/

[Prem Sichanugrist]

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

[Michael Koziarski]

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

-- 

Cheers,

Koz

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.

[Akira Matsuda]

On Mon, Sep 24, 2012 at 5:39 AM, Aaron Patterson
<tende...@ruby-lang.org> wrote:
> Do you know where the code is that does this? I'd like to understand
> why we do it before agreeing / disagreeing.

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

--
Akira Matsuda<ron...@dio.jp>