Add a simple Twig_RuntimeLoaderInterface implementation #2311
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I would not name it Btw, to avoid breaking the goal of the loader, a map of It would then end up as this: $twig->addRuntimeLoader(new Twig_RuntimeLoader(array(
// even better would be to change the way the engine and the csrf token manager are
// instantiated to keep them lazy as well, but it depends on how they are used elsewhere
// in the app
TwigRenderer::class => function() use($rendererEngine, $csrfTokenManager) {
return new TwigRenderer($rendererEngine, $csrfTokenManager);
},
)); |
stof
reviewed
Dec 22, 2016
| /** | ||
| * @author Robin Chalas <robin.chalas@gmail.com> | ||
| */ | ||
| class Twig_RuntimeLoader implements \Twig_RuntimeLoaderInterface |
There was a problem hiding this comment.
the \ must be removed, as Twig 1.x still supports PHP 5.2
| return $this->mapping[$class]; | ||
| } | ||
|
|
||
| throw new \InvalidArgumentException(sprintf('Class "%s" is not mapped as Twig runtime.', $class)); |
There was a problem hiding this comment.
It must return null when it does not support the class according to the interface
4edc031 to
02674bf
Compare
xabbuh
reviewed
Dec 22, 2016
| return $runtime(); | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
IIRC the return statement can then simply be omitted to match the Twig code style rules.
xabbuh
reviewed
Dec 22, 2016
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
| class Twig_Tests_RuntimeLoaderTest extends PHPUnit_Framework_TestCase |
There was a problem hiding this comment.
missing blank line before the class declaration
a32a2d6 to
34c9680
Compare
3bef5b7 to
a369587
Compare
|
Changed to |
a369587 to
5b77e44
Compare
5b77e44 to
91c8d59
Compare
fabpot
reviewed
Dec 23, 2016
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ |
|
Thank you @chalasr. |
fabpot
added a commit
that referenced
this pull request
Dec 23, 2016
… (chalasr) This PR was merged into the 1.x branch. Discussion ---------- Add a simple Twig_RuntimeLoaderInterface implementation Next to symfony/symfony#21023 This is related to the BC break reported in symfony/symfony#21008 which has been introduced in symfony/symfony#20093 when decoupling extensions from definitions. What I propose here is to ease the upgrade to symfony 3.2+ by adding a simple `Twig_RuntimeLoaderInterface` implementation here, useful only when using the twig-bridge outside of the symfony fullstack framework (with the Form component for instance). Upgrading would be as simple as: ```diff $twig = new Twig_Environment(...); $rendererEngine = new TwigRendererEngine(array('form_div_layout.html.twig'), $twig); - $twig->addExtension(new FormExtension(new TwigRenderer($rendererEngine, $csrfTokenManager))); + $twig->addExtension(new FormExtension()); + $twig->addRuntimeLoader(new Twig_RuntimeLoader(array(TwigRenderer::class => new TwigRenderer($rendererEngine, $csrfTokenManager))); ``` Instead of having to write this runtime loader yourself. Please see symfony/symfony#21008 for details and a concrete example of how this could help. Commits ------- 91c8d59 Add a Twig_FactoryRuntimeLoader
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Next to symfony/symfony#21023
This is related to the BC break reported in symfony/symfony#21008 which has been introduced in symfony/symfony#20093 when decoupling extensions from definitions.
What I propose here is to ease the upgrade to symfony 3.2+ by adding a simple
Twig_RuntimeLoaderInterfaceimplementation here, useful only when using the twig-bridge outside of the symfony fullstack framework (with the Form component for instance).Upgrading would be as simple as:
$twig = new Twig_Environment(...); $rendererEngine = new TwigRendererEngine(array('form_div_layout.html.twig'), $twig); - $twig->addExtension(new FormExtension(new TwigRenderer($rendererEngine, $csrfTokenManager))); + $twig->addExtension(new FormExtension()); + $twig->addRuntimeLoader(new Twig_RuntimeLoader(array(TwigRenderer::class => new TwigRenderer($rendererEngine, $csrfTokenManager)));Instead of having to write this runtime loader yourself.
Please see symfony/symfony#21008 for details and a concrete example of how this could help.