Expose a way to access template data and methods in a portable way #2236

fabpot · 2016-11-10T21:56:33Z

Twig_Template is an internal class, and most of its methods are marked as being @internal as they are not "safe" to use by end users.

But some of its features are interesting like renderBlock() which is very useful when rendering emails for instance (see #1873).

This PR tries to address both issues by marking the whole Twig_Template class as being internal and by exposing a new way to safely interact with a template besides the obvious render()/display() methods via $twig->load("...").

If also add some convenient methods like hasBlock() or getBlocks() (see ~~#1831~~ and #1882):

$template = $twig->load('index');

echo $template->render($context);
$template->display($context);

if ($template->hasBlock('name') {
   $template->displayBlock('name', $context);
}

foreach ($template->getBlocks() as $block) {
    echo $template->render($block, $context);
}

fabpot · 2016-11-10T22:13:35Z

Comparison of how to do it now (but does not really work well), and the new safe way:

$parameters  = array(
    'customer' => $customer
);

// old unsafe way
$template = $twig->loadTemplate('mail.twig');

$subject  = $template->renderBlock('subject', $parameters);
$bodyHtml = $template->renderBlock('body_html', $parameters);
$bodyText = $template->renderBlock('body_text', $parameters);

// new safe way
$template = $twig->load('mail.twig');

$subject  = $template->renderBlock('subject', $parameters);
$bodyHtml = $template->renderBlock('body_html', $parameters);
$bodyText = $template->renderBlock('body_text', $parameters);

stof · 2016-11-11T09:40:56Z

lib/Twig/Node/Expression/TemplateBlockReference.php

+    {
+        if ($this->getAttribute('output')) {
+            $compiler
+                ->addDebugInfo($this)


shouldn't this always be added ?

we don't add this for inlined code (except for context accesses)

stof · 2016-11-11T09:41:22Z

lib/Twig/Node/Expression/TemplateBlockReference.php

+            $compiler
+                ->raw('$this->env->load(')
+                ->subcompile($this->getNode('template'))
+                ->raw(')->displayBlock(')


should it really use display here ? I think it should be renderBlock

stof · 2016-11-11T09:42:14Z

lib/Twig/Template.php

@@ -650,8 +656,7 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
            throw $e;
        }

-        // useful when calling a template method from a template
-        // this is not supported but unfortunately heavily used in the Symfony profiler
+        // @deprecated in 1.28


should it throw a deprecation warning ?

Koc · 2016-11-11T15:45:00Z

+1 for public api
+0 for internal implementation

fabpot · 2016-11-11T15:46:11Z

@Koc What do you mean by "internal implementation" vs "public API"?

Koc · 2016-11-11T16:20:12Z

I mean that is useful for end twig user and solves my issue but I have nothing to say about implementation.

stof · 2016-11-11T18:18:26Z

lib/Twig/Node/Expression/TemplateBlockReference.php

+        if ($this->getAttribute('output')) {
+            $compiler
+                ->addDebugInfo($this)
+                ->write('$this->env->load(')


Technically, the compiled code could avoid using the Twig_TemplateWrapper layer, as you don't need the output buffer cleanup logic here (it will be cleaned by the rendering of the template using the function already when the exception bubbles to it). This will avoid this level of call stack (and of try/catch). And the Twig code can relies on the Twig internal API.

Btw, this is why rendering blocks directly in the form theming was safe regarding the output buffer: it was always called inside a Twig call cleaning the buffer (unless someone was doing a crazy thing to call the TwigFormRenderer from a different templating engine than Twig). The email rendering use case was the unsafe one, but this one will not rely on template_block

fabpot · 2016-11-12T17:05:58Z

I've extracted the second commit to #2245 and removed comments about it here.

stof · 2016-11-12T19:59:43Z

lib/Twig/TemplateWrapper.php

+ *
+ * @author Fabien Potencier <fabien@symfony.com>
+ */
+final class Twig_TemplateWrapper


Should we really call it a wrapper ? The fact that it is a wrapper around a Twig_Template object is also an implementation detail. I'm wondering whether we can find a better name here (I have no good idea though)

I would have loved to use Twig_Template, but that's not possible. So, I used a descriptive name instead. But I agree that the name is not ideal.

Is it possible to use Twig_Template for 2.0?

Changing the meaning of Twig_Template between 1.x and 2.x means it is very hard to write forward-compatible code. So -1 for this.

stof · 2016-11-12T19:59:59Z

lib/Twig/TemplateWrapper.php

+     */
+    public function getBlocks()
+    {
+        // FIXME


should be done, or should be removed

stof · 2016-11-12T20:00:26Z

lib/Twig/TemplateWrapper.php

+        return ob_get_clean();
+    }
+
+    public function getSourceContext()


please add the phpdoc for the return type, as this is the public API

…tion (fabpot) This PR was merged into the 1.x branch. Discussion ---------- added support for a custom template on the block() function `block()` allows one to render a block of the current template. This PR adds the possibility to use `block()` to render a block of another template: ```twig {{ block('footer', template_name) }} ``` As it uses the same logic as for the regular `block()`, you can also use it in a test: ```twig {% if block('footer', template_name) is defined %} ... {% endif %} ``` This removes the needs to use the internal `Twig_Template::renderBlock()` method in the Symfony Web Profiler for instance. And combined with #2236, it allows us to really mark the whole `Twig_Template` class as being internal. I'm aware that #1302 asked for being able to pass a context to `block()` (implemented in #1433), but I prefer not to in favor of adding the `with` tag (see #719 and #1054). When the `with` will be implemented, the code in the Symfony Web Profiler will become something along the lines of: ```twig {% with { 'collector': profile.getcollector(name), 'profiler_url': profiler_url, 'token': profile.token, 'name': name } %} {{ block('toolbar', template) }} {% endwith %} ``` Commits ------- 5ed59be added support for a custom template on the block() function

fabpot · 2016-11-13T03:17:22Z

This one is finished now (docs and tests added). If nobody has a better name than Twig_TemplateWrapper, I will merge this one as is.

…ortable way (fabpot) This PR was merged into the 1.x branch. Discussion ---------- Expose a way to access template data and methods in a portable way `Twig_Template` is an internal class, and most of its methods are marked as being `@internal` as they are not "safe" to use by end users. But some of its features are interesting like `renderBlock()` which is very useful when rendering emails for instance (see #1873). This PR tries to address both issues by marking the whole `Twig_Template` class as being internal and by exposing a new way to safely interact with a template besides the obvious `render()`/`display()` methods via `$twig->load("...")`. If also add some convenient methods like `hasBlock()` or `getBlocks()` (see ~#1831~ and #1882): ```php $template = $twig->load('index'); echo $template->render($context); $template->display($context); if ($template->hasBlock('name') { $template->displayBlock('name', $context); } foreach ($template->getBlocks() as $block) { echo $template->render($block, $context); } ``` Commits ------- d7062f3 exposed a way to access template data and methods in a portable way

SpacePossum · 2016-11-15T21:54:30Z

awesome, thanks!

This PR was merged into the 2.7 branch. Discussion ---------- bumped min version of Twig to 1.28 | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | see twigphp/Twig#2236 | License | MIT | Doc PR | n/a Commits ------- b8f7614 bumped min version of Twig to 1.28

gharlan · 2016-11-18T12:16:48Z

lib/Twig/Template.php

        if ($object instanceof Twig_TemplateInterface) {
+            @trigger_error('Using the dot notation on an instance of '.__CLASS.' is deprecated since version 1.28 and won\'t be supported anymore in 2.0.', E_USER_DEPRECATED);


typo: __CLASS -> __CLASS__

fabpot force-pushed the expose-render-block branch 3 times, most recently from 8d98933 to 827f22d Compare November 10, 2016 22:09

fabpot force-pushed the expose-render-block branch 4 times, most recently from 9d32db7 to 7b86197 Compare November 10, 2016 22:24

fabpot mentioned this pull request Nov 10, 2016

bumped min version of Twig to 1.28 symfony/symfony#20484

Merged

fabpot force-pushed the expose-render-block branch from 7b86197 to cf19660 Compare November 10, 2016 22:34

stof reviewed Nov 11, 2016

View reviewed changes

fabpot force-pushed the expose-render-block branch from 599a32e to dbfc800 Compare November 11, 2016 15:49

fabpot force-pushed the expose-render-block branch from dbfc800 to 8cba0b3 Compare November 11, 2016 16:33

stof reviewed Nov 11, 2016

View reviewed changes

fabpot force-pushed the expose-render-block branch 6 times, most recently from eca75e3 to 0e42931 Compare November 12, 2016 16:54

fabpot mentioned this pull request Nov 12, 2016

added support for a custom template on the block() function #2245

Merged

fabpot force-pushed the expose-render-block branch 3 times, most recently from 8d37844 to 5843bd6 Compare November 12, 2016 19:13

stof reviewed Nov 12, 2016

View reviewed changes

fabpot force-pushed the expose-render-block branch 5 times, most recently from 6f4e006 to a244308 Compare November 13, 2016 03:14

fabpot changed the title ~~[RFC][WIP] exposed a way to access template data and methods in a portable way~~ Expose a way to access template data and methods in a portable way Nov 13, 2016

fabpot force-pushed the expose-render-block branch from a244308 to 9ab1523 Compare November 13, 2016 03:20

exposed a way to access template data and methods in a portable way

d7062f3

fabpot force-pushed the expose-render-block branch from 9ab1523 to d7062f3 Compare November 13, 2016 03:45

fabpot merged commit d7062f3 into twigphp:1.x Nov 15, 2016

Koc mentioned this pull request Nov 16, 2016

[Mailer] Request for some mailer improvements Sylius/Sylius#6749

Closed

fabpot deleted the expose-render-block branch November 17, 2016 14:45

gharlan reviewed Nov 18, 2016

View reviewed changes

stof mentioned this pull request Dec 6, 2016

Add the possibility to render block in function #2283

Closed

Koc mentioned this pull request Apr 19, 2017

Missing documentation for block rendering #2450

Closed

acrobat mentioned this pull request May 21, 2019

[AllBundles] Replace internal loadTemplate twig method Kunstmaan/KunstmaanBundlesCMS#2466

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Expose a way to access template data and methods in a portable way #2236

Expose a way to access template data and methods in a portable way #2236

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

		if ($object instanceof Twig_TemplateInterface) {
		@trigger_error('Using the dot notation on an instance of '.__CLASS.' is deprecated since version 1.28 and won\'t be supported anymore in 2.0.', E_USER_DEPRECATED);

Uh oh!

Expose a way to access template data and methods in a portable way #2236

Expose a way to access template data and methods in a portable way #2236

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!