Refactor the render context stack to avoid a static property

Problem/Motivation

The RenderContext stack relies on a static property, which essentially functions as a global to hoover up cacheablemetadata when functions return strings. If possible we should try to properly isolate render context at each recursion level of a render array.

Steps to reproduce

Proposed resolution

Copy and pasted from @fabianx's issue at ✨ [Braindump] Fabianx' Drupal 8 performance master plan Postponed: needs info .

RenderContext braindump:

08:22 Or TL;DR:
08:23 - Return out-of-band + render_array always
08:23 - If you need to return a string, merge with whatever is the current rendering context on the stack
08:23 merge into
08:23 so its preserved.
08:24 - If there is no rendering context => Exception
08:24 === done
08:25 If you need whatever comes below isolated from the rest, start a new render context via closure.
08:25 => early rendering possible and again encouraged :p
08:25 => drupal_add_js possible again :p

Problem/Motivation

Drupal 6 used drupal_add_js / drupal_add_css exclusively, that made caching difficult and there was one global that tracked it all.

theme() was rendered directly to a string and that was all there is.

=> One global state, tracked per page request

In Drupal 7 there was both drupal_add_js() / drupal_add_css() to track JS and CSS and #attached for js / css, but #attached was very much preferred.

=> State per render array, so it was cacheable and global state per page -> still treated as a big page

In Drupal 8 there is only #attached and global state was mostly removed, however because of twig rendering of render arrays also needs to preserve state and hence we introduced a stack.

=> State per render array, state per render recursion 'call stack'.

This stack is constructed on demand when its not yet there and that is a problem as it then for edge cases acts like an implicit global again.

The stack being merged back into the render arrays makes it also way simpler to rely on just #attached for render caching.

Proposed resolution

All of those have in common that there needs to be some way to track the state of the system, in D6 one global, in D7 globals and pass-around, in D8 Stack and pass-around.

The optimal solution would be to have drupal_render() return a Renderable of some kind: call it HtmlFragment, render array with #markup and #attached, value array [$markup, $attached], value object, etc.

This had never been feasible due to the BC break of drupal_render() no longer returning a rendered string.

However must it be changed?

No, the reason why a stack based approach works is that we only need to merge the render array at distinctive points:

a) when we have #cache 'keys' or #render_placeholder set in the render array
b) when we return from a Renderer::render() call (and hence not call recursively our own via Renderer::doRender())

The stack and all other things so far have been implicit. The idea is to now make it explicit and then opt-in several things to have a default 'Rendering context'.

- Whenever something is rendered, it needs to be rendered onto a rendering context.
- When something is recursively rendered (a new render() call while being in another render() call, it is rendered into a sub-context (new level) of the current rendering context and then merged back once we return from that render() call.

The functions would look like this in pseudo-code:

public function renderOnNewRenderContext($callable) {
  $old_render_context = $this->renderingContext;
  $this->renderingContext = new RenderContext();
  $this->renderingContext->pushFrame(new BubbleableMetadata());
  $markup = call_user_func($callable);
  $frame = $this->renderingContext->popFrame();
  $this->renderingContext = $old_render_context;

  $elements = [];
  $elements['#markup'] = $markup;
  $frame->applyTo($elements);

  return $elements;
}

public function render($elements) {
  if (!isset($this->renderingContext)) {
     throw new Exception("Need something to render on."); 
  }

  $markup = $this->doRender($elements);
  $frame = $renderingContext->popFrame();
  $frame->mergeWith($elements);
  $this->renderingContext->pushFrame($frame);

  return $markup;
}

protected function doRender() {
  if (!empty($elements['#cache']['keys'])) {
      $this->renderingContext->pushFrame(new BubbleableMetadata());
  }

  // render everything, etc. ...

  if (!empty($elements['#cache']['keys'])) {
    $frame = $this->renderingContext->popFrame();
    $frame->mergeWith($elements);
    $frame->applyTo($elements);
  }

}

Remaining tasks

User interface changes

API changes

=======

More braindump:

- hook_entity_view() uses drupal_add_js(), but needs to end-up within the same render array level => #pre_render pattern solves that, else Exception.

====

Of note is that unless something wants to #cache or placeholder something in the tree, it does not matter if in the end it all ends up merged within the same 'global'.

Even in D6 / D7 one global works as long as there is no other cache than the page cache!

Hence only #cache / #render_placeholder needs to start a new level => reduces complexity.

====

Even for a new context a new level would be sufficient.

Every level means completely isolated rendering and it is removed from the stack completely in the end.

=> Do we even need a stack, is the call stack not enough? => Likely problems with Exceptions, but then we destroy the stack, batch API, we return from the call stack ...

Lets see if this would work:

public function renderOnNewRenderContext($callable) {
  $old_render_context = $this->renderingContext;
  $this->renderingContext = new BubbleableMetadata();
  $markup = call_user_func($callable);
  $frame = $this->renderingContext;
  $this->renderingContext = $old_render_context;

  $elements = [];
  $elements['#markup'] = $markup;
  $frame->applyTo($elements);

  return $elements;
}

public function render(&$elements, $is_root_call = FALSE) {
  if (!isset($this->renderingContext)) {
     throw new Exception("Need something to render on."); 
  }

  $markup = $this->doRender($elements, $is_root_call);
  $this->renderingContext->mergeWith($elements);

  return $markup;
}

protected function doRender(&$elements, $is_root_call = FALSE) {

  // Render cache items within a new render context.
  // @todo move down to render_children instead.

  if (!empty($elements['#cache']['keys']) && !$is_root_call) {
     $renderer = $this;
     $elements = $this->renderOnNewRenderContext(function () use ($elements, $renderer) {
       return $renderer->render($elements, TRUE);
     });
     return $elements['#markup'];
  }
}

That totally would work it turns out and totally be enough to bridge the gap. This is so far the simplest implementation of all D7 render_cache and D8 Stack ...

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet