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

Improve handling of whitespace and duplicate class removal #276

Merged
merged 12 commits into from
May 31, 2024

Conversation

thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented May 31, 2024

This PR does a handful of things:

Tweak whitespace handling inside concatenation expressions

For example, in the following code:

<div className={" flex " + borderClass + " underline sm:block "} />

After formatting the output is

- <div className={" flex " + borderClass + " underline sm:block "} />
+ <div className={"flex" + borderClass + "underline sm:block"} />

However, this results in invalid class names. We'll now only remove the whitespace on one side of each of the strings (depending on if it's the start or not), so the output will be:

- <div className={" flex " + borderClass + " underline sm:block "} />
+ <div className={"flex " + borderClass + " underline sm:block"} />

This means you're less likely to need to add extra whitespace only strings or disable whitespace removal to ensure the output stays functionally the same.

Add tailwindPreserveDuplicates option

Normally removing duplicate classes (whether known or unknown) is a safe operation to do. However, when using a templating language like Fluid or Blade it's possible to have "duplicate" classes that are important because they're compiler directives. The problem is that Prettier only considers the file to be HTML so we don't know about these directives.

For example, in Fluid, you might have something like this:

<div
  class="
    {f:if(condition: isCompact, then: 'grid-cols-3', else: 'grid-cols-5')}
    {f:if(condition: isDark, then: 'bg-black/50', else: 'bg-white/50')}
    grid gap-4 p-4
  "
>
</div>

Our plugin sees {f:if(condition: as a class, isCompact, as a class, then: as a class, etc… but doesn't know that they are directives so they are treated like unknown classes and moved to the front of the class list in their original order.

Further, when we remove duplicate classes the second then:, else:, and {f:if(condition:, etc… are all removed, resulting in invalid Fluid code:

<div
  class="
    {f:if(condition: isCompact, then: 'grid-cols-3', else: 'grid-cols-5')}
    isDark,  'bg-black/50', 'bg-white/50')}
    grid gap-4 p-4
  "
>
</div>

To fix this, we've added a new tailwindPreserveDuplicates option that prevents this behavior. It is false by default but can be enabled if you're using a templating language where this is a problem:

{
  // … your config
  "tailwindPreserveDuplicates": true
}

Fixes some bugs

  • Angular expressions that had either duplicate classes or whitespace removed could result in mangled output. This is now fixed.
  • Removal of duplicate classes and whitespace has been disabled for Svelte and Liquid files. This is due to limitations in their respective plugins that make it difficult to change the length of the resulting code (in some situations) and still output valid code.

Fixes #273

This is important when using templating languages
We’d have to modify the start/end locations of many nodes in the AST. Technically, only AST nodes appearing _after_ the string on the same line but that might actually be sibling nodes and ancestor nodes. It’s a better option for now to disable it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removal of duplicate classes also removes inline conditions
2 participants