[Feature Proposal] allow procs / lambdas in `locals` for `render` calls on collections to support dependent variables

rubyonrails-core
,
1

I’m proposing to introduce support for something like this:

render partial: "objects/object", collection: @objects,
  locals: { dependent_variable: ->(o) { some_fun(o) } }

or maybe this would be better?

render partial: "objects/object", collection: @objects, locals: { x: :foo},
  dependent_locals: { dependent_variable: ->(o) { some_fun(o) } }

and so in your partial you can call dependent_variable as if you passed it in directly. The added benefit here is that the variable can work as either being directly passed in for a single render call, or as a dependent variable in a collection render call, and your partial is unaware of the difference. i.e.

# doesn't use a proc, but the template doesn't have to know!
render partial: "objects/object", object: object, locals: { dependent_variable: some_fun(object) }

  • “why not define the variable in the template itself?”

    • this doesn’t seem very appealing to me. In fact, haml lint throws a fit over it (not sure about ERB linters)

  • “why not just use helper methods”

    • for my current use case, I need to reference the value I’d like to assign to the local variable several times throughout the template, so it becomes quite verbose

The implementation for the first approach only involves a couple of lines of code changes, not including tests (I’m currently running it locally). I have not looked into the second approach yet but it seems like it would be more involved.

What do folks think about this? Would it be useful?

2

The more that I think about this the more I feel inclined to go with the second approach. Under the hood, I would merge the resulting local assignments into locals (because in the template itself it shouldn’t care if they are local or local_dependent), but I don’t think we should have this behavior for procs in locals by default – someone might very well want to pass a proc or lambda to locals and so the first approach would screw this up.

Unless anyone has any objects, I think I’ll open a PR which adds dependent_locals as an option to collection render calls and follows the behavior I described in the original post.

3

Don’t think we want either version of this, sorry.

How about just extracting a local variable within the partial at the top:

<% variable = some_fun(object) %>

Works for individual renders as well as collection renders. And the solution feels clearer too.

2 Likes
4

hi there!

How about just extracting a local variable within the partial at the top:

<% variable = some_fun(object) %>

Works for individual renders as well as collection renders. And the solution feels clearer too.

I had mentioned that (aside from not being particularly appealing), some linters (haml-lint) in particular don’t seem to approve of this.

aside from that, it doesn’t seem like the most natural place to define a variable. I think it also opens the door to encouraging variable definitions all over template files (I’ve actually seen this done before and it was very messy), vs having a clear idea of what variables you expect to see in the template once you open it (after seeing a render call for it somewhere). Furthermore, it ties the partial directly to how the local variable is defined, and I’m not sure how attractive that is from a design standpoint.

Maybe the example that I gave wasn’t a very good one.

I think this sort of thing would be useful in this sort of situation where we have something like this:

Let’s say you have a model Record with fields name and contents, and some other properties. Suppose you have a view where you would like to display all the records, but also flag each “duplicate” record by prefixing its name with "(Duplicate of #{original.id}"). Suppose further that you have another view for displaying all records, but you don’t want this duplicate information included (i.e. maybe it’s client facing and you choose to only show the “original” records").

At present, you could do something like:

  • wrap this logic up into a wrapper class with something like an is_duplicate field and a reference to the original, and then check each object while rendering
  • have separate templates / partials
  • etc.

What I’m recommending is being able to do something like this:

# show with duplicates controller
def index
  # assume there isn't a large amount of records for simplicity
  # so I don't have to deal with paging for this example
  # assume further that the smallest ids for each group are the "originals"
  @grouped_records = Record.all.group_by(&:name).transform_values { |group| group.sort_by(&:id) }
end

# HAML
# in the view with duplicates
- @grouped_records.each do |_, record_group|
    # render the first record of each group, and render the duplicates if
    # there are any
    = render record_group[0] locals: { header: record_group[0].name }
    = render record_group[1..],
      dependent_locals: {
        # we can even extract the prefix addition out into a helper method
        header: ->(r) { "(Duplicate of #{record_group[0].id})" r.name) }
      }

# in the index view without the duplicates
# assume there is a controller action with `@records = Record.only_originals` or something
= render @records, dependent_locals: {
  # maybe on different views we even want to use a different header
  # aside from just the name. Maybe different prefixes. *shrug*
  header: ->(r) { r.name }
}

# finally, in the `show` template, maybe we use the same partial again
# assume we have a variable @record = Record.find(...) in the controller
= render "records/record", record: @record, locals: { header: @record.name }

So now we have a few different ways that we could use this, but the partial is none the wiser:

%h3= header
.contents
  = record.contents
# etc
...

Therefore we’ve increased the flexibility of how and where we can use our partial / its local variables, and define the expected local header variable that it uses, all with a neat / expressive syntax.

The best part of this is that the partial does not care how header is actually being assigned, which I think is extremely attractive from a design standpoint.

So in summary, I would argue in favor for a change like this because:

  • It increases the flexibility of any given partial that utilize local variables in an expressive manner
  • allows the partial to remain ignorant of the details of how its local variables are constructed
  • encourages less code to be written for special cases as a result of said flexibility
  • is a natural extension to the concept of collection rendering*
  • I would find it useful and I think others might too :slight_smile: (I would love to hear more from folks)

* what I mean by this is that we already have a locals concept, and when you’re rendering a single object you can easily do things like locals: { foo: object.bar }, or other sorts of things that are dependent (or not dependent) on the object itself. This currently offers flexibility for local variable definitions for for singular objects. It does not translate nicely when rendering collections, which is what I’m trying to remedy.