#theme_wrappers should be able to have a unique set of variables per wrapper hook

I was going to have a look to see if something could be done inside drupal_render() so that we don't have to do much work as theme() wouldn't know the difference.

  if (isset($elements['#theme_wrappers']) && !isset($elements['#render_children'])) {
    foreach ($elements['#theme_wrappers'] as $theme_wrapper) {
      $elements['#children'] = theme($theme_wrapper, $elements);
    }
  }

could be like:

  if (isset($elements['#theme_wrappers']) && !isset($elements['#render_children'])) {
    foreach ($elements['#theme_wrappers'] as $theme_wrapper) {
      $overrides = array();
      foreach ($elements as $key => $value) {
        if($key[0] === '#' && isset($elements['#' . $theme_wrapper . '_' . substr($key, 1)])) {
          $overrides[$key] = $elements['#' . $theme_wrapper . '_' . substr($key, 1)];
        }
        else {
          $overrides[$key] = $value;
        }
      }
      $elements['#children'] = theme($theme_wrapper, $overrides);
    }
  }

because theme() doesn't modify anything in $elements by reference, there's probably no harm in sending it a fake array.

it seems like Core doesn't need this to change,

Have a look at the awkward patches being posted/committed under #1595614: [meta] Remove all the theme functions and templates in core that simply output a link. Replace with #type 'link' render arrays that either nest #theme inside #theme_wrappers elements or even early-render elements directly into #children!

Also, as I mentioned in #2031301: Replace theme_more_link() and replace with #type 'more_link', the current setup precludes defining #type 'more_link' (or similar) to setup default render elements for commonly re-used link markup throughout core.

But all of the existing twig templates for the theme wrappers would have to be updated to use the new variables too, right?

No, what I put up in #2 means that the theme functions would see exactly what they would have seen before.

For example if #2 was implemented, if you had

array(
  '#theme' => 'image',
  '#theme_wrappers' => array('container'),
  '#container_attributes' => array('class' => 'baz'),
  '#attributes' => array('id' => 'foo'),
  '#uri' => 'bar',
);

Then theme('image') would "see" these variables:

array(
  'attributes' => array('id' => 'foo'),
  'uri' => 'bar',
);

and theme('container') would "see" these variables:

array(
  'attributes' => array('class' => 'baz'),
);

Because #container_attributes would be temporarily copied over the top of #attributes before theme('container') is called from within drupal_render(). This would happen without needing to define 'container_attributes' in hook_theme() for 'container' because it is detected dynamically based on a naming convention.

AFAICS this wouldn't impact the operation of anything past the point theme() is called, unless I'm missing something obvious.

Something like this.

that would be 'form' overriding 'id' when 'form_id' is set >.<

The most obvious solution would just be to change the proposed convention to something that is more obscure.

patch just updates the convention to something safer:

array(
  '#theme' => 'image',
  '#theme_wrappers' => array('container'),
  '#wrapper_container_attributes' => array('class' => 'baz'),
  '#attributes' => array('id' => 'foo'),
  '#uri' => 'bar',
);

Would be the new format.

#10, yeah, multiple theme wrappers per element are a thing.

At first I just wanted to kill theme wrappers too, but then I realised that if we keep them we can do cute things like this inside hook_element_info():

$types['more_link'] = $types['link'];
$types['more_link'] += array(
  '#title' => t('More'),
  '#attributes' => array(
    '#title' => t('Show more content'),
  ),
  '#theme_wrappers' => array('container'),
  '#wrapper_container_attributes' => array(
    'class' => array('more-link'),
  )
);

The problem is that #theme_wrappers are useful for setting up default structures in #type that move beyond a single #theme or #pre_render callback. You can't put nested children in a #type default because there's no standard way to name child elements.

I didn't think of the use-case in #12. nice work :)

Could we get an extra test in with that example?

Updated test as per #13

i'll have a look at the docs. Don't forget to set to needs work if we need doc updates :)

here we go, added docs.

If #2044105: Introduce #alters for \Drupal\Core\Render\Renderer::render() happens then I'm sure a lot of the single tag wrappers could be turned into #render callbacks (as #render is modelled off #theme_wrappers anyway) which would make them simpler and faster.

It's a little dangerous to be thinking of #theme_wrappers 'container' as the only thing we have to be considering in the discussion about "wrappers". For example, from memory I believe the overlay module uses #theme_wrappers array('overlay', 'html') to wrap #theme 'page', which is quite a bit more complex than a single h2 or div. I don't know to what extent we'd have to re-write the overlay module to convert this pattern into nested #theme hooks rather than wrappers, it's something we'd have to investigate.

For now, just assume that simply swapping #theme_wrappers out for a bunch of "one tag" 'container' theme functions is not possible and the full conversion of #theme_wrappers in core to #theme would be "non-trivial".

Correct me if I'm wrong, but given where we are in the D8 release cycle I think we actually aren't allowed to remove #theme_wrappers until D9 at the earliest, as that would represent massive API breakage.

We should decide a desired course of action or we'll lose our window to make this immediately actionable (if there even still is one :P):

A. Do nothing now, push this issue to D9. We have to be content with early-rendering arrays into #children where wrappers and hooks conflict.
B. Commit the extended form of #theme_wrappers in #17 and forge forward with this idea, exploring it as a new paradigm for avoiding early-render. We'd also want to then incorporate this pattern into #2044105: Introduce #alters for \Drupal\Core\Render\Renderer::render()
C. Commit the extended form of #theme_wrappers in #17 as a stop-gap for D8, allowing us to clean out early-render but file a followup D9 ticket to remove #theme_wrappers. We'd then want to disallow multiple #render callbacks (it's currently an array) and allow only one #render per-element like how #theme works #2044105: Introduce #alters for \Drupal\Core\Render\Renderer::render(), otherwise we'll just repeat all the same mistakes with render callbacks in the future.
D. ???

I don't have a strong preference for B or C but I'm not a huge fan of A because I find early-rendering anything into #children just to support trivially simple configurations like a link in a div kind of depressing >.<

The problem with the drillability discussions in the issue queue so far (that I've seen) is that they are very "early days", high level, very theme()-centric and focus on what we want but not so much on actionable issues that can be followed up or referenced in discussions like this one. There's no instructions or consensus as to what needs to be preserved/exposed/removed/extended at #2008450: Provide for a drillable variable structure in Twig templates to "allow drillability" in any of #cache, #printed, #access, #pre_render, #theme_wrappers, #markup, #post_render.

AFAIK, all we have so far to guide our progress towards drillability is:

- There's a single "proof of concept" patch that attempts to handle #theme correctly but the consequences of defining any of the other standard attributes in drupal_render() on an element are ignored.
- We have a broad consensus that to make something drillable we have to split rendering of elements and *parts* of elements into at least two phases "prepare" and "make string", with the implication that all the steps to "prepare" can be identified reliably and accessed publically and "make string" can either be inferred once the variable is prepared or explicit instructions as to how an element should be "made into a string" can be found on the element somewhere.

This says nothing about #theme_wrappers. It's not even clear that themers "drilling" *want* theme wrappers to be rendered around the "parts" of the element that they're drilling into. If we think back to Jacine's blog post in 2012 about 'theme components' referenced in #1804488: [meta] Introduce a Theme Component Library, one of the key concepts outlined there is that with a two-tier structure for a theme system (she calls it "container" and "item", but it's roughly translatable to "#theme" and "#theme_wrappers" IIRC) at least one serious themer specifically doesn't/didn't want the wrapper to be rendered for elements when being drilled (items), only when the element is being rendered in its entirety.

I did actually bring up the issue of #theme_wrappers with the current proposals for drillability at https://drupal.org/node/2008450#comment-7602499 but haven't heard anything back yet :( It's very unclear to me what the intention is for #theme_wrappers and drilling and I think any further discussion on the topic should live in that issue.

Sooo, in the interests of not de-railing this issue and keeping drillability discussion somewhere with high visibility, can we agree that this issue can propose removing #theme_wrappers only if we can't resolve the issue of conflicting variable names with other #theme_wrappers hooks or #theme hooks and #2008450: Provide for a drillable variable structure in Twig templates can propose remove #theme_wrappers only if they're deemed fundamentally outside what can be supported at the same time as Twig drillability (we can also decide if that would actually be a good enough reason to remove #theme_wrappers over there).

Here's a patch that implements the alternate syntax in #20 sans-#inherit, which I personally believe is overkill at this point and is really blurring the lines between #theme, #theme_wrappers and child elements (not in a good way).

One bonus of the syntax in #20 is that it is compatible with multiple instances of the same #theme_wrappers hook, eg:

array(
  '#attributes' => array('class' => 'foo'),
  '#theme_wrappers' => array(
    'container' => array(
      '#attributes' => array('class' => 'bar'),
    ),
  'container',
  ),
);

I added a test for that, although I don't know how much mileage this use-case will get in the wild it is still a nice clean-up.

I also fleshed out the docs a bit as the new syntax is more confusing even if it is more concise.

Updated issue summary.

updated issue summary to match latest patch

/sigh. I don't know why I always seem to get the docs wrong somehow :/

#26: 2042285-26.patch queued for re-testing.

#31: drupal-theme-hooks-used-2042285-31.patch queued for re-testing.

looks good to me :) although, maybe we should mention the new syntax is optional and you can just use strings as values for #theme_wrappers if you don't need the extra attributes flexibility.

I actually wrote a blog post on this as well, but it didn't come through Planet Drupal for some reason :(

opened #2068217: array('hook') syntax for suggestions is broken for #theme_wrappers