smarty-php / smarty

Smarty is a template engine for PHP, facilitating the separation of presentation (HTML/CSS) from application logic.
Other
2.26k stars 712 forks source link

Unable to override built-in modifiers. #1048

Open 2vcreation opened 3 months ago

2vcreation commented 3 months ago

Hello,

As it could be done in previous versions, with V5 we are not able to override a built-in modifier using registerPlugin function (Even with unregisterPlugin before).

For example :

//Will not work, default one in DefaultExtension->getModifierCallback() will be used instead.
$this->unregisterPlugin(Smarty::PLUGIN_MODIFIER,"date_format"); //With or without this line, same result
$this->registerPlugin(Smarty::PLUGIN_MODIFIER,"date_format",[$this,"myDateFormatModifier"]); 

//Will not work, defaut one on DefaultExtension->getModifierCompiler() will be used instead (during template compilation)
$this->registerPlugin(Smarty::PLUGIN_MODIFIER,"json_encode",[$this,"myJsonEncode"]); 

//Will not work, default one will be used instead (and triggers an error directly with a message to use "join" instead, changing params orders)
$this->registerPlugin(Smarty::PLUGIN_MODIFIER,"implode",[$this,"myImplodeModifier"]); 

...

//Will work, as there is no "myownmodifer" defined in DefaultExtension.
$this->registerPlugin(Smarty::PLUGIN_MODIFIER,"myownmodifier",[$this,"myOwnModifier"]); 

By Searching in the issues, i have seen that there is a workaround, as it's suggested in #1011 (kind of describing that problem with a custom "json_encode" modifier). Solution seems to be to create a custom Extension, implementing getModifierCompiler() and getModifierCallback() functions, and register it between Core and Defaut extensions. In that way our Custom Extension can handle modifiers before DefautExtension does it.

But it's not that easy for all cases, as it brings complexity with the different types of modifiers : "Compiler" Ones (if i understand correctly, executed first when creating compiled templates files), and "Classic" Ones (executed after).

For example, let's say that i want to handle a modifier in MyCustomExtension->getModifierCallback() (not at compilation), if that modifier exists in DefautExtension->getModifierCompiler(), my implementation will be (silently) ignored too. That's because DefaultExtension->getModifierCompiler() will be executed way before MyCustomExtension->getModifierCallback();

So to summarize, if i understand correctly, and from what i've tested so far, for now with v5, every time i want to add/handle a modifier, i have to take care of these points :

I'm not very enthousiastic with that Extension solution, it's not very intuitive or user fiendly, and seems to required to think about more cases than it appears. It would be very very much simpler if by default, adding a modifier will take the lead over default ones defined in DefaultExtension.

As soon as we use "registerPlugin" function, don't we exepect that Smarty use the callback we just explicitly defined ?

2vcreation commented 3 months ago

Here is a try to solve the problem (with Extension) :

Idea is to replace DefaultExtension by MyCustomExtension, but still extending the first one. As a result if i register a Modifier with "registerPlugin" function, it will not be silently ignored by DefaultExtension. But if i don't, default modifiers defined in DefaultExtension behaviour will continue to work.

MyCustomExtension example :

class MyCustomExtension extends \Smarty\Extension\DefaultExtension
{
    /** @var \Smarty\Smarty */
    private $smarty;

    public function __construct(\Smarty\Smarty $smarty) {
        $this->smarty = $smarty;
    }

    public function getModifierCallback(string $modifierName)
    {
        return ($this->smarty->getRegisteredPlugin(\Smarty\Smarty::PLUGIN_MODIFIER, $modifierName)) ? null : parent::getModifierCallback($modifierName);
    }

    public function getModifierCompiler(string $modifierName): ?\Smarty\Compile\Modifier\ModifierCompilerInterface
    {
        return (
            $this->smarty->getRegisteredPlugin(\Smarty\Smarty::PLUGIN_MODIFIER, $modifierName)
            ?? $this->smarty->getRegisteredPlugin(\Smarty\Smarty::PLUGIN_MODIFIERCOMPILER, $modifierName)
        ) ? null : parent::getModifierCompiler($modifierName);
    }
}

Smarty config example :

$smarty->setExtensions([
    new \Smarty\Extension\CoreExtension(),
    new MyCustomExtension($smarty),   //Replaces new \Smarty\Extension\DefaultExtension()
    new \Smarty\Extension\BCPluginsAdapter($smarty)
]);

//Will work now
$smarty->registerPlugin(Smarty::PLUGIN_MODIFIER,"date_format",[$this,"myDateFormatModifier"]); //Not overriden by DefaultExtension->getModifierCallback()
$smarty->registerPlugin(Smarty::PLUGIN_MODIFIER,"json_encode",[$this,"myJsonEncodeModifier"]); //Not overriden by DefaultExtension->getModifierCompiler()
$smarty->registerPlugin(Smarty::PLUGIN_MODIFIERCOMPILER,"round",[$this,"myRoundModifierCompiler"]);  //Not overriden by DefaultExtension->getModifierCompiler()

An issue anyway :

I still think that the default behaviour is tricky, and that we shouldn't have to replace DefautExtension by a Custom one. DefaultExtension could be modified accordingly to test if a modifier has been registered before.

2vcreation commented 3 months ago

Fix proposition in #1049

antman3351 commented 2 months ago

@wisskid I think this behaviour needs to be added to the docs under "Upgrading from an older version" section in bold as it doesn't throw any errors and can lead to data loss.

In my case I overrode date_format which was now outputting invalid date/times into date and time inputs 😬