FRAMEWORK method addFlash definition in AbstractController differs from underlying FlashBag->add

[crayner]

Symfony version(s) affected: 4.4.0 - 5.0

Description
The addFlash method in the AbstractController (5.0) and the ControllerTrait (4.4) limit the definition of a message to a string, but the Flashbag itself does not limit the message by type at all.

The addFlash in the controller method is:

 
    /**
     * Adds a flash message to the current session for type.
     *
     * @throws \LogicException
     */
    protected function addFlash(string $type, string $message): void
    {
        if (!$this->container->has('session')) {
            throw new \LogicException('You can not use the addFlash method if sessions are disabled. Enable them in "config/packages/framework.yaml".');
        }
        $this->container->get('session')->getFlashBag()->add($type, $message);
    }

and the Flashbag method is

    /**
     * {@inheritdoc}
     */
    public function add(string $type, $message)
    {
        $this->flashes[$type][] = $message;
    }

I have been using the Flashbag with a Message as an array [message, params, domain] and twig just looks for an iterable message and renders appropriately with translation.

How to reproduce
Any controller code that uses addFlash must use a string ONLY for the message.

Possible Solution
Remove the restriction on the addFlash so that addFlash and FlashBag->add are defined equally.

    /**
     * Adds a flash message to the current session for type.
     *
     * @throws \LogicException
     */
    protected function addFlash(string $type, $message): void
    {
        if (!$this->container->has('session')) {
            throw new \LogicException('You can not use the addFlash method if sessions are disabled. Enable them in "config/packages/framework.yaml".');
        }
        $this->container->get('session')->getFlashBag()->add($type, $message);
    }

Additional context
I would vote for allowing the addFlash to not be limited to only strings. An example of my current flash renderer in TWIG is:

{% trans_default_domain 'messages' %}
{% for label, messages in app.flashes %}
    {% for message in messages %}
        <div class="{{ label }}" id="message{{ label ~ loop.index0 }}">
            {% if message is iterable %}
                {{ message[0]|trans(message[1]|default({}), message[2]|default('messages')) }}
            {% else %}
                {{ message|trans }}
            {% endif %}
            {% if close_message is not defined or close_message %}
                <button class="button close {{ label }}" title="Close Message" type="button" onclick="closeMessage('message{{ label ~ loop.index0 }}')">
                    <span class="fas fa-times-circle fa-fw {{ label }}"></span>
                </button>
            {% endif %}
        </div>
        <script>
            function closeMessage(id){
                var element = document.getElementById(id)
                element.classList.toggle("hidden");
            }
        </script>
    {% endfor %}
{% endfor %}

Appreciate any feedback!

Craig

[ro0NL]

more or less duplicates #28991 :)

AFAIK we should loosen ControllerTrait, as per https://github.com/symfony/symfony/issues/28991#issuecomment-441358703`, for consistency.

Ultimately users are in control, and responsible if e.g messages are valid payload/serializable/etc.

Perhaps, given the type is mixed at the API level, simply renaming $message to $payload (or $data) would clarify 🤔 nowhere we refer e.g. a FlashMessageBag.. it's just a FlashBag.

[crayner]

Thanks