Bug #100196
closed
Remove "final" on public (!) core viewhelpers
0%
Description
Making core viewhelpers final has a lot of bad consequences and it does not really add any value. Documentation reasons for adding final are as follows:
The Core takes this step to clarify that single ViewHelpers are not part of the PHP API, their internal handling may change any time, which is not considered breaking... Using ViewHelpers provided by Core extensions in Fluid templates is of course fine as long as they are not marked @internal. Arguments to casual ViewHelpers are considered API and are subject of the general Core deprecation strategy. In general, the base extensions EXT:fluid, EXT:core, EXT:frontend and EXT:backend deliver various general purpose ViewHelpers that can be used, while specific extensions like EXT:beuser add @internal ViewHelpers that should not be used in own templates.
This is all fine but then <f:image> viewhelper, for example, was and is the way to go in custom extensions. It is not internal, however it is final. It causes all kind of issues with developing any serious project that wants to build upon TYPO3 as a framework.
Firsts, what does final has anything to do with internal implementation of viewhelpers? If you want to extend the viewhelper, you call its parent method and build upon the result. Whatever is inside the parent method can change in any way as long as the result of the calls is ok. For example, <f:image> is expected to return the image and it is not important what is inside. But I want to globally wrap it with something, or change its output, I either make my own viewhelper extending <f:image> under another name or I xclass <f:image>. I absolutely do not care about internal implementation because I am fine as long as it works and returns proper tags. And final has absolutely nothing to do with internal code of this viewhelper.
Secondly, in programming final is general used for something that you should not change. Something like a logging severity. However the class that implements a certain logic is rarely final because by definition OOP is meant to extend and reuse. For example, TYPO3 core has a class named BestUrlMatcher, which extends Symfony's UrlMatcher. Also MatchedRoute extends Symfony's Route. Would it be good if Symfony suddenly makes those classes final because "its internal implementation should not change"? I guess not because then TYPO3 will have to copy most of these classes locally and modify. This will be bad OOP. TYPO3 core reuses and extends a lot of Symfony classes but it suddenly disallows people to reuse its own classes? Not really fair...
Next, this is bad from the security point of view. Since viewhelpers are final, people will simply start copying them to local extensions if they want to extend them. Most likely they will modify them in the same place. Now if a security issue is discovered in a viewhelper, it will be fixed in the core. However, how many sites will rework their copied code and patch it? If they could extend the code, they would get the update automatically. If they have to copy the code, they will not get the update automatically and it may cause their TYPO3-based web sites less security, which will indirectly taint TYPO3 reputation as a pretty secure CMS.
Next, bug fixes, same as above: bug fixes in the framework will work as long as the source class was not copied locally. So this gets broken by final too.
Next, it simply breaks the spirit of TYPO3. I am using TYPO3 since version 3.4 and I was a core dev for many years, so I know what I am talking about. One of TYPO3 goals was to provide a flexible reusable framework that developers can build upon. This final change tells developers: "This is ours, you cannot reuse anymore". It also makes a bad precedent telling developers that the system becomes closed and more components can become non-modifiable in future, which is against the whole TYPO3 spirit and general principles of "open source" being open, reusable and extendable.
Of course, a developer could use composer patching to get rid of this change but why would TYPO3 want to complicate life for developers that way?
What good is archived by making viewhelpers final? Nothing really. It simply makes life worse for developers and projects will be less secure.
Please, revise this decision! I agree that internal viewhelpers could or should be final. But public viewhepers provided by TYPO3 to every other developer should be extendable.
Thank you.
Updated by Christian Kuhn about 1 year ago
- Status changed from New to Rejected
Dmitry Dulepov wrote:
Firsts, what does
finalhas anything to do with internal implementation of viewhelpers? If you want to extend the viewhelper, you call its parent method and build upon the result. Whatever is inside the parent method can change in any way as long as the result of the calls is ok. For example,<f:image>is expected to return the image and it is not important what is inside. But I want to globally wrap it with something, or change its output, I either make my own viewhelper extending<f:image>under another name or I xclass<f:image>. I absolutely do not care about internal implementation because I am fine as long as it works and returns proper tags. Andfinalhas absolutely nothing to do with internal code of this viewhelper.
You can "overload" the view helper resolver in this case and let it resolve to some other class if `f:image` is called. Use existing configuration options. No need to extend core VH's.
Secondly, in programming
finalis general used for something that you should not change.
Exactly. You must not extend VH classes. They are not API. That's why they are final, now.
Next, this is bad from the security point of view. Since viewhelpers are final, people will simply start copying them to local extensions if they want to extend them. Most likely they will modify them in the same place. Now if a security issue is discovered in a viewhelper, it will be fixed in the core. However, how many sites will rework their copied code and patch it? If they could extend the code, they would get the update automatically. If they have to copy the code, they will not get the update automatically and it may cause their TYPO3-based web sites less security, which will indirectly taint TYPO3 reputation as a pretty secure CMS.
Example: When you for instance overwrite render() in image VH, you most likely can't call parent::render() since the only thing you can do is adding stuff to $this->tag depending on additional arguments you'd have to declare as well. And this relies on parent and will break in case image VH decides to init the tag again in render(), or if it is made static. The image VH is considered 'complete' with existing arguments. If you need more, create a feature request, or have your own VH which will then most-likely not need all the other arguments of image VH. In most cases, you'll have to have an own render(), without calling parent, which voids your 'security' argument.
Next, bug fixes, same as above: bug fixes in the framework will work as long as the source class was not copied locally. So this gets broken by
finaltoo.
Same scenario as above.
Please, revise this decision! I agree that internal viewhelpers could or should be final. But public viewhepers provided by TYPO3 to every other developer should be extendable.
VH's will stay final. Reduce your technical debt. You'll find your solutions will be more sustainable, especially when upgrading. Neither xclass'ing nor extending "leaf" classes misusing core code as API are a good solution. Core changes VH's at will, don't rely on their internals. Declaring them final hardens that on language level. You can "inject" your own VH by manipulating the resolver via config if you really need to do this.