smarty-php/smarty

Unable to override built-in modifiers.

2vcreation opened this issue · 3 comments

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 :

  • If it's not handled in DefaultExtension :
    Best scenario, i can do whatever i want.
    I can use registerPlugin function.
    Or i can handle it in a "MyCustomExtension" extension.

  • If it's handled in DefaultExtension->getModifierCallback() :
    I can't use registerPlugin function (it will be silently ignored)
    I can handle it in MyCustomExtension->getModifierCallback() or ->getModifierCompiler(), as both will be executed before DefaultExtension->getModifierCallback().

  • If it's handled in DefaultExtension->getModifierCompiler() :
    I can't use registerPlugin function (it will be silently ignored)
    I can't handle it in MyCustomExtension->getModifierCallback() (it will be silently ignored too)
    I have to handle the modifier in MyCustomExtension->getModifierCompiler() (i have no other choice).

  • And in the future updates of Smarty, if DefaultExtension is changed (adding more modifiers, or moving from a type to another), then MyCustomExtension could be (silently) obsolete.

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 ?

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.

Fix proposition in #1049

@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 😬