Project

General

Profile

Actions

Task #73046

closed

Suggested rule breaking alias AbstractNode -> ViewHelperNode for backwards compatibility

Added by Claus Due about 8 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2016-01-31
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
8
PHP Version:
Tags:
Complexity:
easy
Sprint Focus:

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).

Actions #1

Updated by Claus Due about 8 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.

Actions #2

Updated by Gerrit Code Review about 8 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

Actions #3

Updated by Gerrit Code Review about 8 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

Actions #4

Updated by Gerrit Code Review about 8 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

Actions #5

Updated by Gerrit Code Review about 8 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

Actions #6

Updated by Anonymous about 8 years ago

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

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF