added a Twig runtime loader #20092
added a Twig runtime loader #20092
Conversation
6c63f41 to
dee1e09
Compare
dee1e09 to
76260c9
Compare
| public function __construct(ContainerInterface $container, array $mapping) | ||
| { | ||
| $this->mapping = $mapping; | ||
| } |
There was a problem hiding this comment.
Injecting a container but not actually assigning it
There was a problem hiding this comment.
fixed, added some tests as well :)
f701d66 to
447d443
Compare
|
Seems like the TwigBundle and SecurityBundle are having some difficulties on 5.5, I think their constraints are not solid for 5.5 |
447d443 to
6a8ed85
Compare
6a8ed85 to
7fc174b
Compare
|
It was just because Twig 1.x was not merged yet into master, fixed now. |
This PR was merged into the 3.2-dev branch. Discussion ---------- added a Twig runtime loader | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | see twigphp/Twig#1913 | License | MIT | Doc PR | not yet This is the Symfony part of twigphp/Twig#1913. It introduces a Twig runtime loader. Commits ------- 7fc174b [TwigBundle] added a Twig runtime loader
| public function load($class) | ||
| { | ||
| if (!isset($this->mapping[$class])) { | ||
| throw new \LogicException(sprintf('Class "%s" is not configured as a Twig runtime. Add the "twig.runtime" tag to the related service in the container.', $class)); |
There was a problem hiding this comment.
shouldn't it return null instead, to play well with multiple runtime loaders, as supported by Twig ?
There was a problem hiding this comment.
I thought about that but I don't think people will register some other runtime loaders when using Symfony. And returning null here means that debugging an issue would be very difficult.
The other possibility would be for Twig to catch exceptions and keep it for when no loaders work.
There was a problem hiding this comment.
The other possibility is to inject a logger and log such errors. WDYT?
I don't like catching the exceptions in Twig as we try to avoid that as much as possible (mainly for performance reasons).
| $definition = $container->getDefinition('twig.runtime_loader'); | ||
| $mapping = array(); | ||
| foreach ($container->findTaggedServiceIds('twig.runtime') as $id => $attributes) { | ||
| $mapping[$container->getDefinition($id)->getClass()] = $id; |
There was a problem hiding this comment.
you should validate that services are public (and not abstract), to provide an early error message for such mistake
| @@ -128,6 +131,7 @@ | |||
| <service id="twig.form.renderer" class="Symfony\Bridge\Twig\Form\TwigRenderer" public="false"> | |||
There was a problem hiding this comment.
must be public to be lazy-loadable
This is the Symfony part of twigphp/Twig#1913. It introduces a Twig runtime loader.