Get rid of the <% yield p = np %> requirement.

[andrewculver]

Quoting some feedback I received privately:

I wonder if there's a way to do without <% yield p = np %>. It all feels Railsy, except for that.

[andrewculver]

I haven't looked at the related Rails internals at all, but perhaps there's some way of implementing something there? I'm not sure whether there would be performance implications if you invoked Nice Partials in every partial render indiscriminately. But you also wouldn't want to yield indiscriminately. I'm open to any and all suggestions on this one, I don't love <% yield p = np %>, but it seems required at the moment.

[andrewculver]

(But come on... it's funny. /cc @peterkeen)

[andrewculver]

On the other hand, I also don't hate it. It serves a function and in conjunction with the explanation provided in the README, I feel like it actually demystifies what is going on. It makes really clear the idea of the "shared context object" or whatever we want to call it.

[domchristie]

Part of me thinks that if there's more magic, it could move away from being "just partials", but then perhaps I don't know enough about the internals to see what's possible?

<% yield p = np %>is mostly Rails code, utilising render blocks, so it can also be used to capture the block content, like so:

<%# index.html.erb %>
<%= render 'path/to/partial' do |p| %>
  <%= p.content_for :heading, 'Hello world' %>
  Lorem ipsum
  <%= p.content_for :footnotes, 'Foo bar' %>
<% end %>

<%# _partial.html.erb %>
<% content = yield(p = np) %>
<h1><%= p.content_for :heading %></h1>
<%= content %>
<footer><%= p.content_for :footnotes %></footer>

[domchristie]

I suppose it doesn't matter where you call np, just as long as the partial calls yield p before any call to p.yield or p.content_for. So you could pass it in:

<%# index.html.erb %>
<%= render 'path/to/partial', p: np do |p| %>
  <% p.content_for :heading, 'Hello world' %>
<% end %>

<%# _partial.html.erb %>
<% yield p %>
<h1><%= p.content_for :heading %></h1>

This might be more Rails-y 🤷‍♂️ Developers are familiar with defining locals, so it'd be reasonably understood to look for them there.

Note: I don't think this would work when rendering collections because:

1. I've just discovered that Rails doesn't call the block when passing in a collection (and so Nice Partials won't work in those cases)
2. Even if it did call the block, we'd need to ensure that each partial was given it's own NicePartial::Partial instance

[andrewculver]

@domchristie From my perspective, I'd like to try and avoid the invoker of the partial from having to repeat any non-unique part of the set up whenever they use a partial, so I think p = np belongs in the partial.

[andrewculver]

I think I figured out conceptually where a right place to inject this all "into Rails" could be: Just override render in a view helper. That method could yield to the passed-in block with "np" as a parameter and also assign "p = np" into the locals for the partial that would be rendered by super. The problem is:

1. You don't necessarily want this happening in every partial for performance reasons. (?)
2. It breaks the normal Rails behavior of passing a block into a partial.

This is getting into hacky territory, but one potential solution to those two problems would be to inspect the target partial and try to detect whether it's using Nice Partials. At first blush, this feels sort of wrong, error prone, and exactly the kind of magic people don't like. Not sure how to proceed.

[domchristie]

I'd like to try and avoid the invoker of the partial from having to repeat any non-unique part of the set up whenever they use a partial

Ah, of course!

Just override render in a view helper. That method could yield to the passed-in block with "np" as a parameter and also assign "p = np" into the locals for the partial that would be rendered by super

Oh, interesting. It looks like _layout_for is designed to be overridden, and could provide a path for this:

  def _layout_for(*args, &block)
    name = args.first

    if block && !name.is_a?(Symbol)
      p = NicePartials::Partial.new(self)
      capture(*args, &-> { block.call(p) })
    else
      super
    end
  end

Just need to work out:

• an improved condition of when to call it*, avoiding unnecessary calls, and to preventing conflicts when people call <%= yield my_own_object %>
• how to inject the NicePartials::Partial instance into the partial

I don't think the current approach is that bad and I like that it's transparent, but it's interesting to explore :)

---

* some ideas: some kind of annotation within the partial or denoted by the file name? or perhaps nice partials should live in a specific directory? I'm not sure.

[domchristie]

Also, another benefit of an explicit <% yield p = np %> call is that developers can rename p to whatever they like. With that in mind, here's an alternative API, inspired by the form_for method:

<%= np do |p| %>
  <% yield p %>
  <%= p.content_for :heading %>
<% end %>

  def np(&block)
    NicePartials::Partial.new(self).yield_self do |p|
      capture(p, &block)
    end
  end

Personally I still prefer the original one-liner, but it's something!

[andrewculver]

Picking this thread back up, one idea I've had in the back of my mind is that we could do _file.nice.html.erb vs _file.html.erb and that would automatically do the yield p = np call before rendering the view and make p available in the view. The idea of putting nice before html.erb is that .html.erb would still be recognized by IDEs, etc. and do the normal syntax highlighting, etc.

[pascallaliberte]

nice

[kaspth]

I haven't looked into how viable this is yet but Erubi, Rails' erb backend, supports a preamble that you theoretically could use to insert the yield p = np code into. https://github.com/jeremyevans/erubi/blob/595dfaff86e3dd9dff09cbdfc8bc043bfcd64c6f/lib/erubi.rb#L85

[andrewculver]

@kaspth Amazing work on this one! Thank you!