For our site, we wanted to create a new service very similar to the messenger service, which is defined in core.services.yml as follows:
messenger:
class: Drupal\Core\Messenger\Messenger
arguments: ['@session.flash_bag', '@page_cache_kill_switch']
Our custom service needed to use a separate FlashBag. The session.flash_bag service is also in core.services.yml:
session.flash_bag:
class: Symfony\Component\HttpFoundation\Session\Flash\FlashBag
public: false
Looking at this, you would think that you could just make a new service called something like mymodule.flash_bag. However, if you do you will find that it doesn't work. The reason is that the session service is defined like this:
session:
class: Symfony\Component\HttpFoundation\Session\Session
arguments: ['@session_manager', '@session.attribute_bag', '@session.flash_bag']
tags:
- { name: service_collector, tag: session_bag, call: registerBag }
The session.flash_bag service gets special treatment: it is passed into the constructor of Session. If you want to define an additional FlashBag, you need to tag it with the session_bag tag so the session service persists it between page loads. No other service uses FlashBag and none at all uses the session_bag tag so there's no example to copy.
Add a note to session.flash_bag that if you want to add more flash bags, they need to be tagged with the session_bag tag.
Add documentation to core.services.yml
None.
None.
None.
None needed.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3200213_5.patch | 692 bytes | ghost of drupal past |
Comments
Comment #2
ghost of drupal pastComment #3
jhodgdonI didn't understand what was going on, but chx explained it to me in Slack and pointed to this related issue. I'm updating the issue summary with hopefully a more complete and hopefully correct explanation.
I suggest the following wording for the comment:
The session.flash_bag service has special treatment: it is passed in as an argument to the session service constructor. If you need to add an additional bag to the session, make sure your service is tagged with the "session_bag" tag so that it will be collected.
Comment #4
ghost of drupal pastComment #5
ghost of drupal pastComment #6
jhodgdonThe latest patch looks good to me, and I think this issue makes a good argument as to why this documentation is useful to have in there.
Comment #7
longwaveIs there a good reason
session.flash_bagis special cased? Should we tag it instead and drop it from the constructor?Comment #8
jhodgdonThe Session class is in Symfony and expects it to be put into its constructor.
https://api.drupal.org/api/drupal/vendor%21symfony%21http-foundation%21S...
If you don't pass it into the constructor, it does this:
and then it gets registered and it is special because you can call getFlashBag() to return it. Presumably we or Symfony is relying on that somewhere?
So, I don't think we can do what you suggest in #7. The Session class needs to have a FlashBag of its own.
Comment #9
ghost of drupal pastThe
Sessionclass ensures there is always a special session bag which is not merely a session bag a flashbag and can be retrieved withSession::getFlashBag. Any bag service tagged with thesession_bagtag, on the other hand, might or might not be a flash bag and can be retrieved via thegetBag()method. It surely is possible to forego the usage of the special case flash bag and register a different flash bag for messenger and tag it appropriately. The code change required, I believe, would be quite small: drop'@session.flash_bag'from theargumentsofsessionand add thesession_bagtag to the definition ofsession.flash_bag. The backwards compatibility considerations, however, are just beyond me: currently it is possible to go around theMessengerclass and manipulate messages usingSession::getFlashBag. If we use our own bag, this will break. This of course might not be a big deal. Anothe tiny issue is thatSessionwould create its own flash bag anyways and it would just sit there unused. On the other hand, it's not clear to me what advantage we would gain by doing this. I think the time for this change would come when core wants to use multiple flash bags and it's just not clear which one should be the session flash bag. Until then, I think at least, the current situation is the simplest and semantically most correct.Comment #10
longwaveWas wondering if something like this would work
but I think setting both factory and session_bag tag is confusing and/or will cause a circular reference, and doesn't really help with developer discovery anyway.
Therefore I think just improving the documentation is enough.
Comment #11
andypostRelated CR and there's example in tests https://www.drupal.org/node/3109877
Special approach for flashbag looks was needed for BC so docs here useless
Comment #13
catchCommitted 3b82a3b and pushed to 9.2.x. Thanks!