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

Add filter to $output before it is cached #2910

Merged
merged 3 commits into from Feb 24, 2024
Merged

Add filter to $output before it is cached #2910

merged 3 commits into from Feb 24, 2024

Conversation

jl-a
Copy link
Contributor

@jl-a jl-a commented Jan 30, 2024

Related:

Issue

When filtering $output with timber/output the result is not cached. This is inefficient if the filter being applied is deterministic based on $output content, because the filter results could also be cached rather than being run every time.

Solution

This pull request adds another filter, timber/output/pre-cache, immediately after the template is rendered and before the $output is cached. If this filter is used, then its results will be cached.

Impact

This pull request is backwards compatible, and does not break any existing uses of timber/output. The apply_filters call may add a tiny bit of overhead to each render (not including the actual filter, if any are added) , but overall this could be considered negligible.

Usage Changes

No changes to how Timber is used, other than an additional filter.

Considerations

Testing

No tests needed.

@Levdbas Levdbas added this to the 2.1.0 milestone Feb 22, 2024
@coveralls
Copy link

coveralls commented Feb 24, 2024

Coverage Status

coverage: 87.914% (-0.8%) from 88.741%
when pulling c417ecc on jl-a:2.x
into a602ec5 on timber:2.x.

Copy link
Member

@nlemoine nlemoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all good thanks @jl-a!

@nlemoine nlemoine merged commit d1356fd into timber:2.x Feb 24, 2024
25 of 27 checks passed
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.

None yet

4 participants