symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
816 stars 299 forks source link

Twig parser breaks files with '{#' in strings in 2.8.1 #880

Open Lustmored opened 1 year ago

Lustmored commented 1 year ago

I have a template (Extending other) with the following translated message in a block, in ICU message format:

{{ '{visited, plural, =0 {Nobody visited this room yet} one {1 person visited this room} other {# people visited this room} }'|trans({ 'visited': 6 }) }}

Twig components update 2.8.1 broke the template with linter error:

A template that extends another one cannot include content outside Twig blocks. Did you forget to put the content inside a {% block %} tag?

I have narrowed it down to {# existing in a string in a template - looks like some code comments removing code got bugged. Changing the string to:

{{ '{visited, plural, =0 {Nobody visited this room yet} one {1 person visited this room} other { # people visited this room} }'|trans({ 'visited': 6 }) }} or {{ "{visited, plural, =0 {Nobody visited this room yet} one {1 person visited this room} other {\# people visited this room} }"|trans({ 'visited': 6 }) }}

resolves the issue, but it definitely is an unexpected regression.

WebMamba commented 1 year ago

Yes good catch! The bug is from this line: https://github.com/symfony/ux/blob/feb8a2744d682c59e7f5cc54892343568eb7e5e5/src/TwigComponent/src/Twig/TwigPreLexer.php#L44 Hummmm, I'm not sure how to resolve it. Maybe if we are inside {{ }} we don't look for {# comment. But I am not sure if this is the only place where we should ignore comments 🤔

weaverryan commented 1 year ago

Yup, look tricky to solve. We need to rewrite our pre lexer to have a proper token system. Or, perhaps even more useful, change it to a "post lexer":

A) Call the normal Lexer class - it returns a TokenStream() filled with a Token[] array. B) Loop over that Token[] array. Look specifically for, I believe, Token objects with the type "text" - these would be normal, non-Twig code. We would look for opening <twig: specifically inside of these ONLY. This would already fix the commenting problem, as Twig already excludes comments from the Token[] array C) Once we find an open tag, we would start our own new Token onto the stream to represent this - basically the token equivalent of what Twig normally adds for {% component.

I'm leaving out some details... and it'll be tricky. But we'll be relying on Twig's lexing a lot more, which is the only way to get this right. Even when we come to :attribute="dynamicVar" situations, we could, I believe, isolate the dynamicVar and send just THAT into the an instance of the Lexer for it to tokenize it.

carsonbot commented 4 months ago

Thank you for this issue. There has not been a lot of activity here for a while. Has this been resolved?

carsonbot commented 4 months ago

Could I get a reply or should I close this?

carsonbot commented 3 months ago

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!