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

Making the Lexer initialize itself lazily, to avoid loading the extension set early #3837

Merged
merged 1 commit into from
May 3, 2023

Conversation

weaverryan
Copy link
Contributor

@weaverryan weaverryan commented May 3, 2023

Hi!

Over on Symfony UX, we are pushing Twig in some crazy, "unconventional" ways by overriding the lexer to add a new HTML-like syntax - e.g. <twig:Alert type="success"> - https://github.com/symfony/ux/blob/2.x/src/TwigComponent/src/Twig/ComponentLexer.php

Normally, the internal Lexer is not instantiated until it's needed - i.e. when a template is rendered. However, when overriding the lexer, our ComponentLexer needs to be instantiated early and set onto the Environment. And since it needs to extend Lexer, Lexer::__construct() is called much earlier than normal. One line in the constructor -

'operator' => $this->getOperatorRegex(),
- causes the "extension set" to be loaded inside Environment. Some applications rely on being able to set extra Twig extensions after Environment is created, but before a template is rendered and our custom lexer currently breaks that.

So, the suggestion is to make the work inside __construct() done later. Everything is private, so I believe this is safe.

Cheers!

@weaverryan
Copy link
Contributor Author

fabbot failure is on an existing line - so leaving that alone

src/Lexer.php Show resolved Hide resolved
@fabpot
Copy link
Contributor

fabpot commented May 3, 2023

Symfony UX supports Twig 2.x, so maybe we should fix this on 2.x instead?

@javiereguiluz
Copy link
Contributor

@weaverryan we hit this bug this morning when upgrading an app (which uses UX) to Symfony 6.3. We call $twig->addGlobal() in a listener and we saw the error message "Unable to add global "foobar" as the runtime or the extensions have already been initialized.".

It was really tricky to debug this ... but Nicolas point me to here 🙌 So, thanks for taking care of this.

@fabpot fabpot changed the base branch from 3.x to 2.x May 3, 2023 17:47
@fabpot
Copy link
Contributor

fabpot commented May 3, 2023

Thank you @weaverryan.

@fabpot fabpot merged commit 72230e2 into twigphp:2.x May 3, 2023
@weaverryan weaverryan deleted the lazy-lexer branch May 3, 2023 18:37
@fabpot
Copy link
Contributor

fabpot commented May 3, 2023

@weaverryan New Twig versions released.

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

Successfully merging this pull request may close these issues.

None yet

4 participants