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.

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

Jump to bottom

Parser doesn't allow <body {% block body_attributes %}{% endblock %}> #61

Open
jraller opened this issue Aug 27, 2024 · 6 comments
Open

Parser doesn't allow <body {% block body_attributes %}{% endblock %}> #61

jraller opened this issue Aug 27, 2024 · 6 comments
Labels
Feature: Request Help Wanted Please send PR, I don't know what to do Parser Logic related on how to parse twig template
Milestone
Version 1.0

Comments

@jraller
Copy link

jraller commented Aug 27, 2024

A twig template with

    <body {% block body_attributes %}{% endblock %}>
        {% block body %}
        {% endblock %}
    </body>

in it passes the linter symfony console lint:twig templates, but prettier-plugin-twig's parser reports:

[error] templates/base.html.twig: Error: ERROR: Invalid token
[error] 18 | {% endblock %}
[error] 19 |
[error] > 20 | <body {% block body_attributes %}{% endblock %}>
[error] | ^
[error] 21 | {% block body %}
[error] 22 | {% endblock %}
[error] 23 |
[error]
[error] A tag must consist of attributes or expressions. Twig Tags are not allowed.
[error] at TokenStream.error (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/melody/melody-parser/TokenStream.js:129:24)
[error] at Parser.error (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/melody/melody-parser/Parser.js:452:21)
[error] at Parser.matchAttributes (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/melody/melody-parser/Parser.js:442:22)
[error] at Parser.matchElement (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/melody/melody-parser/Parser.js:333:14)
[error] at Parser.parse (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/melody/melody-parser/Parser.js:207:32)
[error] at Parser.matchElement (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/melody/melody-parser/Parser.js:343:41)
[error] at Parser.parse (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/melody/melody-parser/Parser.js:207:32)
[error] at Object.parse (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/@zackad/prettier-plugin-twig/src/parser.js:82:24)
[error] at parse4 (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/prettier/index.mjs:20685:24)
[error] at async coreFormat (file:///C:/Users/Jason/AppData/Roaming/npm/node_modules/prettier/index.mjs:21146:7)

usage for this when calling the body_attributes block allows for passing in class and data-controller as follows:

{% block body_attributes %}
    class="bg-gray-100 font-sans leading-normal tracking-normal mt-12" data-controller="style"'%}
{% endblock %}
@jraller
Copy link
Author

jraller commented Aug 27, 2024

It looks like this dates back to trivago#60

@zackad
Copy link
Owner

zackad commented Nov 18, 2024

I'm not sure how to tackle this issue. Adding support for twig tag inside html element might not be complicated. Example

<div {% block class %}class="hidden"{% endblock class %}>
</div>

On the other hand, if we need to imlement support for twig tag inside attribute value, example

<div class="{% block class %}hidden{% endblock class %}">
</div>

the parser logic will be significantly more complex.

Ideally, all twig features should be supported. However we lack the skill to actually implement this features.

Should we partially implement this feature, allow twig tag inside html element? Skipping twig tag inside html attribute value. Or don't bother to add this feature if not fully implemented? Waiting for feedback.

@Levdbas
Copy link

Levdbas commented Nov 24, 2024

I think there are way more elegant solutions this, like using an extension like: https://github.com/parisek/twig-attribute. And then just setting the variable that sets all the attributes via the child template.

@zackad
Copy link
Owner

zackad commented Nov 26, 2024

TBH, I'm not a big fan of using twig tag inside html element. There's other solution that I think more elegant, such as using extension like example above or using something like twig-component.

In my experience, the usage of block expression on html attribute can be achived using ternary expression (which this plugin supported) in some cases. And so far I can live without this feature.

Unless there's strong opinion that this should be implemented, I don't want to add this feature. Feel free to send pull request. Maybe I can work from there.

Thanks.

@zackad zackad added the Help Wanted Please send PR, I don't know what to do label Nov 26, 2024
@renestalder
Copy link

renestalder commented Nov 26, 2024 •
edited
Loading

I don't know what the non-existing specification (not even joking here) of Twig says about this. But in my advanced experience building with Twig, using block inside the HTML tags was never a solution for something that I couldn't have done otherwise. I saw this in use for the first time when reviewing Twig code written by sole PHP developers. I'm not a PHP developer, so I might not necessarily understand the background to this approach. I only write frontend code and I do this most of the time in Twig. So, I might be able to give some inputs on how this can be done in a way it is more readable anyway.

Here are the (mostly better) alternatives for this, from my point of view:

  • Using variables: Simply passing this down as a variable and rendering it with the |raw pipe (in case you pass the whole attribute/value string and not only the value), and if you want so for security, the |e pipe, is more readable in any case.
  • Using the block function: For example in your case {{ block('class') }}.
  • Pass down a key/value structure and add your own function to Twig to render it as an attribute-key/value string. I've done somethig like this with Twig Macros because my limited PHP understanding. But adding a function to Twig is pretty straight forward.
  • Install Symfony extensions: Like in the case of class, the HTML Extras and Extra-Bundles with the html_classes function is the cleanest approach.

Or as @zackad said, the Twig Components from Symfony's UX initiative. In case you are anyway in the Symfony ecosystem.

(my 50 cents as a Twig user using the Prettier formatter)

@ryanleichty
Copy link

Just installed this in order to test it and this is the first thing I ran into. We use twig tags like this everywhere, so converting everything isn't a small task and probably not a goal of ours. I know liquid supports tags like this and they have a prettier plugin as well.

@zackad zackad mentioned this issue Dec 16, 2024
Closed
@zackad zackad added this to the Version 1.0 milestone Dec 16, 2024
@zackad zackad added the Parser Logic related on how to parse twig template label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature: Request Help Wanted Please send PR, I don't know what to do Parser Logic related on how to parse twig template
Projects
None yet
Milestone
Version 1.0
Development

No branches or pull requests
5 participants
@renestalder @jraller @zackad @ryanleichty @Levdbas