Illustration: Inconsistent ViewHelper behavior uncached vs. cached

[s2b]

Note that this PR isn't intended to be merged, but to illustrate inconsistent ViewHelper behavior.

Even after eliminating variable side-effects of the ForViewHelper, it still behaves inconsistently in uncached templates when it's implemented with render() instead of renderStatic(). The reason behind this is that there will only be one instance of the ViewHelper class if the ViewHelper is called recursively in a template. In the inner loop, $this->arguments of ForViewHelper will be set again, and these new arguments will leak out to the outer loop because they are not part of renderingContext but of the ViewHelper object.

This means that even with the current Fluid implementation, there can be edge cases where ViewHelper objects are reused in uncached templates. In my opinion, there are two alternatives to resolve this:

1. ViewHelper reuse should be officially supported, which means that the ViewHelper context would need to be reset at the appropriate places (similar to ViewHelperNode::updateViewHelperNodeInViewHelper()). This would also mean that ViewHelpers could be defined as shared in Symfony DI. I think that this is doable, but could be quite confusing in the code.
2. ViewHelpers shouldn't be reused at all, which is similar to the behavior of renderStatic() and of cached templates. Just to illustrate the problem, this would be a quick'n'dirty solution:

add clone in ViewHelperNode.php:

return $renderingContext->getViewHelperInvoker()->invoke(clone $this->uninitializedViewHelper, $this->arguments, $renderingContext);

[NamelessCoder]

The reason behind this is that there will only be one instance of the ViewHelper class if the ViewHelper is called recursively in a template.

See https://github.com/TYPO3/Fluid/pull/806#issuecomment-1672211779 but apparently this was not noticed by anyone.

I believe this is exactly that side effect I warned about and it all started here: https://github.com/TYPO3/Fluid/pull/787. Back when I was maintaining things, a ViewHelper instance in a ViewHelperNode was guaranteed to be unique as every instance was created when the ViewHelperNode was created - they would remain 1:1 paired throughout rendering.

TL;DR: don't declare ViewHelpers as shared instances, it's a can of worms. I recommend rolling back and rethinking those design decisions starting from https://github.com/TYPO3/Fluid/pull/787 and clearly documenting that if you declare your ViewHelper as a shared instance or singleton - then you are solely responsible for ensuring that it works correctly as a shared instance or singleton.

[s2b]

I have validated that the illustrated problematic behavior has already been present in Fluid v1.0.5. 819ca245f335a34a55e5c12818770091af73d259 successfully addressed the problem for the ForViewHelper back then, however it was already using renderStatic() back then. The same code is in effect today and still works for renderStatic().

However, if I check out v1.0.5 locally, modify the ForViewHelper to use render() instead of renderStatic(), the behavior is the same as today: Recursive rendering loses variable states. This means that renderChildren() has never been able to properly isolate the rendering context if a ViewHelper wasn't defined as static.

[s2b]

This issue is addressed with #970