Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to override built-in modifiers. #1048

Open
2vcreation opened this issue Jul 30, 2024 · 3 comments · May be fixed by #1049
Open

Unable to override built-in modifiers. #1048

2vcreation opened this issue Jul 30, 2024 · 3 comments · May be fixed by #1049

Comments

@2vcreation
Copy link

2vcreation commented Jul 30, 2024

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 ?

@2vcreation
Copy link
Author

2vcreation commented Jul 31, 2024

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 2vcreation linked a pull request Jul 31, 2024 that will close this issue
@2vcreation
Copy link
Author

Fix proposition in #1049

@antman3351
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants