Skip to content

[TwigHooks] Fix Twig 3 deprecations #311

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.

Sign up for GitHub

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

Open
acrobat wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[TwigHooks] Fix Twig 3 deprecations #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 34 additions & 13 deletions src/TwigHooks/src/Twig/Node/HookNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,51 @@
namespace Sylius\TwigHooks\Twig\Node;

use Sylius\TwigHooks\Twig\Runtime\HooksRuntime;
use Twig\Attribute\YieldReady;
use Twig\Compiler;
use Twig\Node\Expression\ArrayExpression;
use Twig\Node\Node;

#[YieldReady]
final class HookNode extends Node
{
public function __construct(
Node $name,
?Node $context,
bool $only,
int $lineno,
?string $tag = null,
) {
parent::__construct(
[
'name' => $name,
'hook_level_context' => $context ?? new ArrayExpression([], $lineno),
],
[
'only' => $only,
],
$lineno,
$tag,
);
if (\func_num_args() > 4) {
trigger_deprecation('sylius/twig-hooks', '0.10.0', \sprintf('The "tag" constructor argument of the "%s" class is deprecated and ignored (check which TokenParser class set it to "%s"), the tag is now automatically set by the Parser when needed.', static::class, func_get_arg(4) ?: 'null'));
}

if (class_exists(\Twig\Node\Expression\FunctionNode\EnumCasesFunction::class)) {
parent::__construct(
[
'name' => $name,
'hook_level_context' => $context ?? new ArrayExpression([], $lineno),
],
[
'only' => $only,
],
$lineno,
);
} else {
Copy link
Member

@loic425 loic425 Jan 26, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer an early return style.
But maybe in the opposite way to remove the code easily after bumping the package.
That's only a suggestion, cause it's ok like this too.

Copy link
Author

@acrobat acrobat Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm also using the early return in regular code but mostly when doing deprecations like this I structure it in an if/else to make more clear what to remove when going to the next major or when dependencies are bumped

Copy link
Contributor

@NoResponseMate NoResponseMate Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also prefer reversing the conditions and early returning, in other places as well, but ultimately think it's fine as is

Copy link
Member

@loic425 loic425 Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acrobat Ok, could you try to apply the early returning that will contain the code to remove?

// Remove when twig < 3.12 support is dropped
$tag = func_get_arg(4);

parent::__construct(
[
'name' => $name,
'hook_level_context' => $context ?? new ArrayExpression([], $lineno),
],
[
'only' => $only,
],
$lineno,
$tag,
);
}
}

public function compile(Compiler $compiler): void
Expand All @@ -49,7 +70,7 @@ public function compile(Compiler $compiler): void
HooksRuntime::class,
))->raw("\n");

$compiler->raw('echo $hooksRuntime->renderHook(');
$compiler->raw(sprintf('%s $hooksRuntime->renderHook(', class_exists(YieldReady::class) ? 'yield' : 'echo'));
$compiler->subcompile($this->getNode('name'));
$compiler->raw(', ');
$compiler->subcompile($this->getNode('hook_level_context'));
Expand Down
33 changes: 31 additions & 2 deletions src/TwigHooks/src/Twig/TokenParser/HookTokenParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use Sylius\TwigHooks\Twig\Node\HookNode;
use Twig\Node\Node;
use Twig\Node\Nodes;
use Twig\Token;
use Twig\TokenParser\AbstractTokenParser;

Expand All @@ -26,11 +27,21 @@ public function parse(Token $token): Node
{
$lineno = $token->getLine();
$stream = $this->parser->getStream();
$hooksNames = $this->parser->getExpressionParser()->parseExpression();
if (method_exists($this->parser, 'parseExpression')) {
$hooksNames = $this->parser->parseExpression();
} else {
// Remove when Twig 3.21 support is dropped
$hooksNames = $this->parser->getExpressionParser()->parseExpression();
}

$hookContext = null;
if ($stream->nextIf(Token::NAME_TYPE, 'with')) {
$hookContext = $this->parser->getExpressionParser()->parseMultitargetExpression();
if (method_exists($this->parser, 'parseExpression')) {
$hookContext = $this->parseMultitargetExpression();
} else {
// Remove when Twig 3.21 support is dropped
$hookContext = $this->parser->getExpressionParser()->parseMultitargetExpression();
Copy link
Author

@acrobat acrobat Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is deprecated and no alternative is provided. In twig the usage of this method was replaced by a private method, so I'm applying the same logic here

twigphp/Twig@42c82e1#diff-c4fd0102f3bc6095b6b2c644eed251b98a65e46d2ed80329d97e9da3e97b984f

}
}

$only = false;
Expand All @@ -40,11 +51,29 @@ public function parse(Token $token): Node

$stream->expect(Token::BLOCK_END_TYPE);

if (class_exists(\Twig\Node\Expression\FunctionNode\EnumCasesFunction::class)) {
return new HookNode($hooksNames, $hookContext, $only, $lineno);
}

// Remove when twig < 3.12 support is dropped
return new HookNode($hooksNames, $hookContext, $only, $lineno, $this->getTag());
}

public function getTag(): string
{
return self::TAG;
}

private function parseMultitargetExpression(): Nodes
{
$targets = [];
while (true) {
$targets[] = $this->parser->parseExpression();
if (!$this->parser->getStream()->nextIf(Token::PUNCTUATION_TYPE, ',')) {
break;
}
}

return new Nodes($targets);
}
}