Deprecate RendererInterface::render()'s sole $is_root_call parameter

Just ran into this exception and found the @todo via 📌 Try to replace path alias preloading with lazy generation Active and 📌 Create placeholders for more things Active - if you combine the two MRs from those issues and don't have bigpipe installed, you get the exception on various pages.

Converted the patch to an MR and dealt with conflicts/cs issues.

Lots of failures. Might be worth moving Wim's patch to a second MR and see where we land with that - looks like bc could be added quite straightforwardly to Wim's approach.

OK after fixing the unit tests in Wim's approach I realised the bug in Fabian's approach, so we've now got green MRs for both.

However, this is an important improvement from Fabian's:

   $child_element = &$elements[$key];
        if (isset($child_element['#cache']['keys'])) {
          $new_context = new RenderContext();
          $elements['#children'] .= $this->executeInRenderContext($new_context, function () use (&$child_element, $new_context) {
            return $this->doRender($child_element, $new_context);
          });
          // @todo This should not be necessary.
          if (!$new_context->isEmpty()) {
            $frame = $context->pop()->merge($new_context->pop());
            $context->push($frame);
          }
        }
        else {
          $elements['#children'] .= $this->doRender($elements[$key], $context);
        }
      }

e.g. when rendering children, we no longer rely on the global render context state but or even a class property, but pass the context into the method.

I think that this will either solve, or approach solving, the issues I'm running into on 📌 Try to replace path alias preloading with lazy generation Active (where we enter and leave different rendering contexts).

catch → changed the visibility of the branch 2511330-alternate-approach to hidden.

OK #28 was premature but now it's green.

I think the next step here is 📌 Pass RenderContext around in the Renderer Active but that will require interface changes to add the parameter to a couple of methods and maybe more things, so one step at a time.

✨ Improve query and cache API so that render() doesn't have to be called to add query cache metadata Active is closely related too.

Moving this to a bug report, it wasn't a bug as such when it was originally added to core, more of a 'limitation', but now we're using fibers for placeholder rendering, if code actually fiber suspends a decent amount, which is done in 📌 Try to replace path alias preloading with lazy generation Active , everything explodes on cache misses.

The problem was (completely) missed in 📌 Add PHP Fibers support to BigPipe RTBC , but this pre-existing issue solves it, alongside 📌 Pass RenderContext around in the Renderer Active which I just opened this week as a follow-up to this issue. The combination of the two allows that path alias issue to pass nearly all tests, but anything that uses fiber suspend in multiple placeholders will trigger the same exception at the moment.

There's very good existing test coverage of this, as shown by the several commits on both MRs here tracking down and fixing various test failures. I don't think we need explicit test coverage of the fiber suspend issue yet, it could possibly be added in 📌 Pass RenderContext around in the Renderer Active but not here because as soon as you fix this issue you run into that one, keeping separate for ease of review since neither are simple and they don't conflict.

As I get it just need deprecation test

I don't think this needs a deprecation test - the deprecation path is only the trigger_error() and one line method call, no actual bc layer to test. Moving back to needs review.

Some comments on MR 10795.

Thanks for looking, think I resolved all of those.

My mistake about the use Drupal\Core\Render\RenderContext; suggestion. It's in the same namespace, so it was unnecessary and flagged by PHPCS.

Also looks like phpstan baseline needs regenerating.

Added a couple comments on the typehints too.

Argh I spotted the RenderContext thing after commit and before push, but failed to commit the change.

Pushed a commit for that and the other couple of comments. I think the phpstan complaint was real. Should be back to green.

lgtm