Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #979

Merged
merged 3 commits into from
Aug 20, 2024
Merged

Backport #979

merged 3 commits into from
Aug 20, 2024

Conversation

s2b
Copy link
Contributor

@s2b s2b commented Aug 20, 2024

No description provided.

Previously, `CompileWithContentArgumentAndRenderStatic` could
be used to create ViewHelpers that can get their default input both
from their child content and from a defined argument. This feature
has now been integrated into AbstractViewHelper. To use it, the
ViewHelper class needs to override the method
`getContentArgumentName()` and return the name of the
argument to be used.

Example:

```
public function initializeArguments(): void
{
    $this->registerArgument('value', 'string', '');
}

public function getContentArgumentName(): string
{
    return 'value';
}
```

`$this->renderChildren()` can then be used to get either the
defined argument or as fallback the ViewHelper's child content.

With this definition, the ViewHelper can be used in the following
ways:

```
<my:viewhelper value="test" />
<my:viewhelper>test</my:viewhelper>
{myInput -> my:viewhelper()}
```

Since it's no longer necessary, the trait is now deprecated and
will be removed with Fluid v5. There will be a soft deprecation
in Fluid v2 to let users know that this API will no longer be available
with Fluid v5 and will throw a deprecation error in v4.

All ViewHelpers still using the trait have been migrated to
`CompileWithRenderStatic`, which has been modified to support
the new implementation.
With the introduction of the new `getContentArgumentName()`
API for ViewHelpers, there has been an oversight in `TemplateParser`:
It had special handling for the old method name
`resolveContentArgumentName()`, which is now deprecated. Since
`resolveContentArgumentName()` is now called by the new API
`getContentArgumentName()`, we can just call the new API in the
`TemplateParser` now, without special handling for the deprecated
trait.

As a consequence, we need to adjust some tests and fixtures. Also,
we add more dedicated test cases to our new test file.

In the future, there needs to be a cleanup of `AbstractViewHelper`
and `ViewHelperInterface`, since internal classes depend on API
only defined in `AbstractViewHelper`. In fact, we arguably made things
a bit worse now, but this is a topic for Fluid v5.
# Conflicts:
#	phpstan-baseline.neon
#	src/Core/ViewHelper/Traits/CompileWithContentArgumentAndRenderStatic.php
@s2b s2b merged commit e073a81 into 2.15 Aug 20, 2024
10 checks passed
@s2b s2b deleted the backport branch August 20, 2024 20:18
@bwaidelich
Copy link

bwaidelich commented Sep 3, 2024

FYI: This change broke most of the Fluid adapter tests in Flow because mocking like

$this->viewHelper->expects(self::once())->method('renderChildren')->will(self::returnValue('some text'));

won't work any longer and, worse, because executing the tests leads to an exception

Error: Call to a member function evaluateChildNodes() on null
typo3fluid/fluid/src/Core/ViewHelper/AbstractViewHelper.php:329

We're investigating how to fix this on our side

@s2b
Copy link
Contributor Author

s2b commented Sep 3, 2024

Very sorry about that! If it helps, you can contact me directly and we can have a look at it together.

moin@praetorius.me

@bwaidelich
Copy link

@s2b No need to be sorry, it was just a FYI. I think we "fixed" the issue with neos/flow-development-collection#3390

@lolli42
Copy link
Member

lolli42 commented Sep 9, 2024

Quick note.
You may be better off avoiding mocks in VH tests.
Both fluid standalone and typo3 core basically transitioned to "functional" tests: Feed a TemplateView with some fluid template test string that calls the VH, render(), assert result string.
This turns out to be much more easy to read and avoids internal knowledge within tests.
Bonus: It's trivial to do this twice to test if a VH behaves identical with "template not cached and template cached" (we found and fixed quite a few issues due to this).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants