added Twig runtimes for "critical" Twig extensions #20094
Conversation
| * | ||
| * @see FragmentHandler::render() | ||
| */ | ||
| public function renderFragment($uri, $options = array()) |
There was a problem hiding this comment.
While I agree with this change, it might break hacky solutions that wrap around this using decorators or extending this extension.
Twig extensions are not explicitly mentioned in the BC promises but should imo be considered internal (thus would allow this change)
There was a problem hiding this comment.
Let's be pragmatic here. This is definitely not an extension point.
| * | ||
| * @author Fabien Potencier <fabien@symfony.com> | ||
| */ | ||
| class HttpKernelRuntime |
There was a problem hiding this comment.
As usual: any reason to allow inheritance?
| { | ||
| return $this->handler->render($uri, $strategy, $options); | ||
| } | ||
|
|
||
| public function controller($controller, $attributes = array(), $query = array()) |
There was a problem hiding this comment.
if we want to optimize even further, we could turn such methods into static methods
| @@ -71,5 +71,17 @@ public function process(ContainerBuilder $container) | |||
| if ($container->has('assets.packages')) { | |||
| $container->getDefinition('twig.extension.assets')->addTag('twig.extension'); | |||
| } | |||
|
|
|||
| if (class_exists('Symfony\Component\Yaml\Parser')) { | |||
There was a problem hiding this comment.
As the definition of the container now depends on the existence of a class, we should define a ClassExistenceResource and add such a resource to the container for such patterns (outside the if), so that the debug container is reloaded if you install the class
There was a problem hiding this comment.
That's for another PR as this is not the only place where we are doing this in a compiler pass.
40e9cb5 to
2ac3e4a
Compare
…onent is not available
…ssionLanguage component is not available
2ac3e4a to
c541804
Compare
…bpot) This PR was merged into the 3.2-dev branch. Discussion ---------- added Twig runtimes for "critical" Twig extensions | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a This PR converts some Twig extensions to use a separate runtime. It also contains some optimizations to not load extensions when the associated component is not installed. Commits ------- c541804 fixed tests 812fbb4 decoupled the Twig HttpKernelExtension runtime from the extension 79efb4c [TwigBundle] removed ExpressionLanguage Twig extension when the ExpressionLanguage component is not available 3d4ad0b [TwigBundle] removed Stopwatch Twig extension when the Stopwatch component is not available d0792e4 [TwigBundle] removed YAML Twig extension when the YAML component is not available
This PR was merged into the 3.2-dev branch. Discussion ---------- Class existence resource | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | see #20094 | License | MIT | Doc PR | n/a Commits ------- 222b56d [TwigBundle] added support for ClassExistenceResource when relevant d98eb7b [Config] added ClassExistenceResource
This PR converts some Twig extensions to use a separate runtime. It also contains some optimizations to not load extensions when the associated component is not installed.