There is no real benefit over usingtextTemplate andhtmlTemplate (ok, you only have one template instead of two... but the text template can only be automatically created based on the HTML one, so...);
It means having more than one way to do the same thing (do I set the subject on the object directly or in the template for instance);
A major drawback that is not easy to spot: the template is HTML, so thesubject andtext block must be carefully crafted to avoid avoid HTML escaping.

carsonbot added Status: Needs Review Feature labels

Apr 3, 2019

javiereguiluz approved these changes

Apr 3, 2019

View reviewed changes

Copy link

Member

javiereguiluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I agree.

While documenting the Mime component (pending PR here:symfony/symfony-docs#11280) I also found a bit confusing to have multiple ways of doing the same thing.

Also, using pure Twig code to define all email parts (such as the priority, subject, from, to, etc.) looks appealing at first but it generates complex and not very readable code in real-world applications. Better use PHP to create the message and (optionally) Twig to render its contents.

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Apr 3, 2019

Copy link

Member

stof commentedApr 3, 2019

👍 for me.

My current system in the Incenteev codebase looks like thistemplate way (minus theconfig block), and usage makes me wish to rewrite that to use separate templates (except I won't do it until I can migrate to symfony/mailer instead, as doing the rewrite now would be a waste of time).

nicolas-grekas added this to thenext milestone

Apr 3, 2019

nicolas-grekas approved these changes

Apr 3, 2019

View reviewed changes

Copy link

Member

nicolas-grekas commentedApr 3, 2019

(but see fabbot ;) )

[Twig] removed TemplatedEmail::template()

5e61b75

fabpot force-pushed thetemplate-mime-simplification branch from1775552 to5e61b75Compare

April 3, 2019 18:46

fabpot merged commit5e61b75 intosymfony:master

Apr 3, 2019

fabpot added a commit that referenced this pull request

Apr 3, 2019

feature#30853[Twig] Remove TemplatedEmail::template() (fabpot)

de7d7a9

This PR was merged into the 4.3-dev branch.Discussion----------[Twig] Remove TemplatedEmail::template()| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aI propose to remove `TemplatedEmail::template()` for several reasons: * There is no real benefit over using `textTemplate` and `htmlTemplate` (ok, you only have one template instead of two... but the text template can only be automatically created based on the HTML one, so...); * It means having more than one way to do the same thing (do I set the subject on the object directly or in the template for instance); * A major drawback that is not easy to spot: the template is HTML, so the `subject` and `text` block must be carefully crafted to avoid avoid HTML escaping.Commits-------5e61b75 [Twig] removed TemplatedEmail::template()

vincenttouzet pushed a commit to vincenttouzet/symfony that referenced this pull request

Apr 3, 2019

featuresymfony#30853[Twig] Remove TemplatedEmail::template() (fabpot)

25c3cbc

This PR was merged into the 4.3-dev branch.Discussion----------[Twig] Remove TemplatedEmail::template()| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aI propose to remove `TemplatedEmail::template()` for several reasons: * There is no real benefit over using `textTemplate` and `htmlTemplate` (ok, you only have one template instead of two... but the text template can only be automatically created based on the HTML one, so...); * It means having more than one way to do the same thing (do I set the subject on the object directly or in the template for instance); * A major drawback that is not easy to spot: the template is HTML, so the `subject` and `text` block must be carefully crafted to avoid avoid HTML escaping.Commits-------5e61b75 [Twig] removed TemplatedEmail::template()

Copy link

Member

lyrixx commentedApr 4, 2019•
edited
Loading

There is no real benefit over usingtextTemplate andhtmlTemplate (ok, you only have one template instead of two... but the text template can only be automatically created based on the HTML one, so...);

This is not really true about link :
<a href="XXX"> click here</a> will result inclick here without the link :/