Task #79464

Epic #77909: Enhance EXT:form

Feature #77910: Introduce new form framework

EXT:form - Fluid usage optimization

Added by Bjoern Jacob almost 3 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Must have
Category:
Form Framework
Target version:
Start date:
2017-01-25
Due date:
% Done:

100%

TYPO3 Version:
8
PHP Version:
Tags:
Complexity:
Sprint Focus:

Description

Claus suggested some very useful and necessary optimizations:

It might solve your issue to create a fresh View instance before each of the templates in \TYPO3\CMS\Form\Controller\FormEditorController::renderFormEditorTemplates.

although it means a bit more init, you're sure to receive a completely fresh parser etc. (it would not be necessary if you switched to renderSection and kept those in - for example - a partial):

FormView.php

See https://review.typo3.org/#/c/51410/8/typo3/sysext/form/Classes/Mvc/View/FormView.php.

Line 243: protected function getCurrentParsedTemplate(): ParsedTemplateInterface
This one may come back to bite you one day. Compare with https://github.com/TYPO3/Fluid/blob/master/src/View/AbstractTemplateView.php#L346 - this variant of that method no longer respects the rendering stack and you may experience issues while rendering partials or when using layouts.

If your problem stems from reusing the View to render additional templates (and I think it does) then may I suggest a solution that's a more appropriate fix:

Use a fresh View instance to render each template file. Your template file is the entry point and as you experience, once you set it, it's hard to get rid of again (Views are not designed for such reuse).

An alternative could be to render renderables only by action name and force people to overlay TemplateRootPaths if they wish to add/override renderable templates (in other words, never specifying the actual template filename but only an emulated action name).

Personally I think you should have enforced that renderables must exist as partial template files and then provide one template file which "renders a renderable" and make that template file render an optional partial using a name provided from the renderable, with all arguments. It would be more efficient and more override-friendly and in the long run, it would free you up by not requiring a pretty significant amount of custom rendering of Views.

Other optimizations

In \TYPO3\CMS\Form\Controller\FormEditorController::renderFormEditorTemplates you might consider either rendering sections or partials instead, using $view->renderPartial / $view->renderSection. A new action template for each seems like overkill.

You might also consider assigning the objects and fitting them with the necessary getters, rather than extracting their configurations and assigning those to template variables. Third party VH implementations would thank you for making the original objects available ;)

General Q: are the path configurations truly required enough to merit an exception? If one does not provide a certain path, the expected path gets generated based on extension context (as configured in Request / RenderingContext)

\TYPO3\CMS\Form\ViewHelpers\RenderAllFormValuesViewHelper should be converted to static callable + use CompileWithRenderStatic

\TYPO3\CMS\Form\ViewHelpers\RenderRenderableViewHelper could use CompileWithContentArgumentAndRenderstatic, make $renderable optional and describe that `{renderable -> form:renderRenderable()}` is also valid (nice to have)


you should definitely consider now, refactoring all the built-in Renderables templates to become either partials or sections, then render these as optional and insert a warning "Template for Renderable %s not found in paths %s" if null is returned.
I would also consider dropping this mapping from the .yaml file (another thing is, I wouldn't have used yaml - I would have made it accept an array which could then come from, for example, TypoScript or other existing sources)
the reason for this recommendation: you copy much of the context when rendering these templates and instead of doing that, you could reuse the context and avoid nearly all your View init duplication.


Related issues

Blocked by TYPO3 Core - Bug #79519: pathinfo() does not strip sub-paths Closed 2017-01-28

Associated revisions

Revision 605b0c8d (diff)
Added by Ralf Zimmermann almost 3 years ago

[!!!][TASK] EXT:form - Refactor fluid rendering

This patch is a followup of issue #79439.

EXT:form uses "fluid" as the default rendering strategy.
Therefore, EXT:form has to work close with the concepts of fluid to
avoid current and future problems.
Until now, EXT:form tried to reuse a fluid view instance by
reconfiguring the instance on each nesting level, but fluid is not
intended for such a purpose.
Therefore, the templates have to be moved/ changed and some
configuration has to be changed.
The patch breaks EXT:form only for people who are using custom
configurations/ templates.

Resolves: #79464
Releases: master
Change-Id: I6346b888b47a52bcc995c7d4cd3acdc65a1396c8
Reviewed-on: https://review.typo3.org/51442
Tested-by: TYPO3com <>
Reviewed-by: Anja Leichsenring <>
Tested-by: Anja Leichsenring <>
Tested-by: Bjoern Jacob <>
Tested-by: Andreas Steiger <>
Tested-by: Tobi Kretschmann <>
Reviewed-by: Susanne Moog <>
Tested-by: Susanne Moog <>

History

#1 Updated by Bjoern Jacob almost 3 years ago

  • Description updated (diff)

#2 Updated by Ralf Zimmermann almost 3 years ago

  • Status changed from Accepted to In Progress

#3 Updated by Gerrit Code Review almost 3 years ago

  • Status changed from In Progress to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#4 Updated by Gerrit Code Review almost 3 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#5 Updated by Gerrit Code Review almost 3 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#6 Updated by Gerrit Code Review almost 3 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#7 Updated by Gerrit Code Review almost 3 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#8 Updated by Gerrit Code Review almost 3 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#9 Updated by Ralf Zimmermann almost 3 years ago

  • Description updated (diff)

#10 Updated by Gerrit Code Review almost 3 years ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#11 Updated by Gerrit Code Review almost 3 years ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#12 Updated by Ralf Zimmermann almost 3 years ago

  • Subject changed from Fluid usage optimization to EXT:form Fluid usage optimization

#13 Updated by Ralf Zimmermann almost 3 years ago

  • Subject changed from EXT:form Fluid usage optimization to EXT:form - Fluid usage optimization

#14 Updated by Gerrit Code Review almost 3 years ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#15 Updated by Gerrit Code Review almost 3 years ago

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#16 Updated by Gerrit Code Review almost 3 years ago

Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#17 Updated by Ralf Zimmermann almost 3 years ago

  • Target version set to 8.6

#18 Updated by Gerrit Code Review almost 3 years ago

Patch set 12 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51442

#19 Updated by Ralf Zimmermann almost 3 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#20 Updated by Riccardo De Contardi about 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF