From b44c1ae639bea0bbde8623b191e05b9cb17fb2cf Mon Sep 17 00:00:00 2001 From: Simon Praetorius Date: Sat, 17 Aug 2024 19:04:57 +0200 Subject: [PATCH] [BUGFIX] Properly restore renderingContext in renderChildren() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It has long been possible to render recursive templates with Fluid, either by using partials or sections. In both cases, `` is used to initiate the recursive behavior. For example, the following template creates a recursive main menu by using sections: ```xml
  • {item.title}
``` However, this was only working because `ForViewHelper` was implemented as static ViewHelper by using Fluid's trait `CompileWithRenderStatic`. The trait changed Fluid's handling of ViewHelpers in a way that variables stay consistent even during recursions. This was made possible back in Fluid 1.0.5 with 819ca245f335a34a55e5c12818770091af73d259. However, this never worked properly with ViewHelpers not using `renderStatic()` but rather the default `render()` method. In those implementations, `$this->renderChildren()` is used to access and render the ViewHelper's child nodes rather than executing the closure directly passed to `renderStatic()`. To support recursions also for ViewHelpers not using `renderStatic()`, this patch introduces new state to `AbstractViewHelper` to keep track of rendering contexts during recursions. Note that this state is only used for uncached templates, for cached templates this already worked without any changes. To explain the new behavior, let's take the example above and let's also assume that `ForViewHelper` is implemented using `render()` rather than `renderStatic()`. In uncached templates, the new behavior works as follows: 1. `$view->render()` gets called for the template 2. `TemplateParser` creates nodes from template string, among them one `ViewHelperNode` for each ViewHelper. 3. `RenderViewHelper` gets called by executing `$viewHelperNode->execute()`, which in turn calls `ViewHelperInvoker`, which in the end calls the render method of the ViewHelper (in this case `RenderViewHelper::renderStatic()`) 4. `RenderViewHelper` calls `$view->renderSection()`, which clones the current rendering context and puts the appropriate variables in it. The old rendering context gets saved to be restored later for the parent template. 5. Inside the section, `ForViewHelper` gets called, we end up in `ForViewHelper::render()` (because we use our custom implementation without `renderStatic()`) 6. `$this->renderChildren()` is called for each `{item}`. 7. If `{item.children}` is defined, things get interesting. `RenderViewHelper` calls `$view->renderSection()` again, which creates and saves another new rendering context, see 4. We're in a recursion now. 8. With each recursion level finishing, the view restores the original rendering context, so that the variables before the recursion are available to the rest of the template file. So the template/partial/section gets restored properly, but the `ForViewHelper` still has the last rendering context from within the recursion because Fluid didn't restore it to the previous state. In fact, Fluid couldn't really do that from outside because `$forViewHelper->renderChildren()` initiated the whole recursion and is still running (once for each recursion level). To solve this dilemma, this patch implements a rendering context stack similarly to the one used in the `$view` object, but on the ViewHelper level. Each ViewHelper keeps track of recursive rendering contexts for itself – again, only for uncached, non-static ViewHelpers. In addition to that, the patch simplifies the logic of `buildRenderChildrenClosure()`, which covers both cached/uncached and the contentArgumentName feature. --- src/Core/ViewHelper/AbstractViewHelper.php | 38 +++++++++++++--------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/Core/ViewHelper/AbstractViewHelper.php b/src/Core/ViewHelper/AbstractViewHelper.php index 9d10dc157..6e025333e 100644 --- a/src/Core/ViewHelper/AbstractViewHelper.php +++ b/src/Core/ViewHelper/AbstractViewHelper.php @@ -68,6 +68,16 @@ abstract class AbstractViewHelper implements ViewHelperInterface */ protected $renderingContext; + /** + * Stores rendering contexts in a situation where ViewHelpers are called recursively from inside + * one of their child nodes. In that case, the rendering context can change during the recursion, + * but needs to be restored properly after each run. Thus, we store a stack of rendering contexts + * to be able to restore the initial state of the ViewHelper. + * + * @var RenderingContextInterface[] + */ + protected array $renderingContextStack = []; + /** * @var \Closure */ @@ -308,23 +318,19 @@ public function renderChildren() */ protected function buildRenderChildrenClosure() { - $argumentName = $this->getContentArgumentName(); - $arguments = $this->arguments; - if ($argumentName !== null && isset($arguments[$argumentName])) { - $renderChildrenClosure = function () use ($arguments, $argumentName) { - return $arguments[$argumentName]; - }; - } else { - $self = clone $this; - $renderChildrenClosure = function () use ($self) { - if ($self->renderChildrenClosure !== null) { - $closure = $self->renderChildrenClosure; - return $closure(); - } - return $self->viewHelperNode->evaluateChildNodes($self->renderingContext); - }; + $contentArgumentName = $this->getContentArgumentName(); + if ($contentArgumentName !== null && isset($this->arguments[$contentArgumentName])) { + return fn() => $this->arguments[$contentArgumentName]; + } + if ($this->renderChildrenClosure !== null) { + return $this->renderChildrenClosure; } - return $renderChildrenClosure; + return function () { + $this->renderingContextStack[] = $this->renderingContext; + $result = $this->viewHelperNode->evaluateChildNodes($this->renderingContext); + $this->setRenderingContext(array_pop($this->renderingContextStack)); + return $result; + }; } /**