Task #73046
closedSuggested rule breaking alias AbstractNode -> ViewHelperNode for backwards compatibility
100%
Description
This suggested change breaks good practice but could be judged important enough to implement regardless; if nothing else then as a temporary measure and inform users of the deprecation and how to migrate.
Problem:
ViewHelperInterface::compile() in standalone Fluid now requires an actual ViewHelperNode instead of AbstractNode as it was before. The reason for changing this should be fairly obvious.
Implication:
This signature change means that ViewHelpers which implement a custom compile() method must update their signature (dot not apply to ViewHelpers implementing only renderStatic). Two ViewHelpers in TYPO3 CMS Fluid were migrated as part of the standalone Fluid merge - but third-party ViewHelpers would require either migration or an alias.
Rule break:
The alias would have to be created so that TYPO3\CMS\Fluid\Core\Parser\SyntaxTree\AbstractNode becomes an alias of TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\ViewHelperNode. This is obvious less than ideal but solves the problem immediately and prioritises not breaking the public API by somewhat breaking the non-public API.
Consequences:
Implementing such an alias would impact two things, both of which would be considered internal API:
- Overridden TemplateParsers (before standalone Fluid merge) might not function, depending on the nature of overrides (conditions checking class names may fail)
- Custom Nodes (implemented via an overridden TemplateParser, before standalone Fluid) would break either in function or form (incompatible constructor signatures on php7 at least)
Both of which are definitely considered edge cases, of which I personally only know a single implementation - the template analyzer/validator of EXT:builder.
I suggest implementing this alias despite the rule breaking it implies. The benefits (non-breaking public API during template compiling) should far outweigh the breaks caused to internal API as described in the two edge cases above.
The change should be fitted with a breaking changes RST and inform about how to migrate (switch AbstractNode to ViewHelperNode in compile() method signature of third-party ViewHelpers).
Updated by Claus Due almost 9 years ago
Added note: any such custom TemplateParser implementation would already break in many other ways; most imported classes would no longer exist, internal function names have changed and the actual conversion now happens in the NodeConverter. As such, anyone implementing a solution that would be affected by this "bad" alias would already face more serious issues requiring migration and migrating would automatically mean moving away from the affected AbstractNode class.
Updated by Gerrit Code Review almost 9 years ago
- Status changed from New to Under Review
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/46420
Updated by Gerrit Code Review almost 9 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/46420
Updated by Gerrit Code Review almost 9 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/46420
Updated by Gerrit Code Review almost 9 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/46420
Updated by Anonymous almost 9 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 41e20868794d0abfcd6771e67695791d16b9bc1d.