Bug #73565

AbstractConditionViewHelper should not implemented CompilableInterface but the IfViewHelper should

Added by Andreas Allacher almost 2 years ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Should have
Assignee:
-
Category:
Fluid
Start date:
2016-02-19
Due date:
% Done:

100%

TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:
On Location Sprint

Description

Due to the CompilableInterface improvments in 7.6 the AbstractconditionViewHelper now implements CompilableInterface.
However that doesn't allow one to create an own Condition ViewHelper without CompilableInterface.

The correct way would be to add CompilableInterface to the IfViewHelper and remove it from AbstractConditionViewHelper.


Related issues

Related to TYPO3 Core - Task #76874: Method evaluateCondition on AbstractConditionViewHelper should be public Closed 2016-06-29

Associated revisions

Revision de4f139b (diff)
Added by Claus Due over 1 year ago

[TASK] Move CompilableInterface to each condition ViewHelper

This change moves the CompilableInterface away from the abstract
condition ViewHelper class, over to each of the implementations.
This is done in order to prevent incompatible third party
ViewHelpers from incorrectly evaluating conditions - instead,
such ViewHelpers will now be uncompilable but function correctly.

Any third party ViewHelper can opt-in to being compilable by
implementing the CompilableInterface and ensuring that the
``evaluateCondition`` method is available (and is at least of
``protected`` visibility).

Change-Id: If2dac75debe2ce5872a79d8e54037bb338240f27
Resolves: #73565
Releases: 7.6
Reviewed-on: https://review.typo3.org/48725
Reviewed-by: Anja Leichsenring <>
Tested-by: Anja Leichsenring <>
Reviewed-by: Jigal van Hemert <>
Reviewed-by: Jan Helke <>
Tested-by: Jan Helke <>

History

#1 Updated by Mathias Schreiber almost 2 years ago

Note to self: This is 7.6 only

#2 Updated by Andreas Allacher almost 2 years ago

True.
Fluid Standalone / master doesn't have CompileableInterface yet, does it?

#3 Updated by Claus Due almost 2 years ago

This is indeed only relevant for 7.6.

Regarding CompilableInterface on Fluid standalone (and also the TYPO3 adapter, and the Flow adapter most likely too): it is intentionally removed, never to be re-integrated. It is replaced with a default implementation that compiles execution via the ViewHelperInvoker unless overridden with a custom compile() method. To stop the compiler from compiling, a Fluid StopCompilingException must be thrown from the compile() method.

It simply disappears on 8.0 and onward, with CompilableInterface becoming a low-level PHP alias (not XCLASS or similar) for ViewHelperInterface, for compatibility reasons. It has already been removed from all core ViewHelper classes in dev-master.

My advice/conclusion: this compile() method should either be made to call "evaluateCondition" which should be made public so it can be called statically from compiled templates. That should allow any condition ViewHelper which implements evaluateCondition to function in both uncompiled and compiled methods.

#4 Updated by Andreas Allacher almost 2 years ago

So any ViewHelper is basically compiled in 8.x (Standalone Fluid) and called via the ViewHelperInvoker.
That way one can also use member variables and if not overriding renderStatic the render Method of a newly created class via ViewHelperInvoker would be called?
And thanks to the ViewHelperResolver overwrite in CMS fluid, dependency injection should also still work correctly.
Will have to do some tests here, once I have time.
Personally, I like the removal in regards to CompilableInterface. That way if one really wants to make a non compilable ViewHelper it is way easier as otherwise any extended ViewHelper would also implement CompileableInterface.
And compile only helped if one was able to make a ViewHelper completely static.

#5 Updated by Claus Due almost 2 years ago

In bullet point form here are the levels of compilability a ViewHelper can support.

  • Wholly uncompilable (only possible by throwing StopCompilingException from custom compile() method and will prevent any other compiling-related feature from being used; effect corresponds to not implementing CompilableInterface in earlier Fluid versions)
  • No custom implementation, just render() method override, causing ViewHelperInvoker to invoke the ViewHelper with that method and the compiled arguments array and child closure.
  • Custom renderStatic() method to replace the default implementation with the goal of making more efficient calls (avoiding for example ViewHelperInvoker). Should be called from render() method too.
  • Custom compile() method generating compiled code with more efficient calls to renderStatic(). Just as an example, to discard or change the render children closure.
  • Custom compile() method generating compiled code that calls purely static methods or methods on any currently available variable or property on AbstractCompiledTemplate (and the few methods added into the compiled class).

ViewHelpers which use child nodes or the ViewHelperNode are necessary to mention here: although child node access isn't directly related to compiling, it is only possible to do at two points:

  • In a custom compile() method to generate the PHP code that gets put in the compiled template.
  • In a render() method or methods exclusively called via render() - may not be available in methods called via renderStatic(), validateArguments() or any other public method other than render().

But how and why to work with such nodes is a completely different chapter :)

#6 Updated by Gerrit Code Review over 1 year ago

  • Status changed from New 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/48724

#7 Updated by Gerrit Code Review over 1 year ago

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

#8 Updated by Anja Leichsenring over 1 year ago

  • Sprint Focus set to On Location Sprint

#9 Updated by Anonymous over 1 year ago

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

Also available in: Atom PDF