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

Refactor gradient implementation to work around prettier/prettier#17058 #16072

Merged
merged 5 commits into from
Jan 30, 2025

Conversation

RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented Jan 30, 2025

This PR fixes an issue where tools like Prettier remove important trailing commas in CSS variables, making gradients invalid.

We encoded the , in the --tw-gradient-position to ensure that if the var(--tw-gradient-position) is used, that the , was there. And if the variable was not used that we didn't end up with a double ,, rendering the gradient invalid.

However, when running Prettier (there might be other tools that do this as well), the trailing comma in the --tw-gradient-position was removed which made the entire gradient invalid. E.g.:

  .bg-gradient-to-r {
-   --tw-gradient-position: to right in oklab,;
+   --tw-gradient-position: to right in oklab;
    background-image: linear-gradient(var(--tw-gradient-stops));
  }

Notice how the , is removed.

This PR fixes that, by moving the , to where the variable is being used. The only side effect is that we have to guarantee that the --tw-gradient-position is always present. In our testing (and using UI tests) this should be the case.

Related Prettier issue: prettier/prettier#17058

Fixes: #16037

We encoded the `,` in the `--tw-gradient-position` to ensure that _if_
the `var(--tw-gradient-position)` is used, that the `,` was there. And
if the variable was _not_ used that we didn't end up with a double `,,`
rendering the gradient invalid.

However, when running prettier (there might be other tools that do this
as well), the trailing comma in the `--tw-gradient-position` was removed
which made the entire gradient invalid. E.g.:
```diff
  .bg-gradient-to-r {
-   --tw-gradient-position: to right in oklab,;
+   --tw-gradient-position: to right in oklab;
    background-image: linear-gradient(var(--tw-gradient-stops));
  }
```
Notice how the `,` is removed.

This commit fixes that, by moving the `,` to where it is used. The only
side effect is that we have to guarantee that the
`--tw-gradient-position` is always present. In our testing (and using UI
tests) this should be the case.

Thus, hopefully this fix is safe in all situations.
@RobinMalfait RobinMalfait requested a review from a team as a code owner January 30, 2025 15:03
@adamwathan adamwathan changed the title Make gradients implementation more stable Refactor gradient implementation to work around prettier/prettier#17058 Jan 30, 2025
@RobinMalfait RobinMalfait enabled auto-merge (squash) January 30, 2025 15:44
@RobinMalfait RobinMalfait merged commit 2242941 into main Jan 30, 2025
5 checks passed
@RobinMalfait RobinMalfait deleted the fix/issue-16037 branch January 30, 2025 15:46
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.

[v4] Background gradient does not display (generated css has syntax error)
2 participants