magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.53k stars 9.31k forks source link

Decide for one templating system (phtml or Twig) #370

Closed mzeis closed 11 years ago

mzeis commented 11 years ago

As suggested by @Flyingmana I moved the discussion from #107 into a new issue.

In the mentioned issue, @sshymko said:

Most probably all existing phtml templates are going to be kept, no Twig templates are going to be provided out of the box. However, ability to create your own Twig templates is already there.

To which I responded:

Please stick with one solution: either phtml or Twig. I believe it will do more harm than good if you split up the community into one half phtml, one half Twig. More choices isn't always good.

I'm afraid "good" developers would use Twig, "bad" developers would use phtml. You will get a wild mix of two different templating systems.

Some reactions from others in the same issue:

@SchumacherFM:

Twig: +1 for @mzeis otherwise you will see in the phtml again a lot of business logic. it is too alluring to use incorrect and inperformant PHP patterns in the templates ...

@michaelgrosser:

Agreed with the above. A mix of template patterns is going to lead to a lot of the sloppy code and sloppy extension problems that exist in the Magento ecosystem (mainly 3rd party plugins) today. If the decision is to embrace Twig, it should be embraced across the board and not selectively.

@benmarks:

+1 for standardizing on one template idiom. Swappable template types could possibly be a "nice-to-have", but I'm more than happy to have Twig be the One True Template Type®.

@alex-m:

+1 for Twig, it's the only template format with logic that can be parsed with both JS and PHP that I've come accross

@philwinkle:

:+1: for standardization. :-1: for Twig. There are enough things to teach with regard to Magento; another idiom, templating, or meta-language isn't one I care to have to learn or instruct.

Flyingmana commented 11 years ago

thank you :)

two cons from me:

  1. twig does not prevent module creators from writing bad Templates. It makes it only harder by forcing them, to write a service/filter/pipe first, which makes the logic in templates even harder to read by hiding it. Also jumping to Models,Classes and so on is way harder to implement.
  2. Twig has an own LayoutBlock princip, in the symfony world it is not uncommon, that single templates change the content of multiple Blocks. This would affect very strong the current magento Layout system if implemented correct. Else we could only ignore the possibility, forbid it, or be ok with 2 co-existing layout systems.

I personally prefer mustache, as it is language independent and is easy usable via javascript and a big pro for ajax applications. But still Iam for keeping phtml as main template language. It keeps the entry level low for new developers and its easier to read when good written.

any other template language is already easy to implement by adding a simple block class.

michaelgrosser commented 11 years ago

I think my biggest reason for supporting a move to Twig or something similar is that I'm fairly tired of seeing code with Mage::getModel('some/thing')->getCollection() done straight in a phtml file. Twig would at least prevent that. While I agree that it doesn't safeguard against all sloppy coding, the barrier to entry of having to go hack up a service class is much higher. Typically, I'd say this is high enough to prevent a lazy developer from going rouge and doing things in the template that don't belong there for the sake of expedience.

Troubleshooting becomes much more difficult when you have to start hunting for model requests and collection builds in phtml files versus at least knowing the code is in a defined place in which it belongs, even if it is sloppy.

rgranadino commented 11 years ago

+1 for twig (or anything to get away from .phtml files).

clockworkgeek commented 11 years ago

I'm fairly tired of seeing code with Mage::getModel('some/thing')->getCollection() done straight in a phtml file.

I'm tired of seeing any imperative code in templates, it should be entirely declarative like XSLT because HTML is also declarative. Symphony (not Symfony) does this and is arguably more future-safe for it.

airbone42 commented 11 years ago

+1 for twig to reduce possibility of xss attacks on the view layer.

duffybelfield commented 11 years ago

+1 for standardisation and +1 for Twig

My only concern is that Twig being more complicated will encourage more "hack and fudge" development ;)

barbazul commented 11 years ago

+1 for having a single templating system -1 for Twig.

I don't care to learn or teach a new language. Pure PHP does the job better than any templating language out there.

Though I am also tired of seeing crappy code in templates, its a problem that has a simple fix which users of Git have come to know as "hooks". Simply reject any commits with Mage::getModel or similar inside of a of a phtml file. Of course, that would reject a lot of Magento built in templates (price.phtml I'm looking at you) but "good" programmers don't really need to keep those under version control, right?

The truth is that "having the ability" to write crappy quick hacky code into templates is actually an asset that has allowed me to code proof-of-concept prototypes that I can show my customer for him to decide whether to go forward with one funcionality or another in a very easy quick way.

In my company, we have NEVER used a community module's templates as provided. Having two template engines would mean that every time I download a new community module, it would be a flip of a coin whether it uses the same engine as the rest of my codebase or the other (thus making it useless or less than simple to implement). Then the dicussion will turn to "should all modules provide both alternatives?", which will basically be moving the problems I am seeing as a Magento solution agency to the Magento module agencies, but not really taking away the issue.

Now, if there are REALLY good reasons to move forward with Twig (or whatever templating engine) then that should be the only option available and the whole of Magento templates should be changed. The way I see it, going half the way (adding support but not endorsing it from the core) could have a negative impact in the Magento developer comunity as a whole

joshdw1 commented 11 years ago

Please only support one template system, allowing multiple will lead to unnecessary confusion and division. I'm not too concerned about which template system, my personal preference would be Twig, mostly for the reasons already mentioned.

centerax commented 11 years ago

+1 for Twig, and for only ONE option available.

Thanks

kalenjordan commented 11 years ago

:thumbsup: for standardization and :thumbsup: for twig.

The purpose of Magento 2 is to modernize the stack without as much burden for backwards compatibility. Yes there will be a little bit of a learning curve, but much of the PHP community has already moved to using twig.

One of the things I like a lot about it is the way that you can do inheritance. I think in some ways it has a better block / layout system than Magento's native one.

artlantis commented 11 years ago

I think many of developers are in this topic have more free time to learn something new. What is your pros and cons about phtml? Nobody explain of that. As you might guess, Magento's herself complex system and many developers are spent so much time learn. My choice, leave as it is.

clockworkgeek commented 11 years ago

much of the PHP community has already moved to using twig

[citation needed]

mertgokceimam commented 11 years ago

Having 2 different template Will only cause confusion on long term.

I also support moving forward with twig only

Sent from my iPhone

On 3 Eki 2013, at 20:56, "clockworkgeek" notifications@github.com<mailto:notifications@github.com> wrote:

much of the PHP community has already moved to using twig

[citation neededhttp://xkcd.com/285/]

Reply to this email directly or view it on GitHubhttps://github.com/magento/magento2/issues/370#issuecomment-25642956.

kalenjordan commented 11 years ago

@clockworkgeek Well played :) Primarily I had symfony in mind when saying that.

sylvainraye commented 11 years ago

@clockworkgeek +1 you made my day :-)

+1 for just one template engine standard. Keep it simple stupid. Prevent conflict, misunderstanding and integration problems.

If the reason to change the template engine is to prevent bad practices, bad news, do not use PHP! It's full of it.

Twig is mainly used by the Symfony community, it's a good engine and prevent adding PHP code into X/HTML templates. However it doesn't prevent to put all HTML code in a twig variable!!! You can still have bad practices.

Phtml template have already been used by Magento 1.x, and the doc to customize it is widely spread. As it was said, I totally agree to prevent adding complexity to the complexity. Magento has enough complex design patterns which load the charge of work to not add a new layer of difficulty.

Personally, I don't see why change. Even if I like Twig and use it for Symfony projects, it's a solution for new projects, not for an existing one and so established as Magento.

The fact is, it will add more cost to deliver a Magento project. As least, during the first year(s) of existence.

dimasdwika commented 11 years ago

+1 for having one standard template engine and I prefer to use twig since template files should be simple and easy syntax. But having template files in PHP, make bad developer doing more things in template level.

Also, if the worry is jumping around classes in Twig. Twig still be able to do it. It's just need to do it in proper way.

ScreamingDev commented 11 years ago

-zillion for having a template engine in a template engine in a template engine. Did u read @barbazul before commenting? This thread is like "Oh, I know Twig, I vote for it."...

You can have proper, clean, flexible and maintainable PHP but you like to choose another overhead and another instance over performance and KISS? I can count the amount of people that like to change magento core / templates even without having hands (for those who still noodle over it: it's zero).

Go back to the roots and think about it very carefully. Just like knowing and loving Twig is not enough to make it a standard. Enumerate pros that PHP already have is neither a reason to decide. Did you already define some in front standards? Is it Push- or Pull- between the Model and the View? Just realized the waste of time for this discussion, because there are no upstream standards. Old problem: Talking about the flowers on the balcony while the foundation of the house is slobber.

Gimme one good reason why Twig. That some developer of extensions or script kiddies can't behave and messed up phtml is not an option. How difficult do you like to make Magento anyway?

{% endrage %}

ajbonner commented 11 years ago

Summary: +1 for Standardisation -1 for Twig.

I love Twig, but I'm not entirely comfortable with the idea of it being a default.

My main reservation about Twig, is that it is a current fashion. So it's far from impossible that a newer, trendier, templating language could spring up in a few years.

Just think if Magento had used Smarty as its templating engine way back in 2007? That would feel /very/ gross now. I feel a phtml engine for all its drawbacks (and there are many), is at least flexible in the face of future changes to best-practice and within the PHP language itself.

Another parallel that comes to mind is that of Prototype vs JQuery. When Magento was being developed the js/dom abstraction layer wars were not resolved. And while it's arguable, even today, that Prototype is technically superior, JQuery won the war for developer and designer mindshare. So we see now JQuery jerry-rigged into Magento, but it's not a comfortable fit and it is far from a first class citizen. I fear something potentially similar happening with templating if we went down the Twig route.

Allowing developers to plugin their own templating engines would be nice. But I feel only a single engine should be officially distributed and supported out of the box.

kalenjordan commented 11 years ago

Invoking smarty...well played, @ajbonner . I think you sold me.

@sourcerer-mike, pretty sure you're only allowed to minus a maximum of one at a time :smile: - yes I read @barbazul 's comment. For me, preventing people from doing bad stuff isn't the primary motivation behind wanting Twig. Having the ability to writer cleaner templates is.

fabpot commented 11 years ago

Twig is not only used by Symfony, but by many other big Open-Source projects like Drupal, phpBB, eZPublish; projects from the Magento community like OroCRM and Akeneo; and many other smaller ones like Piwik, doclear, Craft, ...

Just my 2cents ;-)

akira28 commented 11 years ago

+1 to Twig reinventing the wheel (or using a worn one) is dangerous :)

QWp6t commented 11 years ago

Magento 1x prided itself on being some sort of web designer's godsend ... assuming one took the time to overcome the (often steep) learning curve. In reality, it was a nightmare for beginners, and even for experienced developers, Magento's theme engine is mediocre at best. It's confusing even for experienced developers because there are too many ways to go about doing any given task, and oftentimes the "right" way of doing something is a matter of taste. Impatient developers gave up and just shoehorned shitty code wherever they could.

:+1: fewer options is better

:+1: standardization

:+1: :+1: :+1: :+1: :+1: :+1: :+1: :+1: :+1: :+1: :+1: :+1: some better docs this time around

:8ball: Twig

lespinosa commented 11 years ago

I think this here is not with what you feel more comfortable, it's about being consistent to the present, to follow a standard, and you have to be honest, the template system using Magento 1.x, is a headache for anyone, and if we want Magento forward and meets the new standard of venture and we should always stay the same, support the standardization and my vote is for.

Twig.

barbazul commented 11 years ago

I am sorry, but not being a Twig user myself I fail to see how Twig would fix the template system in Magento. So far I was under the assumption that it is just a templating system that needs a special parser to produce HTML code.

From my understanding the complexity of Magento current templating system is the layout engine which might seem alien to the regular non-Magento-specialist PHP/frontend developer. Is Twig supposed to replace the layout system? I was under the assumption that was not the case, as a lot of updates to such engine have been already implemented by the Magento team.

Assuming the layout engine stays the same, how is Twig going to simplify the current templating system other than switching languages?

michaelgrosser commented 11 years ago

Twig ensures that you can't put extensive business logic in your template. It allows for simple logical constructs like IFs and loops, but you can't do things like query a Collection in your template or instantiate a model object. I think the difference with the layouts would be that you would have to define a Datasource for the template. The Datasource would pass set data to the Twig template to be used in variables since you would not be able to directly trigger class methods from inside the template. It's basically a way to ensure better practices are following with regard to keeping business logic separate from presentation logic.

benmarks commented 11 years ago

Change of heart. It's really not the responsibility of Magento to guard against poor dev practices. Open source PHP apps are going to be subject to abuse. Standardizing on Twig introduces an unnecessary external dependency which is not necessarily the best choice for all developers now and very likely will not be as M2 evolves. Why should anyone have to use one templating system over another? I get that users of/proponents of Twig might not be able to understand what the big deal is, but, if all instances of "Twig" in this thread are replaced with "Smarty" then perhaps the point becomes clear.

philwinkle commented 11 years ago

:thumbsup: @benmarks ... I fully agree. Moving abuses from the view layer elsewhere doesn't eliminate abuse, it relocates. Well-said.

kalenjordan commented 11 years ago

From my understanding the complexity of Magento current templating system is the layout engine which might seem alien to the regular non-Magento-specialist PHP/frontend developer. Is Twig supposed to replace the layout system?

@barbazul really good point. I actually don't know personally whether they would change the layout system at all. And using twig's layout system would be basically the main reason that I would vote for it. It's what I love about twig - it makes it so much easier to write clean, extensible, DRY template code.

benmarks commented 11 years ago

From my understanding the complexity of Magento current templating system is the layout engine which might seem alien to the regular non-Magento-specialist PHP/frontend developer. Is Twig supposed to replace the layout system?

I would hope not, though I'm saying this in part because I do not see corresponing functionality in the Twig world. There has already been a decent amount of effort into making layout XML more about view composition, adhering to DRY, and less about business logic. Layout XML has a distinct, novel, and useful purpose, and I resubmit that Twig (or any other external tech) should not be a standard to program the M2 framework towards.

alankent commented 11 years ago

I don't notice much about security in this thread. Code such as <?php echo $x; ?> can be dangerous as it does not escape HTML sensitive characters. Do people worry about the security aspects much? Is this because it's not a risk, or people don't understand the risk? (Many templating systems escape output by default, addressing this security concern.)

artlantis commented 11 years ago

@alankent Dear Alan, how should be? Give us an example! Even if you use blah; exit(); in the echo command, just output as it's because echo print out the strings!

alankent commented 11 years ago

I started writing a reply to what I think your question was, but it got long so I turned it into a blog post at http://alankent.wordpress.com/2013/10/19/html-escaping-for-secure-web-pages/. In summary, you should call htmlspecialchars() to escape HTML sensitive characters.

larsroettig commented 11 years ago

+1 for Twig, and for only ONE option available.

sshymko commented 11 years ago

Hi.

Thanks for bringing up such an important topic. Comments in this thread have influenced the decision to keep a single template engine in Magento 2. Complete transition to Twig is not feasible in terms of effort required to convert all existing templates. The decision has been made to support solely PHTML templates out of the box. Twig support is going to be removed from the code base with the next publication from the internal repo. Template engine abstraction will remain. It would be possible for a module/extension to introduce new template engine into the system and associate it with templates having certain file extension.

Thanks everyone, who participated in the discussion, much appreciated.

Closing.