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

\TypeError thrown when TwigFunction has an invalid callback. #4393

Open
menno-ll opened this issue Oct 11, 2024 · 4 comments
Open

\TypeError thrown when TwigFunction has an invalid callback. #4393

menno-ll opened this issue Oct 11, 2024 · 4 comments

Comments

@menno-ll
Copy link

We're using WordPress and in there we attempt to load Twig templates.
For us we create a WP_CLI command, which registers itself before WordPress itself is loaded.

Previously (twig version v3.11.1 and before) this all worked great.
Twig allowed functions that were not defined yet to still be compiled as php files.
We did this using the same method as described in #3706.
See our repo of the CLI command.

$twig->registerUndefinedFunctionCallback(function ($name) {
    return new \Twig\TwigFunction($name, $name);
});

But unfortunately now, since version v3.12.0, this does not work anymore and throws the following TypeError.

Uncaught TypeError: Failed to create closure from callable: function "_x" not found or invalid function name in /path/to/project/vendor/twig/twig/src/Util/ReflectionCallable.php:45

Now I have to confess i'm not having a full knowledge of the twig codebase yet.
But after searching and looking at the stacktraces, I've found that somewhere around these PR's to be where the problem started: #4206 or #4207

It seems like even if a function does not exist yet on compilation, it still attempts to extract it's attributes with the new CallableArgumentsExtractor.

Would there be a way to return the functionality to not validate those callable arguments if the function at that stage does not exist, therefore allowing the use of functions that are not yet defined on runtime?

Thanks in advance!

@stof
Copy link
Member

stof commented Jan 17, 2025

How would the rendering of your template work if you pass a non-existing PHP callable as the implementation of the Twig function ?

#4207 has indeed introduced more cases where we inspect the callable with reflection at compile-time. It is now done for every usage of a function, while it was done previously only for calls that involved named arguments.
But this means that we never had a case where we fully supported using an undefined function implementation at compile-time. And it was already broken at runtime previously (as rendering the template would try to call that undefined PHP function).

If your use case is to allow using any function in a template as a no-op (which is pretty weird DX to me), use return new TwigFunction($name, function () {}) instead.

@ericmorand
Copy link
Contributor

@stof so you can't compile a template and then exécute it with a runtime that actually provides the function? What is the benefit of this approach?

@stof
Copy link
Member

stof commented Jan 20, 2025

Named arguments in Twig relies on knowing the function signature at compile-time (allowing seamless conversion between snake_case and camelCase argument names would be a no-go if we had to do it at runtime as it would have a performance cost).

@menno-ll
Copy link
Author

How would the rendering of your template work if you pass a non-existing PHP callable as the implementation of the Twig function ?

This I can explain.
The command runs without wordpress or plugins loaded.
This would improve performance of the command by quite a lot.
Because simply put, wordpress itself isn't always that performant, and certainly plugins make this way worse.
And for just transforming twig templates to php files, wordpress simply isn't needed.

And because functions that didn't exist at the time of running that command, the twig file would simply be parsed to php code (that indeed at that time contains unexisting functions), this was not a problem.

The generated PHP files were then used by the wp i18n make-pot command, which takes php files inside a wordpress theme and attempts to parse php code from them. At that time the functions do exist, as wordpress actually is loaded.

This whole thing is only to just allow wordpress to generate translation templates for translation functions used in twig templates.

If your use case is to allow using any function in a template as a no-op (which is pretty weird DX to me), use return new TwigFunction($name, function () {}) instead.

I do agree that maybe having wordpress loaded in when running the command would be the best solution, but due to performance reasons i think the fix suggested is way better than what we had.

I guess this solution is way simpler than i imagined!

I'll try it in the following days in some projects and will update this issue if it's indeed fully resolved.

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

No branches or pull requests

3 participants