Provide for a drillable variable structure in Twig templates

Just one idea:

Background: render elements can already have #pre_render callbacks. These are called by drupal_render() prior to it calling theme(), whereas preprocess functions (and likely their hook equivalents proposed in #2004872: [meta] Theme system architecture changes) are only called from within theme().

Idea: we make it so that drilling within Twig results in #pre_render being called on each drilled element. And, wherever we currently have child render elements being made/altered within preprocess functions, we move that into #pre_render callbacks, reserving preprocess functions (and everything being proposed in #2004872: [meta] Theme system architecture changes) to only the code that's needed for rendering the element by its own template.

Example from the issue summary: because the .src of an image is wanted during drilling from a parent element's template, we should not be setting it in theme_image() or template_preprocess_image(), but instead, move that to a #pre_render of an image render array.

Potential problem with this idea: is it clear what's needed during drilling vs. what isn't? If not, then potentially everything done in preprocess would need to move to #pre_render, and if that's the case, we should rethink whether it even makes sense to keep those as separate concepts.

+1 to #1. It's pretty much exactly the summary of some discussions I've been having in IRC with a few other people as it certainly looks like the path of least resistance for now (can avoid full OO or other sweeping API changes).

drupal_render() should be responsible for making renderable arrays drillable simply because not all renderable arrays are passed to theme() but if/when #2006152: [meta] Don't call theme() directly anywhere outside drupal_render() is cleaned up all renderable arrays will be passed to drupal_render().

I'd also like to bring up (and I'll make an issue for this) that as great as #pre_render (or equivalent is) for helping drillability is how great the current #post_render is at preventing drillability so it needs to be removed.

I'd also like to provide a quick high level summary of the minimum prerequisites for "Twig drillability" (as I understand it) so that hopefully more people can get involved in the discussion.

Basically, if you have user input ($input) and you want to get a string back ($output) there's at least two things that need to happen:

1. Modules need the ability to modify $input -> sanitising bits of it, altering the content, etc... to create a mid-way point between $input and $output ($variables)
2. The modified input needs to be arranged in some way to create a string with the right structure for HTML -> theme functions/templates

If Twig is to "drill" something then it needs to perform step 1 on some part of some structured data and end up with the $variables (the processed structured data) and not to also end up with the result of step 2 as Twig will want to do it's own conversion of what it receives into a string as part of processing the template.

The problem, as @effulgentsia pointed out is that we don't currently have just one phase for processing, we have like 3 or 4 and one even comes after the input is a string (as I said in my previous comment) so we have to start thinking about building new structures -> #2004872: [meta] Theme system architecture changes and/or start redefining "best practice" for our existing frameworks.

#2009132: Remove #pre_render and #post_render from drupal_render() as it prevents proper Twig drillability

#5 - That sounds cool too. I guess we need to start rolling a few proof-of-concept patches soon to move the discussion along.

Related #1818266: [meta] A secure theme system (with twig)

Updated issue summary.

In our Hangout last week we discussed an option basically along the lines of c4rl's #4, where:

a. Given child render elements in a parent template which have not yet been prepared,
b. When trying to access variables of those children which don't exist because _preprocess+ has not yet run,
c. Twig, in TwigTemplate::getAttribute(), should run that item through it's "preparation" stage(s),
d. Making it's variables available to the parent template.

Regardless of implementation (I will let Fabianx chime in on this), the attached patch includes a test for the use case we have been describing.

+++ b/core/themes/test_drillable/node.html.twig
@@ -0,0 +1,134 @@
+    {#
+      Test that we can drill into variables of child elements.
+    #}
+    {% if content.field_image.0 is not empty %}
+
+      {#{{ dump(content.field_image.0) }}#}
+      {#{{ hide(content.field_image.0) }}#}
+
+      {#
+        When we access attributes from a field instance below, Twig should:
+          1. Realize the requested attributes do not exist.
+          2. Realize the field instance is still a render array.
+          3. Pre-render the field instance, returning the prepared variables.
+        When we print {{ content }}, field instances should be rendered with
+          render() as usual.
+      #}
+
+      <img class="layout-first-image" src="{{ content.field_image.0.attributes.src }}" />

We may likely end up needing to drill through the field instances themselves, which include wrapper divs, etc.

Implementation may need to be something more like:

content.field_image.0.item.attributes.src

...where this variable preparation magic happens twice--once on the field and a second time on the field item.

Cleanup and added one more test for #type link child elements.

This is really really important, but not utterly critical. Also, per https://groups.drupal.org/node/298298 Twig isn't supposed to have critical follow-ups.
Demoting per discussion in IRC.

changed Remaining tasks to write tests after use-cases are decided.

For reference: The IRC chat between Fabianx and I on 2013/06/18:

...
Fabianx: steveoliver: So implementation is:
Fabianx: ( proof-of-concept)
Fabianx: * Add a new parameter to render and theme.
Fabianx: Call it $render = TRUE
Fabianx: If $render == FALSE, call theme(..., $render), but do not run the actual theme function but return the $variables array.
Fabianx: Add some code in the d_render() for the case that $render == FALSE and replace the internal structure of the processed element with $variables and save the original in #original.
Fabianx: Do not call post_process or render_children if $render == FALSE.
Fabianx: or drupal_wrapper
Fabianx: or #prefix or #suffix.
Fabianx: That is all ignored for drillability.
steveoliver: …sure
Fabianx: Then when you are able to get a structure from render()
Fabianx: Like:
steveoliver: i get that part..
steveoliver: so up to the determination point for $render = TRUE/FALSE...
steveoliver: you still see that happening on TwigTemplate::getAttribute()
steveoliver: ?
Fabianx: $vars = render( array( '#theme' => 'x', ...), FALSE);
steveoliver: but why not create a separate function, i..e dont_render()...
steveoliver: and leave render alone...
Fabianx: steveoliver: That would be rather slow and its really only needed for render arrays.
Fabianx: steveoliver: That is fine.
steveoliver: sure
Fabianx: steveoliver: Just for theme() it probably makes sense (for a POC) to not copy all code ...
steveoliver: i'm cool with that stuff...
Fabianx: steveoliver: For #types I would say they should define a "drillable_function" as part of their type definition.
Fabianx: For themes use the $variables.
steveoliver: *thinks on that....*
steveoliver: well wouldn't the drillable function just be preprocess?
Fabianx: steveoliver: ( would at least solve it for links for now )
Fabianx: steveoliver: yes
Fabianx: steveoliver: but _especially_ the types we want drillable.
Fabianx: link as the prime example.
Fabianx: steveoliver: So the thing to pass the tests then is to use the TwigReference::__get function.
Fabianx: because all render arrays (at least for printing) are passing through there.
Fabianx: So that is ideal to intercept there.
steveoliver: ok.  jumping back a second: re: *suggestions, *suggestions_alter, *prepare, *prepare_alter…, i doubt that will happen… what do you think?
steveoliver: i wondered about it because calling prepare and prepare_alter would seem the only things needed in our 'dont_render' function ...
Jennifer Lampton: steveoliver: suggestions + suggestions alter can happen pretty easily
Jennifer Lampton: prepare might have to remain preprocess for D8
steveoliver: jenlampton: cool
Jennifer Lampton: sadly 
Jennifer Lampton: I think we'd need a new meta that was "split preprocess into prepare and prepare alter" with a critical follow up to remove preprocess
Jennifer Lampton: we just need to determine how crucial that is for everything else we need to do
Jennifer Lampton: but since there's been no momentum there yet
Jennifer Lampton: *shrugs*
steveoliver: yeah, but the bug needs fixed
steveoliver: Le Bug
steveoliver: specific preprocess functions for theme_suggestions are not called...

Since then..

1. I implemented TwigReference::__get(), but it was never called for the unprepared properties of the first link (test) at /twig-theme-test/drillable-elements.
2. I looked in TwigTemplate::getContextReference() for an opportunity to check for unprepared properties, but came to the conclusion this probably is not the place.
3. So far, I've been testing with this dummy return array, just trying to find and confirm the proper trigger point:

  public function __get($name) {
    return array(
      'title' => 'Title for ' . $name,
      'attributes' => array(
        'href' => 'HREF',
        'class' => 'foo bar'
      )
    );
  }

c4rl, Fabianx, others: If you could have a look around, you might see something I'm missing. I'll keep digging around in the meantime.

nm, got it. __get() needed __isset() implemented.

1. Via __isset(), how do we determine which unavailable properties we'll 'prepare' for?
2. Via __get(), how do we 'prepare' and return the variables?

I see no way to hack drillability out of drupal_render() or theme(). I think we should introduce drillability via new '#drill_callback' properties in hook_theme() and hook_element_info().

The api-implementation.patch provides drillability into the image field(s) of /core/themes/test_drillable/node.html.twig.

The tests are now broken, I suspect because $node->get('field_image', 0)->entity->uri does not return a string but an Object. Something in the Entity/Field system must have changed recently, but I couldn't find a change notice that helped.

a few more notes:
- support theme hook suggestions/fallbacks in drupal_drill_element_callback()
- maybe put this logic in a theme_hook_is_implemented()
- we may prefer theme registry as a service instead of these theme_get_registry() calls

I have a couple of comments on #15:

- Theme suggestions will fail to be discovered in drupal_drill_element_callback() and array syntax theme suggestions will probably even throw errors.
- I think it is important to support lists of multiple #drill_callback functions rather than only allow a single callback per element - even the current #pre_render takes an array of functions to run
- #type and #theme are very commonly set at the same time for render arrays, so if element_info() and hook_theme() both return a #drill_callback we shouldn't ignore one of them. Rather than elseif, maybe we should merge #type and #theme callbacks into a single list of callbacks to run (see the point above about supporting this).

Yep, since all it is, or should be, is implementations of preprocess, I think we can just run those preprocess phases for elements that implement them; If preprocess is not implemented, variables are not available.

The attached patch adds a third parameter to theme() that Twig, via drupal_render_prepare(), tells theme() to return the prepared $variables array rather than $output (HTML).

It works, and is about as simple as it gets.

Implementation patch to follow.

OK, fine... A patch that applies.

Fabianx: steveoliver: nice
Fabianx: steveoliver: the rendered output should be cached "somewhere".
Fabianx: steveoliver: in general the best would probably be to:
Fabianx: a) replace the contents of "this" ( using exchangeArray ) with the new variables and save the original via ->getReference() in #original.
Fabianx: IIRC, #theme should still be set, so then adding a key like: #variables_prepared = TRUE, #theme_hook = 'x' (all data prepared by theme) and then having a check in theme() if things have been already prepared is all that is left here.

+  if (isset($element['#type']) && !isset($element['#theme'])) {
+    $element += element_info($element['#type']);
+  }

I don't understand why we aren't loading the defaults for type if #theme is set. This is new behaviour that I haven't seen elsewhere.

I don't immediately see how this patch handles drillability for elements that follow a #type -> #pre_render -> #markup approach that do important sanitisation work in #pre_render and never hit theme()?

Also, the patch in #21 doesn't seem to deal with #theme_wrappers arrays either?

I'd also love to see some more discussion over at #2009132: Remove #pre_render and #post_render from drupal_render() as it prevents proper Twig drillability and I thought I'd link to #2025259: drupal_render() should preserve the original render element in #original so pre and post render functions can access "raw" input as it would be useful for people actually trying to write #drill_callback functions :)

ah, also, I thought about this a bit more... Does this work with the 'render element' property from hook_theme()?

Related, #2044105: Introduce #alters for \Drupal\Core\Render\Renderer::render() would make it easier to drill elements rendered outside #theme.

I created #2047263: Provide inline_template tag within Twig that inlines a template for usage with a render array, which will take care of the following use-case:

* Drillabillity with support for #post_render, #theme_wrappers and #cache.
* Essentially inline templates.
* It is actionable now and not even that complicated.

The use case for this issue then remains to "just" access the variables and print them out.

The same approach from https://drupal.org/node/2047263 (using #render_function) can be used here to provide a nice twig function:

{% set render_vars = get_render_vars(render_array) %}

Once #2047263: Provide inline_template tag within Twig that inlines a template for usage with a render array is in this is no longer even complicated, though it is not yet automatic drillability, it is another step forward and could be theoretically even called from phptemplate templates.

Possibly related #2009132: Remove #pre_render and #post_render from drupal_render() as it prevents proper Twig drillability

minor grammar

We could maybe do some things here during 8.x.x if we can do it in a way that maintains backwards compatibility, but not for 8.0.x so just bumping for now.

Unassigning; not sure the status of the feasability of this and not working on it at the moment.

as parent closed