[TwigBridge] don't use deprecated and internal Twig functions #52990
Conversation
| @@ -158,6 +158,26 @@ public function getFieldChoices(FormView $view): iterable | |||
| yield from $this->createFieldChoicesList($view->vars['choices'], $view->vars['choice_translation_domain']); | |||
| } | |||
|
|
|||
| /** | |||
| * @internal | |||
There was a problem hiding this comment.
Maybe we can put a comment that this function has been copied from twig_test_empty, to help track changes.
There was a problem hiding this comment.
Or maybe we use CoreExtension::testEmpty() if it exists?
There was a problem hiding this comment.
CoreExtension::testEmpty() is internal, its signature could change, or it could be moved or renamed.
There was a problem hiding this comment.
Given that we intend to migrate Twig into a Symfony component, I think it's fine to access @internal functions.
There was a problem hiding this comment.
Even in Symfony components, it is not fine to use @internal APIs of a different component, as components relies on semver ranges in their inter-component requirements.
twig_test_empty() function (it's deprecated since Twig 3.9)
|
The same seems to be needed for
|
twig_test_empty() function (it's deprecated since Twig 3.9)twig_test_empty() function anymore
twig_test_empty() function anymore| /** | ||
| * @see \Twig\Extension\CoreExtension::testEmpty() | ||
| */ | ||
| private static function isEmpty($value): bool |
There was a problem hiding this comment.
If we rely on the internal twig function for the compiled template, we can do the same for the compilation step and avoid duplicating the code.
But I'm still not sure this is a sane solution to rely on the internal function.
There was a problem hiding this comment.
I agree, let's use the Twig method instead.
There was a problem hiding this comment.
Maybe these methods shouldn't be @internal then?
| /** | ||
| * @see \Twig\Extension\CoreExtension::testEmpty() | ||
| */ | ||
| private static function isEmpty($value): bool |
There was a problem hiding this comment.
I agree, let's use the Twig method instead.
src/Symfony/Bundle/WebProfilerBundle/Twig/WebProfilerExtension.php
Outdated
Show resolved
Hide resolved
54b96d3 to
cb75597
Compare
| // Only insert the label into the array if it is not empty | ||
| if (!twig_test_empty($label->getAttribute('value'))) { | ||
| if (!$isEmpty) { |
There was a problem hiding this comment.
Can we use $label !== null && $label !== '' as condition instead ? We don't need to handle cases about checking emptiness for other kind of values here.
|
Thank you @xabbuh. |
twig_test_empty()andtwig_escape_filter()are deprecated since Twig 3.9