Epic #73121
closedRender() method arguments should be moved to initializeArguments()
100%
Description
To prevent any errors with PHP7 and subclassed ViewHelpers if/when render() method signatures change. Moving the argument registrations away from the render() method means the render() method signature will never change, making it safe to subclass. Medium complexity due to size of code base needing changes.
Updated by Helmut Hummel almost 9 years ago
- Status changed from New to Needs Feedback
Imho subclassing view helpers is an anti pattern.
VHs should not be open for subclassing and should be considered final.
If we have shared functionality, this should be encapsuled in a classed and composition should be used in the VHs in question to pull this functionality in.
See ImageViewHelper as an example.
Updated by Helmut Hummel almost 9 years ago
To be clear: I would support to introduce the requirement to use `initializeArguments()` but for other reasons:
We can add the render method and the initializeArguments methods to the interface with the following benefits:
- Clearer API (only one style to do things)
- It is a much clearer contract for VH authors
- It reduces runtime magic resoliving
Updated by Helmut Hummel almost 9 years ago
To make this reality the following things need to be done:
- Change all core VHs
- Deprecate render methods with arguments (with target for removal in 9)
Updated by Claus Due almost 9 years ago
There are other benefits than being able to subclass the ViewHelpers. For example, avoiding the reflection framework and becoming a better Fluid citizen (not requiring special execution handling to support render method arguments).
As such, the topic of this discussion should not be "should be allow subclassing ViewHelpers" but rather "should we move argument registrations to avoid reflection and executions only supported on TYPO3 CMS". Whether or not we should even allow subclassing ViewHelpers should be another discussion.
Updated by Claus Due almost 9 years ago
To make this reality the following things need to be done:
Yes, that is what I intend to do.
Updated by Claus Due almost 9 years ago
I would like to add here that we can actually scrap the render() method completely. It's not needed. If renderStatic exists, and it always does, that method can be called directly.
Updated by Alexander Opitz over 8 years ago
- Status changed from Needs Feedback to New
- Target version changed from 8.0 to 8 LTS
Updated by Mathias Schreiber about 8 years ago
- Status changed from New to Resolved
Updated by Riccardo De Contardi about 7 years ago
- Status changed from Resolved to Closed