-
-
Notifications
You must be signed in to change notification settings - Fork 43
Lazy loaded Twig Extension #123
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
Conversation
d53af01 to
43f8bd4
Compare
GromNaN
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing a public method from a class is a breaking change, but I think we can make it of the next 0.minor version.
The extension attributes added in Twig 3.21.0 allows to create lazy extension with a single class. I recommend this approach.
The MercureBundle must also be updated to enable autoconfiguration (or add both twig.attribute_extension and twig.runtime tags): https://github.com/symfony/mercure-bundle/blob/main/src/DependencyInjection/MercureExtension.php#L270-L274
src/Twig/MercureExtension.php
Outdated
| * | ||
| * @return string The URL of the hub with the appropriate "topic" query parameters (if any) | ||
| */ | ||
| public function mercure($topics = null, array $options = []): string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could keep this function in this class and use the #[TwigFunction] attribute to make this extension lazy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I need also to remove this config from the bundle? https://github.com/symfony/mercure-bundle/blob/main/src/DependencyInjection/MercureExtension.php#L270-L274
6cdd947 to
3000654
Compare
# Conflicts: # src/Twig/MercureExtension.php
3000654 to
048e351
Compare
|
Hi! Unfortunately it brokes Symfony UX test suite, see symfony/ux#3185: How can we migrate this code? $mercure = $this->twig->getExtension(MercureExtension::class);
if ($eventSourceOptions['withCredentials'] ?? false) {
$eventSourceOptions['subscribe'] ??= $topics;
$controllerAttributes['withCredentials'] = true;
}
$mercure->mercure($topics, $eventSourceOptions);Thanks! |
|
@Kocal the MercureExtension is now registered as a Twig runtime. |
|
Thanks! |
https://symfony.com/doc/8.0/templates.html#templates-twig-extension