Minimal usable Symfony Mailer prototype

I'm a bit unclear on the second step. For a first iteration, I think we can ignore BC and just explore how the new API should look like for a module using it.

Then when we add BC, we kind of have two options. We could do it inside the new service, but the question is how we translate the hook_mail() context stuff, I guess we would need the $params array to be passed in and part of the new API and then deprecate it in 11.x, assuming we can get rid of the old system in 11.0. Since we'll be working with Email value objects, we could have a setMailHookParams($params) and we can deprecate that method then. We would definitely also need to keep the concept of $module and $key parameters at least for BC support, but we kind of discussed already that we want *something* to identify mails in events and templates and so on.

The other option would be that each caller would be responsible for respecting that BC feature flag, Core could provide the setting and a trait or so, something like https://www.drupal.org/node/3379306 → .

Initially, I didn't even think of the first option, but with a value object and those extra method(s), that might work and make it easier for code to still support the old mail hook and API. One thing that I'm not sure about is return values, because the old mail API has that, for example to check if a mail was really sent or if there were errors. How would we handle that as we will have two very different elements to return then?

(I'm of course also unsure about the first step, with our own wrapper objects or not and so on, but I don't have much of an opinion on that right now).

For a first iteration, I think we can ignore BC and just explore how the new API should look like for a module using it.

Well I tried that in ✨ Change core modules to use Symfony Mailer Active 😃. Your response was that this was icing on the cake, and instead I needed to provide a whole slice - hence this issue.

Initially, I didn't even think of the first option, but with a value object and those extra method(s), that might work and make it easier for code to still support the old mail hook and API.

Yes exactly. This is all described in detail in ✨ Migration strategy for moving to Symfony Mailer Active .

I guess we would need the $params array to be passed in and part of the new API and then deprecate it in 11.x

I believe we can avoid that. The BC code inside the service can call the new API then construct the $params array in the majority of cases.

I've been working with DSM for a year now, so I understand the problems quite well. I have this big complicated picture in my head of how everything can work in Core with minimal disruption. It's a bit hard to explain to people, and some of it seems strange and unintuitive, however in most cases I have a reason😃. What I don't necessarily know is what Core wants to be like - as I discover that I can adapt my ideas.

> Well I tried that in ✨ Change core modules to use Symfony Mailer Active . Your response was that this was icing on the cake, and instead I needed to provide a whole slice - hence this issue.

Yeah, these two issues overlap and I'm not sure we need both.

But what I meant there is that it should includes the code changes for a specific module, but that's not actually functional yet, as the things it depends on don't exist yet? Or I missed that in the MR, I also didn't look at it anymore after my comments.

Whereas here I meant we don't need to worry about the BC aspect with the hook params and stuff at first.

But quite possibly these two really are the same issue, at least the way I imagined it.

> I believe we can avoid that. The BC code inside the service can call the new API then construct the $params array in the majority of cases.

I'm not quite sure how it could do that, but we can discuss that when we get to that point. majority of cases isn't enough though, it would nee to work in all cases. If you mean we could do some basic cases, but if you have more specific cases then you can still do it yourself, then yeah, maybe? I guess it also depends on whether or not we'll have explicit builder things and if we basically mandate that hook_mail() and those have the same params? Again, discussion for later.

> Yeah, these two issues overlap and I'm not sure we need both.
In my mind the difference is like this:

• This issue is aiming to write code that could actually run, but only in a restricted case and without tests - it would never be committed.
• The other issue is something we'd eventually commit, following on from an issue that does "Create a new mailer service/API ready for symfony, but still send using the old mail service". However I already made a guess at what it would look like for early review.

> Whereas here I meant we don't need to worry about the BC aspect with the hook params and stuff at first.
This relates to a critical point in my strategy. I believe it's much easier to handle the BC aspect then the mainline aspect.

• For BC we need a mapper from the new API to the old. Actually they are quite similar, so it's not so hard. The existing tests will all still pass. I believe we can commit in a minor release this within Core BC rules.
• For mainline, we need the full working mailer service. We need to change tests in Core that send mail. And we can't commit it, because it's not BC😃.

So IMHO, BC is not a worry, it's a smart way forward. Of course you may ask (because you ask good questions😃) how can we know the interface is correct without mainline? I propose we can prove that with a mainline implementation in Contrib. We already have one anyway, it would just need some minor adjustment where the interfaces are different.

>> The BC code inside the service can call the new API then construct the $params array in the majority of cases.
> I'm not quite sure how it could do that,

Basically it sounds like we are mostly in agreement. As you say, conversion can be automatic if the ID and params remain the same. Otherwise the module would implement legacyMail() to assist in the conversion, and that code would later be deprecated and removed. This is about as easy as we can possibly make it I think. Certainly much better than forcing every bit of code that wants to send a mail to check a flag to determine which API to call.

The argument for "builder things" is here ✨ Create a plug-in system to build emails with Symfony Mailer Active . I've explained why I feel they are unavoidable, and also proposed a way to make them very lightweight with a FallbackMailer.

I feel it could work much better to have a discussion call on how to move forward. Comments on issue can be good for developing ideas, but it's not so flexible and dynamic as "brain storming".

I'm not exactly sure I understand what you refer with "mainline" exactly, but unless I misunderstand you, we can absolutely change/break mail related tests in minor releases. That is, in fact, the only time we can change them. Major releases are for the most part just deprecation removals and dependency updates and adjustments.

If we follow my idea in ✨ Framework for testing Symfony Emails Active , with a per-test flag, we wouldn't even break contrib tests that specifically test user, contact or similar mails from another module that already did the conversion.

So if you set that flag in your tests, and core could as discussed already prepare for that ahead of the actual conversion through the symfony mailer @Mail plugin, it will bypass the legacy mail API and go directly to whatever services and wrappers we'll end up using to Symfony Mailer (and in tests, our test transport). And if that flag isn't set, it will use the legacy route, through mail manager and the hook and the @Mail plugin.

> Certainly much better than forcing every bit of code that wants to send a mail to check a flag to determine which API to call.
Yes, that does all sound quite feasible, I think what I'm concerned about is all those special, undocumented not standardized @Mail plugin specific approaches to advanced features like HTML mails, attachments (\Drupal\simplenews\Mail\MailBuilder::buildNewsletterMail() comes to mind, but I guess that's actually mostly contained within $message, which only happens within and after hook_mail(), so that might actually be much less of an issue than I thought (Some modules like commerce try their very best to keep as much as possible out of the mail hook with \Drupal\commerce\MailHandler::sendMail(), but even they switched to doing the actual render in commerce_mail() to benefit from the mail theme feature.

About mail builders + the fallback idea. I didn't think about it too much, but yeah, the context switching thing is of course a valid point. (FWIW, I don't think "AFAICS this was one of the main motivations for hook_mail() in the old mail API" is quite true, because there is zero context switching in MailManager. only contrib mailsystem does the theme switch, and language happens custom just in user_mail and only config language too, that could also be outside the call. So that context switching evolved later.

I think the FallbackMailer could well be a DefaultMailer or something like that, the use of callbacks and anonymous functions are becoming more and more common, and with both t() being delayed to actual execution (string cast) and rendering as well, quite a lot of use cases could just pass in their render arrays and TranslatableMarkup objects without an issue.

I feel we should try to keep the discussions on their own issue (sorry I linked lot's of issues in my previous comment which started a discussion, but let's try to untangle)

Any further Discussion on ✨ Framework for testing Symfony Emails Active

> we can absolutely change/break mail related tests in minor releases
I mean that AssertMailTrait is a test API, and it would be a big problem for simplenews to make a non-BC change it in a minor release. I agree that Core can changes its own tests in a minor release.

> So if you set that flag in your tests,
This is a flag to choose between @Mail or @MailerTransport testing, and it would default to the old way?? Then yes, good idea, that makes sense.

Any further Discussion on ✨ Migration strategy for moving to Symfony Mailer Active

> I think what I'm concerned about is all those special, undocumented not standardized @Mail plugin specific approaches to advanced features like HTML mails
Yes I agree these would not be easy to convert and I feel the conversion is mostly intended for standard plain text emails.

There is discussion in ✨ Migration strategy for moving to Symfony Mailer Active of 3 possible strategies, and the recommended one for most Contrib modules that send complex mails is to use a major version to switch to the new API. For simplenews, I expect that 5.x would natively use DSM and 4.x would stay using the old API. So you change major version at the same time as you change mail system, and there is no need for conversion.

Any further Discussion on ✨ Create a plug-in system to build emails with Symfony Mailer Active

> there is zero context switching in MailManager
MailManager switches render context.

> I think the FallbackMailer could well be a DefaultMailer
It's an interesting idea, but it only covers advantage #2. We would still need a place to put the metadata.