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

[TASK] Use render() as primary ViewHelper method #983

Merged
merged 1 commit into from
Aug 24, 2024

Conversation

s2b
Copy link
Contributor

@s2b s2b commented Aug 20, 2024

Until now it was best practice to differentiate between object-based
ViewHelpers, using the object method render() as their primary render
method, and static ViewHelpers, using the static method renderStatic().
This distinction has been introduced to Fluid mainly for performance reasons:
With earlier PHP versions it was quite expensive performance-wise to create a
new ViewHelper object for each ViewHelper call. However, PHP's performance
characteristics for objects have changed quite a bit with newer versions,
which questions the usefulness of this performance optimization nowadays.

Our performance tests have shown that the performance gains are still existent
today if you compare renderStatic() to render() ViewHelpers, especially
if no further optimizations are applied to Fluid's core implementation.
However, the impact is quite small and almost not measurable in real-world
scenarios.

In contrast to that, we see meaningful opportunities for the further development
and optimization for Fluid if we streamline and simplify Fluid's APIs. It
improves the developer experience if there's only one correct way to
implement ViewHelpers and if all ViewHelpers use the same API. For Fluid
internally, this opens up new opportunities of optimization and refactoring,
which are currently not feasible to tackle.

Ultimately, we have taken the decision to deprecate renderStatic() with Fluid v4.
ViewHelpers should use render() as their primary render method from now on.
This patch deprecates both the trait CompileWithRenderStatic and the usage
of renderStatic() without any trait applied to the ViewHelper (which is probably
only an edge case). Instead, the ViewHelperInvoker is called both for uncached
and cached templates to initiate and perform each ViewHelper call.

We will backport the @deprecated annotation on CompileWithRenderStatic to
Fluid v2.15 to let users know in advance of this change.

AbstractConditionViewHelper will be adjusted to this change in a follow-up patch.
There will probably also be further code optimization patches that are made possible
because of this decision, either still in Fluid v4 or later in Fluid v5, when
renderStatic() will be removed completely.

@s2b s2b force-pushed the task/renderStaticDeprecation branch 8 times, most recently from 4fad2ea to 296b0bf Compare August 24, 2024 22:11
Until now it was best practice to differentiate between object-based
ViewHelpers, using the object method `render()` as their primary render
method, and static ViewHelpers, using the static method `renderStatic()`.
This distinction has been introduced to Fluid mainly for performance reasons:
With earlier PHP versions it was quite expensive performance-wise to create a
new ViewHelper object for each ViewHelper call. However, PHP's performance
characteristics for objects have changed quite a bit with newer versions,
which questions the usefulness of this performance optimization nowadays.

Our performance tests have shown that the performance gains are still existent
today if you compare `renderStatic()` to `render()` ViewHelpers, especially
if no further optimizations are applied to Fluid's core implementation.
However, the impact is quite small and almost not measurable in real-world
scenarios.

In contrast to that, we see meaningful opportunities for the further development
and optimization for Fluid if we streamline and simplify Fluid's APIs. It
improves the developer experience if there's only one correct way to
implement ViewHelpers and if all ViewHelpers use the same API. For Fluid
internally, this opens up new opportunities of optimization and refactoring,
which are currently not feasible to tackle.

Ultimately, we have taken the decision to deprecate `renderStatic()` with Fluid v4.
ViewHelpers should use `render()` as their primary render method from now on.
This patch deprecates both the trait `CompileWithRenderStatic` and the usage
of `renderStatic()` without any trait applied to the ViewHelper (which is probably
only an edge case). Instead, the `ViewHelperInvoker` is called both for uncached
and cached templates to initiate and perform each ViewHelper call.

We will backport the `@deprecated` annotation on `CompileWithRenderStatic` to
Fluid v2.15 to let users know in advance of this change.

`AbstractConditionViewHelper` will be adjusted to this change in a follow-up patch.
There will probably also be further code optimization patches that are made possible
because of this decision, either still in Fluid v4 or later in Fluid v5, when
`renderStatic()` will be removed completely.
@s2b s2b force-pushed the task/renderStaticDeprecation branch from 296b0bf to 54f5eb7 Compare August 24, 2024 22:14
@s2b s2b marked this pull request as ready for review August 24, 2024 22:14
@s2b s2b changed the title WIP: [TASK] Use render() as primary ViewHelper method [TASK] Use render() as primary ViewHelper method Aug 24, 2024
@s2b s2b merged commit 97aa45a into main Aug 24, 2024
8 checks passed
@s2b s2b deleted the task/renderStaticDeprecation branch August 24, 2024 22:20
s2b added a commit that referenced this pull request Aug 24, 2024
Both the trait `CompileWithRenderStatic` and ViewHelpers implementing
`renderStatic()` without using one of the deprecated traits will be deprecated
with Fluid v4 and will no longer work with v5.

ViewHelpers should use `render()` as their primary rendering method.

Related: #983
s2b added a commit that referenced this pull request Aug 25, 2024
Both the trait `CompileWithRenderStatic` and ViewHelpers implementing
`renderStatic()` without using one of the deprecated traits will be deprecated
with Fluid v4 and will no longer work with v5.

ViewHelpers should use `render()` as their primary rendering method.

Related: #983
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.

2 participants