mezzio / mezzio-platesrenderer

Plates integration for Mezzio
https://docs.mezzio.dev/mezzio/features/template/plates/
BSD 3-Clause "New" or "Revised" License
4 stars 8 forks source link

Remove dependency on laminas-escaper #26

Open pine3ree opened 1 year ago

pine3ree commented 1 year ago

The plates renderer factory automatically registers the EscaperExtension with the plates engine.

As suggested in the class commentary, in order to use different helpers we have to use the provided escaper extension FQCN as a container alias for our own or third party implementation, which feels a bit strange.

Another way is to write our own factory that discard the automatic injection of the extension

In any case the laminas-escaper package is still listed as a composer dependency and gets downloaded even if not actually used,

Wouldn't it be better to just mark laminas-escaper as a suggested package and document its availability through configuration like any other extension? We could also add a escaper class existence test inside the extension constructor, and throw an exception if missing with a message suggesting to "require" the laminas-escaper composer package

kind regards

froschdesign commented 1 year ago

@pine3ree

Wouldn't it be better to just mark laminas-escaper as a suggested package and document its availability through configuration like any other extension?

Why remove an essential extension or mark it as optional when the escape process itself is important in templates?

Reference:

pine3ree commented 1 year ago

Hello @froschdesign .

Of course I am not suggesting not-to-escape. Escaping is essential. (Plates has also a couple of hard coded helpers for that ... laminas-escaper is much better)

But IMO it's the developer's choice which escaper to use and how to name/register related template helpers.

This package seems to impose the usage of laminas-escaper, which is fine for laminas-mvc or monolithic frameworks with their own escapers but sounds contrary to mezzio phylosophy of allowing a developer which package to use for a specific goal.

In brief, escaping is essential (that is obvious) . Escaping using only a particular escaper (laminas-escaper in this case) is not.

kind regards

PS laminas-escaper is great, I am not questioning that. I am questioning the imposition. It should be a suggestion.

froschdesign commented 1 year ago

It should be a suggestion.

Maybe you create a pull request to illustrate your idea but I can already tell you that no one reads the suggestion section in composer.json.

pine3ree commented 1 year ago

If a developer decide to use the laminas-escaper package and helper we can just alter the extension constructor to inform if the package is installed:

pseudo-code (I omitted namespaces for brevity)

//..
class EscaperExtension implements ExtensionInterface
{
    private Escaper $escaper;

    public function __construct(?string $encoding = null)
    {
        if (! class_exists(Escaper::class)) {
            throw new RuntimeException(sprintf(
                "The `%s` class was not found! Please run `composer require "
                . "laminas/laminas-escaper` in order to install it",
                Escaper::class
            ));
        }

        $this->escaper = new Escaper($encoding);
    }
//...

The mezzio-skeleton would have a global configuration for plates that includes the extension

pseudo-code (I omitted namespaces for brevity)


return [
    // container
    'dependencies' => [
        'factories' => [
            EscaperExtension::class => EscaperExtensionFactory::class,
        ],
    ],
    // plates
    'plates' => [
        //....
        'extensions' => [
            EscaperExtension::class,
            //....
        ],
    ],
];

In this way a developer easily can remove the extension and add her/his own

(edited+) laminas-escaper can be included in mezzio-skeleton. But it's easily removable from the application composer.json.