[TwigBundle] Improve the overriding of bundle templates #24264

yceruto · 2017-09-19T18:44:41Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#17557
License	MIT
Doc PR	-

Overriding a Template that also extends itself

Now that bundles inheritance is deprecated and removed (#24160, #24161), I'm wondering if we can solve this old issue defining an exclusive namespace only for root bundles in 3.4 just bundles in 4.0:

twig:
    paths:
        # adding paths behind the scene into TwigExtension
        app/Resources/FooBundle/views: Foo
        vendor/acme/foo-bundle/Resources/views: Foo
        vendor/acme/foo-bundle/Resources/views: !Foo # exclusive

Thus, one can decide when use the exclusive namespace to avoid the issue and then we could say also:

To override the bundle template partially (which contains block) creates a new index.html.twig template in app/Resources/AcmeBlogBundle/views/Blog/index.html.twig and extends from @!AcmeBlogBundle/Blog/index.html.twig to customize the bundle template:

{# app/Resources/FooBundle/views/layout.html.twig #}

{# this does not work: circular reference to itself #}
{% extends '@Foo/layout.html.twig' %}

{# this will work: load bundle layout template #}
{% extends '@!Foo/layout.html.twig' %}

{% block title 'New title' %}

I hear other suggestions about the excluse namespace.

We will need to update http://symfony.com/doc/current/templating.html#referencing-templates-in-a-bundle too to add this convention.

WDYT?

yceruto · 2017-09-25T13:31:12Z

I just updated the description to clarify the proposal, it's not about Bundle suffix but bundle class name instead. So, you have a bundle named AcmeBlogBundle? you will have two namespaces for their templates: @AcmeBlog and @AcmeBlogBundle. The last one is specially useful when you're overriding a template that also extends itself, without configure twig.paths manually.

At the same time, you can from a controller or anywhere to load directly the template of the bundle or the overridden one in your app, without create extra templates or extra configuration to achieve it.

yceruto · 2017-09-27T02:28:44Z

Feat. freeze is coming :) also we can use some preffix or suffix to achieve it, WDYT?

ping @fabpot, @javiereguiluz

javiereguiluz · 2017-09-27T06:32:20Z

I don't dislike the idea ... but I'm not sold on it either. My concern is that you'll have two almost identical namespaces that are completely different: @FOSUser, @FOSUserBundle ... which one should I use? I already have a lot of problems remembering if the namespace contains the Bundle suffix or not. If I see @TwigBundle/... in a template, I don't think I'll remember that it refers to the original template path of the parent bundle.

yceruto · 2017-09-27T10:51:09Z

Other proposal that is easy to remember?

!FOSUser
FOSUser!
FOSUserSource
FOSUserOriginal
FOSUserParent
FOSUserBundle

This namespace should be used only to avoid the circular reference.

xabbuh · 2017-09-27T11:07:37Z

I share @javiereguiluz' concerns though I also do not have a better proposal.

javiereguiluz · 2017-09-27T11:15:56Z

Some random proposals:

{% extends 'parent@Foo/layout.html.twig' %}

{% extends '@@Foo/layout.html.twig' %}

{% extends '@Foo/layout.html.twig', { parent: true } %}

yceruto · 2017-09-27T11:34:32Z

@javiereguiluz only the second one @@Foo can be possible right now as valid Twig namespace, I'll take your proposal, thanks.

yceruto · 2017-09-27T12:15:24Z

It's not intuitive anyway, so we need document it.

ogizanagi

but my own preference would actually be

{% extends '@!Foo/layout.html.twig' %}

xabbuh · 2017-09-28T17:56:14Z

@ogizanagi's suggestion sounds good to me too.

yceruto · 2017-09-28T17:58:54Z

Perfect! I like this too, changed to ! preffix :)

fabpot · 2017-09-29T00:47:48Z

Thank you @yceruto.

…s (yceruto) This PR was merged into the 3.4 branch. Discussion ---------- [TwigBundle] Improve the overriding of bundle templates | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17557 | License | MIT | Doc PR | - ### [Overriding a Template that also extends itself](https://twig.symfony.com/doc/2.x/recipes.html#overriding-a-template-that-also-extends-itself) Now that bundles inheritance is deprecated and removed (#24160, #24161), I'm wondering if we can solve this old issue defining an exclusive namespace only for root bundles in `3.4` just bundles in `4.0`: ```yaml twig: paths: # adding paths behind the scene into TwigExtension app/Resources/FooBundle/views: Foo vendor/acme/foo-bundle/Resources/views: Foo vendor/acme/foo-bundle/Resources/views: !Foo # exclusive ``` Thus, one can decide when use the exclusive namespace to avoid the issue and then [we could to say also](http://symfony.com/doc/current/templating/overriding.html): > To override the bundle template partially (which contains `block`) creates a new `index.html.twig` template in `app/Resources/AcmeBlogBundle/views/Blog/index.html.twig` and extends from `@!AcmeBlogBundle/Blog/index.html.twig` to customize the bundle template: ```twig {# app/Resources/FooBundle/views/layout.html.twig #} {# this does not work: circular reference to itself #} {% extends '@Foo/layout.html.twig' %} {# this will work: load bundle layout template #} {% extends '@!Foo/layout.html.twig' %} {% block title 'New title' %} ``` I hear other suggestions about the excluse namespace. We will need to update http://symfony.com/doc/current/templating.html#referencing-templates-in-a-bundle too to add this convention. WDYT? Commits ------- 0a658c6 Add exclusive Twig namespace for bundles path

nicolas-grekas · 2017-09-29T09:47:57Z

@yceruto or anyone else with some free time: I broke tests when merging this into master, could you have a look please?

yceruto · 2017-09-29T10:56:19Z

@nicolas-grekas Still broken?

nicolas-grekas · 2017-09-29T11:01:48Z

yes

yceruto · 2017-09-29T11:15:51Z

The code in master look good to me:

symfony/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php

Lines 118 to 121 in f46ab8e

    
           if ($paths) { 
        
               // the last path must be the bundle views directory 
        
               $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($path, '!'.$namespace)); 
        
           }

and Travis pass (at least for TwigBundle):

taken from https://travis-ci.org/symfony/symfony/builds/281245817

nicolas-grekas · 2017-09-29T11:17:42Z

Oh nice sorry, I didn't break it because if this PR, but another one, NVM :)

carsonbot added Status: Needs Review TwigBundle Feature labels Sep 19, 2017

yceruto mentioned this pull request Sep 19, 2017

Improve the overriding of bundle templates #17557

Closed

nicolas-grekas added this to the 3.4 milestone Sep 20, 2017

yceruto force-pushed the template_inheritance branch 3 times, most recently from a1fed5b to 32010f8 Compare September 20, 2017 11:17

xabbuh approved these changes Sep 27, 2017

View reviewed changes

ogizanagi approved these changes Sep 28, 2017

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Sep 28, 2017

yceruto force-pushed the template_inheritance branch from 52ddda0 to 0a658c6 Compare September 28, 2017 18:35

Add exclusive Twig namespace for bundles path

0a658c6

Tobion approved these changes Sep 29, 2017

View reviewed changes

fabpot approved these changes Sep 29, 2017

View reviewed changes

fabpot merged commit 0a658c6 into symfony:3.4 Sep 29, 2017

yceruto deleted the template_inheritance branch September 29, 2017 01:34

yceruto mentioned this pull request Sep 30, 2017

Document improvement of the overriding of bundle templates symfony/symfony-docs#8440

Closed

Guite mentioned this pull request Oct 9, 2017

Symfony 4 and 5 Guite/MostGenerator#1068

Closed

Haehnchen mentioned this pull request Oct 9, 2017

[3.4] Improved the overriding of templates Haehnchen/idea-php-symfony2-plugin#1043

Open

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

Guite mentioned this pull request Dec 1, 2017

Symfony 3.4 Guite/MostGenerator#1171

Closed

17 tasks

mikemix mentioned this pull request Jun 26, 2018

Improve documentation about extending templates Sylius/Sylius#9526

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TwigBundle] Improve the overriding of bundle templates #24264

[TwigBundle] Improve the overriding of bundle templates #24264

yceruto commented Sep 19, 2017 •

edited

Loading

yceruto commented Sep 25, 2017 •

edited

Loading

yceruto commented Sep 27, 2017

javiereguiluz commented Sep 27, 2017

yceruto commented Sep 27, 2017 •

edited

Loading

xabbuh commented Sep 27, 2017

javiereguiluz commented Sep 27, 2017

yceruto commented Sep 27, 2017

yceruto commented Sep 27, 2017 •

edited

Loading

ogizanagi left a comment

xabbuh commented Sep 28, 2017

yceruto commented Sep 28, 2017 •

edited

Loading

fabpot commented Sep 29, 2017

nicolas-grekas commented Sep 29, 2017 •

edited

Loading

yceruto commented Sep 29, 2017

nicolas-grekas commented Sep 29, 2017

yceruto commented Sep 29, 2017

nicolas-grekas commented Sep 29, 2017

[TwigBundle] Improve the overriding of bundle templates #24264

[TwigBundle] Improve the overriding of bundle templates #24264

Conversation

yceruto commented Sep 19, 2017 • edited Loading

Overriding a Template that also extends itself

yceruto commented Sep 25, 2017 • edited Loading

yceruto commented Sep 27, 2017

javiereguiluz commented Sep 27, 2017

yceruto commented Sep 27, 2017 • edited Loading

xabbuh commented Sep 27, 2017

javiereguiluz commented Sep 27, 2017

yceruto commented Sep 27, 2017

yceruto commented Sep 27, 2017 • edited Loading

ogizanagi left a comment

Choose a reason for hiding this comment

xabbuh commented Sep 28, 2017

yceruto commented Sep 28, 2017 • edited Loading

fabpot commented Sep 29, 2017

nicolas-grekas commented Sep 29, 2017 • edited Loading

yceruto commented Sep 29, 2017

nicolas-grekas commented Sep 29, 2017

yceruto commented Sep 29, 2017

nicolas-grekas commented Sep 29, 2017

yceruto commented Sep 19, 2017 •

edited

Loading

yceruto commented Sep 25, 2017 •

edited

Loading

yceruto commented Sep 27, 2017 •

edited

Loading

yceruto commented Sep 27, 2017 •

edited

Loading

yceruto commented Sep 28, 2017 •

edited

Loading

nicolas-grekas commented Sep 29, 2017 •

edited

Loading