Add Prettier for Twig and Tailwind CSS to improve coding standards

Just capturing my comments from Slack, this seems to undo the grouping that we introduced in #3556312: Group Tailwind classes in Twig for readability so just want to make sure we are all on the same page. I've asked @balintbrews to take a look and let us know what he recommends.

I realized that the way we added grouping in #3556312: Group Tailwind classes in Twig for readability doesn't communicate to prettier-plugin-tailwindcss where it should look for class names to sort: Prettier runs static analysis only, so it can't figure out that what we define in a variable will end up in the class attribute. I also missed that the class names that we would like to group together should be added as a single string rather than separate strings in an array. So what we're currently seeing is just @zackad/prettier-plugin-twig formatting the array, and prettier-plugin-tailwindcss not recognizing that it needs to do something.

→ The fix is to move the array inside the class attribute, and keep class name groups in single strings: da5afd69.

There would be a much nicer way to do this, and that is making use of the html_cva function, and setting "tailwindFunctions": ["html_cva"] in prettierrc.json. Two for the price of one, we'd get sorting, because that would tell prettier-plugin-tailwindcss to sort inside the arguments of that function call, and html_cva would give us a really elegant way to handle variations in the same component, so we could abandon conditionally concatenating the class names. (We already bundle CVA for Code Components in Canvas to encourage this pattern.)

That all sounds awesome, but there are complications:

1. We'd need to find/write a Drupal module that exposes html_cva from twig-extra-bundle, and make Mercury dependent on that module.
2. Mercury's Storybook setup doesn't use Drupal for rendering (which could be done with the Storybook module), so that environment wouldn't know about html_cva. I'm not sure what it would take to make Twing aware of html_cva, and inject the implementation of the CVA JavaScript library for the rendering. Quickly scanning Twing's documentation, it doesn't seem like it's easily pluggable. So I think with this we're hitting one of the limitations of not using Drupal for rendering the stories.

@balintbrews I see, thank you for sorting that! So the grouping is still something we do ourselves, but if done this way, prettier will not undo it.

I think CVA sounds like a great option, but the refactoring probably is out of scope for 1.0.

Just one other question, should we commit the other prettier fixes all at once? There are changes to almost every Twig file when I run it locallly.

So the grouping is still something we do ourselves, but if done this way, prettier will not undo it.

Right, the grouping should be subject to opinion. Sometimes you don't even want it, other times you would want to group based on the needs of the current component.

I think CVA sounds like a great option, but the refactoring probably is out of scope for 1.0.

👍

Just one other question, should we commit the other prettier fixes all at once? There are changes to almost every Twig file when I run it locallly.

Yes, but we should also make sure that a Prettier check runs on CI, otherwise it's easy to commit without formatting. We would need scripts in package.json, e.g.:

{
  "scripts": {
    "format": "prettier --write .",
    "format:check": "prettier --check .",
  },
}

Then we may want to add a .prettierignore file to exclude certain files. I was going to add all these, but I got confused, so I stopped to ask some questions.

1. There are scripts in package.json to lint the JavaScript code with ESLint, and the CSS code with Stylelint. But I don't see config files for these. I spotted eslintConfig in package.json, but it's not being picked up when I run pnpm lint:js, and it fails with an error message telling us that it is missing the config file. I get a similar error when running pnpm lint:css.
2. There is no CI job for running the linting. I was going to just add a new one for the Prettier check, but since there is no job for running the linting currently, I thought I'd ask why that is.

We need to get these tighten up, because adding formatting with Prettier while not doing any linting and format checks on CI will be very disruptive. Imagine I want to submit an MR, and I want to format and lint my code, but I also get other files changed, because those may have been committed without any quality check.

Turns out ESLint and Stylelint have never been set up for the project. Since both exclude any stylistic rules anyway in favor of Prettier, I just went ahead and added scripts to package.json to work with Prettier, and added the format check as part of the newly added lint job.

I then reformatted all files. Many of them were producing parsing errors due to techniques that static analysis tools can't work with. Fixing these also result in better readability.

Here are the types of errors I had to fix manually (marked with "⚠️ Refactored" comment in GitLab):

1. Even though this is how it's done in Drupal core's template files, we can't append {{ attributes }} to HTML tags without a space. For example, instead of <div{{ attributes }}>, we need to have <div {{ attributes }}>. I won't highlight all the places where I fixed this, because this is a trivial change.
2. Two files, components/button/button.twig and components/blockquote/blockquote.twig added attributes with inline Twig control structures ({% if %}), and/or had them directly within HTML tag attributes. I refactored those to assign values to variables.
3. Many files had opening and closing tags split conditionally. For example:
   

   {% if url is not empty %}
     <a href="{{ url }}">    ← Opening tag in one conditional
   {% endif %}
     <p>...</p>              ← Content outside
   {% if url %}
     </a>                    ← Closing tag in another conditional  
   {% endif %}
   

   This will never be static analysis friendly. But a better reason is that it's also not very readable. A similar issue is using dynamic tag names, like <h{{ heading_level }}>. I refactored the following files and tested manually in Canvas (not in Storybook):

   • components/card-profile/card-profile.twig
   • components/collapsible-section/collapsible-section.twig
   • components/heading/heading.twig
   • components/menu-footer/menu-footer.twig
   • components/menu-footer/menu-footer~main.twig
   • components/menu-utility/menu-utility.twig
   • components/menu-utility/menu-utility~main.twig
   • components/pager/pager.twig
   • templates/views/views-view.html.twig

The followings don't work in Canvas, so I didn't test them:

• components/menu-footer/menu-footer.twig (and components/menu-footer/menu-footer~main.twig)
• components/menu-utility/menu-utility.twig (and components/menu-utility/menu-utility~main.twig)
• components/pager/pager.twig — This component shows up in Canvas, but it renders nothing, and it has no props or slots, so I don't understand how it's supposed to work.

I manually compared how the refactored components render in Canvas to how they do in the 1.x branch, but additional testing by someone who has spent more time with them would be appreciated.

Example pipeline for a commit violating Prettier formatting rules: https://git.drupalcode.org/issue/mercury-3556548/-/pipelines/657011.

Mercury's Storybook setup doesn't use Drupal for rendering (which could be done with the Storybook module), so that environment wouldn't know about html_cva. I'm not sure what it would take to make Twing aware of html_cva, and inject the implementation of the CVA JavaScript library for the rendering. Quickly scanning Twing's documentation, it doesn't seem like it's easily pluggable.

In general I hate to tie a design system to a specific CMS, and to require that CMS to be up and running locally to make local changes. In an official Drupal project like this, though, it might not be terrible. However, we already have a few custom Twing filters and functions (see filters.js and functions.js in vitePlugins/twingCustoms) and it's not hard at all to use any old JS function as one. So if the PHP html_cva function conforms well to the original, we can just import that NPM package and glue it in.

Also, the tailwind_merge filter mentioned in the Twig docs for html_cva is derived from an original JS version, so we can have that easily, too (though the PHP one doesn't seem to support Tailwind v4 yet, so that may be a blocker to this whole effort, depending on how vital tailwind_merge is in practice).

Everything done in this MR seems reasonable to me, but it does need a quick manual test from Pam, in the context of Byte, to make sure we didn't accidentally break any styling.

RTBC from a code review standpoint, but should not be committed until Pam confirms it's good.

Taking a look now...