Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AbstractViewHelper::renderChildrenClosure is not reset properly on shared ViewHelpers, causing broken output on first render #1004

Open
gardiner opened this issue Sep 11, 2024 · 2 comments

Comments

@gardiner
Copy link

Summary

When using a shared ViewHelper with (different) nested content from different partials on a single page, later instances of the ViewHelper incorrectly output the content of previous instances (but only on the first render of a page).

Details

The issue is probably caused because the ViewHelperInvoker will only update the renderChildrenClosure of a shared view helper if a new renderChildrenClosure exists (which is not the case if the specific partial has not been rendered yet), see ViewHelperInvoker. This causes the shared ViewHelper to use a 'stale' renderChildrenClosure from a previous invokation, producing old and incorrect output.

Fix/Workaround

So far we have found (at least) three temporary fixes:

a) Making the ViewHelper in question not shared, e.g. by adding an entry for the specific ViewHelper class to Services.yaml with the property shared: false
b) Manually setting renderChildrenClosure to null inside the ViewHelper, e.g. in ViewHelper::initialize()
c) (more a workaround than a fix) Creating individual subclasses of the ViewHelper and use them for different nested content.

@s2b
Copy link
Contributor

s2b commented Sep 11, 2024

Thank you for this helpful input on this topic. We are aware that shared ViewHelper instances still aren't fully supported yet, so the recommendation would be to set ViewHelpers to shared: false for now. TYPO3 does this already for all ViewHelpers automatically:

https://github.com/TYPO3/typo3/blob/main/typo3/sysext/fluid/Configuration/Services.php#L19

However, b) sounds like a bug nonetheless. I briefly looked into it: Would it help to allow null as a value for the setter and to set it to null instead of not setting it at all in the invoker?

@gardiner
Copy link
Author

Hi @s2b, thanks for your quick response. Yes, I agree, adjusting the setter to accept null and thus allowing it to be reset from the invoker sounds like a clean approach! I tested it and it works as expected. The only downside I could see is that this would lead to a change of the method signature of ViewHelperInterface::setRenderChildrenClosure() which might cause issues for subclasses, but I'm actually not sure about that :)
Cheers, Ole.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants