[TwigBundle] Added a RuntimeExtensionInterface to take advantage of autoconfigure #23037
Conversation
| @@ -150,6 +151,7 @@ public function load(array $configs, ContainerBuilder $container) | |||
| $container->registerForAutoconfiguration(\Twig_LoaderInterface::class)->addTag('twig.loader'); | |||
| $container->registerForAutoconfiguration(ExtensionInterface::class)->addTag('twig.extension'); | |||
| $container->registerForAutoconfiguration(LoaderInterface::class)->addTag('twig.loader'); | |||
| $container->registerForAutoconfiguration(RuntimeExtensionInterface::class)->addTag('twig.runtime'); | |||
|
|
|||
| if (\PHP_VERSION_ID < 70000) { | |||
There was a problem hiding this comment.
Why? (3.* Still supports php 5)
| * | ||
| * @author Grégoire Pineau <lyrixx@lyrixx.info> | ||
| */ | ||
| interface RuntimeExtensionInterface |
There was a problem hiding this comment.
What about moving this to Twig itself?
There was a problem hiding this comment.
It is useless in twig, isn't ?
There was a problem hiding this comment.
but having an interface in SF only will prevent non SF users from tagging their runtime on their side
+1 for Twig (1.x)
There was a problem hiding this comment.
But it is useless to tag the runtime interface on their side. It is only useful in a symfony context.
What is the benefits of moving it to twig ?
There was a problem hiding this comment.
the benefit is that SF can leverage it (as you do here),
and others can also :)
There was a problem hiding this comment.
Should I do that?
Because for me it does not make sense to introduce an interface in twig.
It's just plain dead code. What would be the documentation for this Interface?
There was a problem hiding this comment.
but then it means that your library providing a reusable extension can either decide to couple itself to Symfony, or cannot benefit from autowiring when using it with Symfony (and then, other frameworks supporting autowiring also need to add their own version of this interface).
So I'm in favor of adding the marker interface in Twig.
There was a problem hiding this comment.
| @@ -12,6 +12,7 @@ | |||
| namespace Symfony\Bundle\TwigBundle\DependencyInjection; | |||
|
|
|||
| use Symfony\Bridge\Twig\Extension\WebLinkExtension; | |||
| use Symfony\Bundle\TwigBundle\RuntimeExtensionInterface; | |||
There was a problem hiding this comment.
to be replaced by the upcoming Twig interface (and this one removed)
|
PR updated. |
|
Thank you @lyrixx. |
… advantage of autoconfigure (lyrixx) This PR was merged into the 3.4 branch. Discussion ---------- [TwigBundle] Added a RuntimeExtensionInterface to take advantage of autoconfigure | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - (asked by @nicolas-grekas) | License | MIT | Doc PR | - --- Before: ```yaml #app/config/services.yml services: _defaults: autoconfigure: true AppBundle\Twig\HelloExtension: tags: ['twig.runtime'] ``` After: ```yaml #app/config/services.yml services: _defaults: autoconfigure: true #Yes, there are nothing anymore ``` Commits ------- ba8763b [TwigBundle] Added a RuntimeExtensionInterface to take advantage of autoconfigure
Before:
After: