Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace theming engine with Twig #83

Closed
damienmckenna opened this issue Sep 17, 2013 · 25 comments
Closed

Replace theming engine with Twig #83

damienmckenna opened this issue Sep 17, 2013 · 25 comments

Comments

@damienmckenna
Copy link

@damienmckenna damienmckenna commented Sep 17, 2013

I don't think I need to list the reasons why Twig should be used instead of PHPTemplate, given Jen was one of the Twig project leads. If Backport intends to provide any sort of gateway between D7 and D8, as Jen suggested in a recent blog post, then it really needs to change over to using the Twig template engine instead of hacking up the D7 theme system.

@xiongxin
Copy link

@xiongxin xiongxin commented Sep 17, 2013

+1

@kreynen
Copy link

@kreynen kreynen commented Sep 17, 2013

Correct me if I'm wrong, but wasn't part of the motivation for moving to Twig simply that Twig was packaged w/ Symfony? If Backdrop isn't including Symfony, is it worth moving to Twig vs. giving themers/theme developers already familiar with PHPTemplate in D6 and D7 the same option as developers looking to migrate to a framework/community that is more like what they are already familiar with that Drupal 8.

If the Backdrop team is going to consider moving away from PHPTemplate, please do some performance testing comparing Twig w/ Smarty. Very similar markup, but in most testing of CMS related tasks Smarty is significantly faster (though often using more memory) than Twig.

http://umumble.com/blogs/php/249/
https://github.com/jbroadway/template-bench

These tests showed Twig being faster than Smarty, but test was pretty use case specific and even the tester had some issues with his own approach to testing...

http://sapphirepaw.blogspot.com/2013/08/smarty-vs-twig-part-1.html

@damienmckenna
Copy link
Author

@damienmckenna damienmckenna commented Sep 17, 2013

kreynen: a) you are incorrect, Twig wasn't picked just because it's packaged with Symfony; b) Twig is one of the best templating engines currently available today on PHP; c) it's by the same people as Symfony but is not packaged with it, rather it's a completely separate thing; d) there's zero point in adding a third option, that just further pushes Backport into its own thing thus divorcing it even further from both D7 and D8, which is against one of the goals Jen mentioned in her blog post.

@quicksketch
Copy link
Member

@quicksketch quicksketch commented Sep 17, 2013

there's zero point in adding a third option, that just further pushes Backport into its own thing thus divorcing it even further from both D7 and D8
There might be better options out there (it's largely a matter of taste), but being an easy transition from D6/D7 is the most important goal of the project. Sort of like the DBTNG issue, adding a template engine could help with learning Backdrop, but at the cost of increasing the difficulty of the upgrade path.

I'll let @jenlampton and @c4rl express themselves here, but from hearing the progress of Twig throughout the D8 cycle, I think the takeaway was that Twig helped with the theming situation, but the biggest improvements came from theme cleanup (consistent use of templates) rather than from Twig itself.

Performance-wise, there are a huge number of benchmarks at https://drupal.org/node/1987510. In general, it looks like CPU-time (cu) and Wall-time (wt) in those benchmarks indicate Twig probably caused a 1-5% performance regression. I think they got it down towards the lower end (around 1.5%) by the time the first patch was committed.

@pwFoo
Copy link

@pwFoo pwFoo commented Sep 18, 2013

I think phptemplate should be available. Maybe twig could be added as alternative engine? So people can choose the prefered template engine...

@iPublicis
Copy link

@iPublicis iPublicis commented Sep 18, 2013

Once upon a time Drupal had several theme engines (https://drupal.org/project/theme%20engines).

If the plan is to be in the middle between D7 and D8 I think keeping at least PHPTemplate and add Twig will be a good ideia. Most pre-made themes are PHPTemplate ;)

@tedbow
Copy link

@tedbow tedbow commented Sep 19, 2013

Isn't part of the problem of having multiple theming engines is that it then requires contrib developers to provide templates(which are now tpl files) in multiple formats?

@renebakx
Copy link

@renebakx renebakx commented Sep 20, 2013

Can i toot my own horn here? I have a rather feature complete version of twig for drupal 7 (TFD) on the shelf, that would fit very nicely into backdrop. (https://drupal.org/sandbox/ReneB/1075966) It comes with a psr-0 compatible APC aware autoloader. The only thing is, are you guys willing to convert all .tpl.php files to tpl.twig again?

BTW, TFD does not solve the render array of doom issues, it only comes with a few convenience functions to make life a bit easier. Think in not having to state {{ field.0.value}} but simply {{field}} etc.
It has auto-render, so you can do things like {{header}} instead of having to do {{ render(header)}}

I'am willing to dedicate some of my time to help you guys with a more clean and native implementation given that it will only involve adapting my current code base, and not a rewrite like D8 is needed. Just let me know :)

Oh and quicksketch, twig is not about performance, it's about offering a DDL to themers, easy horizontal and vertical extending of templates without creating piles of PHP code, and thus preventing big piles of PHP logic in templates.

@LionsAd
Copy link

@LionsAd LionsAd commented Sep 21, 2013

Fwiw, it should be petty simple to port the engine part of twig in d8 to back drop.

@ericfg
Copy link

@ericfg ericfg commented Sep 21, 2013

I guess this depends on what the real goal of this project is. If we are striving to fill a gap that drupal development is creating, why is there an advantage to expiring everyone's knowledge of how to do theme work by making the jump to twig?
are we just building a new Drupal with all the same problems and pattern of leaving knowledge in the dumpster every 2 years? are we building a fork of drupal that addresses the concerns that have been debated in the drupal community for at least 8 if not more years (focus on the majority of tiny sites -- the long tail-- and not the few huge players that can afford to hire new kids every 2 years)

@renebakx
Copy link

@renebakx renebakx commented Sep 22, 2013

Oh no... not that discussion again. We had the same discussion in D8, and it is and endless one :)

I will leave it upon the project maintainers to decide if TWIG needs to be ported into backdrop or not. Or perhaps create another fork 'BackTwigged' ;)

But seriously, porting the D8 version or mine makes not a big difference, instead mine has some different syntactic sugar pored over it to make it fit into a D7 workflow. Like not need to convert ALL templates to twig in a project, just create to ones needed for a project.

@LionsAd
Copy link

@LionsAd LionsAd commented Sep 22, 2013

The D8 engine also does not need to be all converted ... But whatever works best! :-)

@renebakx
Copy link

@renebakx renebakx commented Sep 22, 2013

Must admit, not really looked upon the twig.engine file in D8. But I know I had to add a few bit and pieces to the loader to get everything running smoothly in D7 :) Guess the same hurdles were taken with D8

But indeed whatever works best, most syntactic sugar is done in in the Extensions, and those can are made against the twig interface so can be ported to whatever version of the engine is used.

I still opt-in for having both engine available in backdrop core. PHP-Template needs no work at all, and having a twig.engine in core can make the transition easier. Is that a valid option? It is not going to bloat the core that much, especially if the installation of twig is managed by composer :)

@ericfg
Copy link

@ericfg ericfg commented Sep 22, 2013

I'd imagine that having both would be good; would allow existing d7 themes to work without any effort; allow people to continue to work on their projects while they learn twig; allow those already familiar with twig to use what seems like a more advanced templating engine.
In general I like the idea of twig -- using an existing template system and not one created just for backdrop or drupal. On the other hand, having to learn what amounts to yet another programming language and syntax (no matter how close it mimics php) seems like a barrier to entry for many

@duckzland
Copy link

@duckzland duckzland commented Sep 29, 2013

whats the point on forking if the final product will be just D8 with bit difference? If the goal of backdrop is to minimize barrier entry for people who are new to PHP then twig is not the right options simply because there is more people who understand phptemplate than twig regardless on how "cool" it is.

@davidhwang
Copy link

@davidhwang davidhwang commented Oct 1, 2013

there is more people who understand phptemplate than twig

I'm not sure that is true -- phptemplate is a Drupalism created originally for D4.7 -- given Twig uses syntax similar to Handlebars or Smarty or Moustache any other number of project-agnostic curly-brace templating systems, I'd argue the number of people who can readily understand Twig is substantially higher than those familiar with a fairly non-obvious Drupal-specific system.

@quicksketch
Copy link
Member

@quicksketch quicksketch commented Oct 1, 2013

As Drupal 8 has demonstrated, there's really no ability to support more than one theming engine. While it's possible include more than one engine, the practicality of the situation is that whatever template files modules ship with is the only engine people will actually use. Unless you're going to require that all templates are written twice (once in PHPTemplate and once in Twig), Drupal/Backdrop really only supports one theme engine.

At this point I'm likely to agree with those who have stated that converting to Twig means a huge conversion task. It's taken an army of contributors to convert D8 to Twig, and I'm not sure we'll have those kind of resources available. Converting to Twig also means increasing the porting job from D6/7 (if we ship with only the Twig engine) or putting a burden on users needing two engines at once (if core ships with Twig but contrib is allowed to use PHPTemplate).

So overall, I'd say we should put Twig on the back-burner and focus on goals from the manifesto: easing porting, providing end-user features, increasing performance, etc. It can be argued either way on the "learnability" goal, but considering the cost associated with converting, I'm not sure we can afford it.

We can still focus on many of the other goals that Twig brought attention to however: reducing duplicate templates, more suggestions and code-reuse, simplified implementations, reduction of theme functions, etc. There's a lot of theming cleanup that was done in the name of Twig, but that wasn't actually dependent upon it at all. Reducing use of drupal_render() in themes and giant arrays of doom and going back to nicely preprocessed string-only variables can also help clean up our templates and require less Drupal-specific knowledge.

@luiselizondo
Copy link

@luiselizondo luiselizondo commented Oct 1, 2013

+1 for not including Twig in v1.0

@yann-yinn
Copy link

@yann-yinn yann-yinn commented Oct 5, 2013

Twig makes perfectly sense in symfony 2 because it provides a way to handle layout with blocks, blocks overrides, parent and children templates etc... Things you actually need using a naked framework.
1 - Drupal has already all those things, implemented in its own way.
2 - As said, port all templates from a drupal 6 or Drupal 7 from tpl.php to .twig will be part of the pain when trying to upgrade sites to D8 (is somebody actually planning to try that ? i will not)
3 - Php IS a nice template engine for html, using good practices. :-)

@renebakx
Copy link

@renebakx renebakx commented Oct 12, 2013

alrighty then, unsubscribing from this issue. Whenever backdrop is ready to re-take a look at view/logic abstraction, you guys know where to find a code base that would fit seamlessly into Drupal 7 and thus should fit into drupal 7 1/2 aka backdrop :D

@jenlampton
Copy link
Member

@jenlampton jenlampton commented Nov 21, 2013

For the record, I do want Twig in Backdrop. That said, the whole reason Backdrop exists is because things changed too much in Drupal 8. With that in mind, we need to weigh every improvement against the cost of change, and choose wisely.

There is a lot we can do to improve the theme layer without changing the theme engine (in fact, most of what we are doing in D8 has nothing to do with the change to Twig). We can use most of the same logic in template files weather it's Twig or PHPtemplate, the only real difference is syntax.

We'll still be separating presentation logic from backend logic, it just won't be done in different languages. :)

We have a lot of other things to work on in the 1.0 version of backdrop, so I'm marking this issue as postponed for now, but I think it needs to happen in Backdrop 2.0.

@matt2000
Copy link
Member

@matt2000 matt2000 commented Nov 21, 2013

FWIW, my perspective on this is to leverage the performance gains of Backdrop to use it as a practical REST server and do most presentation logic with a client side MVx framework like Angular or Knockout. (Currently doing similar with D7, but running into performance issues with Views (not) caching that my require some hacks to View or Core.)

@klonos
Copy link
Member

@klonos klonos commented Dec 15, 2014

A year+ after...

I've been following the weekly meetings and one thing being repeated as an intro by @jenlampton in every single one is the "cost of change" of upgrading D7 sites to D8 vs Backdrop. This issue here (unless renamed/re-targeted to "support" Twig instead of replacing PHPEngine entirely) is contradictory to that goal. In fact, if PHPEngine is replaced by Twig in 2.x, then all I see is a complete 180o turn to our promise to our target audience and that'd be a very "sneaky" thing IMO.

Can we please get an official clarification here on what is the intention after all, because to me having to change every theme to Twig does translate to cost for end users.

@jenlampton
Copy link
Member

@jenlampton jenlampton commented Dec 18, 2014

@klonos we are not currently planning on replacing the theme engine with Twig. What we are doing instead is learning from what's done in twig templates and doing the same thing - in PHP - without the layer of abstraction.

I still believe that moving to Twig was the right thing for Drupal. It was in line with their principles and got the front-end community excited about using Drupal 8.

As you mentioned, moving to Twig is not inline with Backdrop's principles. It's off the table for 1.x.

If it turns out that switching to Twig brings throngs of front-end developers to Drupal and is a serious game changer in the CMS space, we can re-evaluate for Backdrop 2.0 (but it will need careful consideration, benefits need to be carefully weighed against the cost of change)

@quicksketch
Copy link
Member

@quicksketch quicksketch commented Dec 18, 2014

Let's go ahead and close this. Unless new conditions come up that would make this more appealing (not sure what those would be) Twig won't be more suitable for Backdrop in 2.x than it was for 1.x. In the mean time, we'll have the opportunity to watch the adoption of Twig in both D8 and the wider PHP community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.