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 getLastModified() to extensions #4513

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

GromNaN
Copy link
Contributor

@GromNaN GromNaN commented Dec 23, 2024

Give to extensions the ability to set a last modification date for cache invalidation.

Runtime

Currently, the cache is not invalidated when the signature of a runtime method is modified. This is an issue for templates that use named arguments, as argument names have an impact on the generated class.

With this change, extensions using runtime classes can compute a modification date by including the files on which they depend.

By default, the AbstractExtension checks if there is a file for the runtime class with the same name of the extension suffixed by Runtime instead of Extension.
This is the convention applied in Symfony and Twig Extra: MarkdownExtension has MarkdownRuntime.

Attribute

Contributing to #3916.

The extension class that will get the configuration from attributes will be able to track the classes having attributes to find the last modification date of all this classes.

BC break

In Twig 4.0, the method getLastModified will be added to ExtensionInterface. It is extremely rare to implement this interface without extending AbstractExtension. So adding this method to the interface shouldn't be a problem as the base class has an implementation.

src/ExtensionSet.php Outdated Show resolved Hide resolved
@GromNaN GromNaN force-pushed the extension-last-modified branch from 2b3cd04 to 3a94244 Compare December 24, 2024 10:43
@smnandre
Copy link
Contributor

Do we need to add it to the interface ? Could we not simply ignore extensions not implementing this interface ?

@GromNaN
Copy link
Contributor Author

GromNaN commented Dec 26, 2024

Is it a thing to implement the interface without extending the abstract class? Adding this method should not be an issue for the large majority of projects, but it helps keeping the code simple.

@smnandre
Copy link
Contributor

Well this may break external extensions in prod ... but you're right:

  • there is some time before 4.0
  • no one is forced to upgrade day one
  • and .... yes in the vast majority i guess AbstractExtension is behind :)

@fabpot
Copy link
Contributor

fabpot commented Dec 29, 2024

Is it a thing to implement the interface without extending the abstract class? Adding this method should not be an issue for the large majority of projects, but it helps keeping the code simple.

"large majority" is not enough when talking about BC breaks :)

Let's add a proper interface as this is a new capability (we've done it in the past with ExistsLoaderInterface for instance).

@GromNaN GromNaN force-pushed the extension-last-modified branch from 036e48b to 36ddfc4 Compare December 29, 2024 12:42
@GromNaN
Copy link
Contributor Author

GromNaN commented Dec 29, 2024

Let's add a proper interface as this is a new capability

Ok, interface added.

I'll also add automatic detection of Runtime classes to make it automatically work with Symfony TwigBridge extensions.

@GromNaN GromNaN force-pushed the extension-last-modified branch from 4fdc4b7 to d8bc954 Compare December 29, 2024 13:27
@GromNaN GromNaN changed the title Add ExtensionInterface::getLastModified() Add getLastModified() to extensions Dec 30, 2024
doc/advanced.rst Outdated Show resolved Hide resolved
src/Extension/LastModifiedExtensionInterface.php Outdated Show resolved Hide resolved
@GromNaN GromNaN force-pushed the extension-last-modified branch from d8bc954 to a6b96ab Compare January 2, 2025 13:30
@fabpot
Copy link
Contributor

fabpot commented Jan 2, 2025

@GromNaN LGMT, can you add a note in the CHANGELOG (you will need to rebase as I've just bump the version to 3.19.0)?

…xtension` to track modification of runtime classes
@GromNaN GromNaN force-pushed the extension-last-modified branch from a6b96ab to d8fe3bd Compare January 2, 2025 13:51
@GromNaN
Copy link
Contributor Author

GromNaN commented Jan 2, 2025

Rebased, squashed, changelog updated.

@fabpot
Copy link
Contributor

fabpot commented Jan 3, 2025

Thank you @GromNaN.

@fabpot fabpot merged commit 5dd37d8 into twigphp:3.x Jan 3, 2025
50 checks passed
@GromNaN GromNaN deleted the extension-last-modified branch January 3, 2025 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants