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

Svelte Scoped doesn't work with CLSX #3631

Closed
4 tasks done
ZeroPie opened this issue Mar 17, 2024 · 9 comments
Closed
4 tasks done

Svelte Scoped doesn't work with CLSX #3631

ZeroPie opened this issue Mar 17, 2024 · 9 comments

Comments

@ZeroPie
Copy link

ZeroPie commented Mar 17, 2024

UnoCSS version

0.58.6

Describe the bug

using clsx breaks the classes

Reproduction

https://stackblitz.com/edit/unocss-unocss-nfuydr?file=src%2Froutes%2F%2Bpage.svelte

System Info

No response

Validations

@Simon-He95
Copy link
Contributor

I don’t know if this is a problem with unocss. It seems to be caused by Svelte’s class processing rules. Static classes will be processed, but dynamic classes will be retained. The resulting difference
image

@henrikvilhelmberglund
Copy link
Contributor

Do you get the same result in recent (not 5) Svelte?

@ZeroPie
Copy link
Author

ZeroPie commented Mar 18, 2024

I do, i updated to svelte 4.2.12 and it's still there
https://stackblitz.com/edit/unocss-unocss-jzqdzz?file=package.json

@jacob-8
Copy link
Contributor

jacob-8 commented Mar 22, 2024

I've only ever seen clsx used in Svelte by people coming from the React way of writing things. As I see it, it's a misunderstanding of how to write Svelte code. The Svelte way is to write conditional css classes is class:text-red-500={flag} and not class={clsx(flag && "text-red-500")}. Svelte-Scoped supports all the encouraged ways of writing classes. As well, even though I personally don't recommend it because it makes code harder to read, we also support using inline expressions like class="bg-red {flag ? 'text-white' : 'text-yellow'}". Svelte's ability to simply throw anything inside of brackets obviates the need for clsx.

I'm closing this as we do not need to nor want to encourage use of clsx in Svelte projects. Please do continue the conversation here however, if you have a compelling use case for something you can do using clsx that is not possible using any of the normal Svelte class writing methods nor using a simple style block for the extremely complicated scenarios. What I mean is something like this:

<div 
 class:one-set={flag} 
 class:the-other-set={!flag} />

<style>
  .one-set {
   --at-apply: bg-red-100 underline font-semibold and twenty more classes...;
 }
 .the-other-set {
   --at-apply: bg-blue-100 text-xs opacity-50;
 }
</style>

@ZeroPie
Copy link
Author

ZeroPie commented Mar 27, 2024

@jacob-8 i laughed, i do indeed come from the React world.

The more you know :)
Is this a very obvious thing?
Because a bunch of tutorials/results on google and even some libraries do include and recommend clsx usage.

Maybe losing a line in the docs would help someone in the future?

Thank you.

@jacob-8
Copy link
Contributor

jacob-8 commented Mar 27, 2024

@ZeroPie thanks for the good-natured response to my thoughts. Yes, I'll add a note to the docs as you're not the first one thinking to use clsx here.

React is a mysterious and wonderful beast. I feel like they gave everyone a gift in pushing forward a way of doing web development based around components. But at the same time, I feel they also introduced a lot of conventions that were necessary at the time but now are no longer necessary (and a little awkward, like writing style classes using using camelCase) as Vue, Svelte, and a bunch of others have smoothed off a lot of rough edges. Everyone is now innovating and copying each others' best work in step. It's pretty neat to see.

@henrikvilhelmberglund
Copy link
Contributor

Svelte is still missing a good way to have multiple classes for a certain condition though, class: for each class is kind of clunky and it doesn't work at all if you want the same class for different conditions, at which point you need to edit the condition like {a && b} which is just annoying.

I would personally like something like `classes:"utilities separated by spaces here"={condition}. Hopefully something like this can be added sometime in Svelte 5.

@jacob-8
Copy link
Contributor

jacob-8 commented Mar 29, 2024

I would personally like something like `classes:"utilities separated by spaces here"={condition}. Hopefully something like this can be added sometime in Svelte 5.

You could try creating an RFC proposal for such if you want. It may get traction.

For others following @henrikvilhelmberglund pointed out in a prior issue that the svelte-multicssclass Vite plugin can help with this in lieu of it being provided by the framework: #2322

@ZeroPie
Copy link
Author

ZeroPie commented May 20, 2024

@henrikvilhelmberglund @jacob-8
Thank you for the exchange.
I have something to share now that i have been using {$$props.class} to pass classes from parent to child.
I have run into a problem at times, which is ${props.class} won't necessarily have higher prio than the tailwind classes already present in the children component. So i have to prefix them with ! in order to override children utility classes.

How do you guys deal with it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants