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

Not removed "unused" class from CSS if it used by component #10618

Closed
alkorlos opened this issue Feb 23, 2024 · 4 comments
Closed

Not removed "unused" class from CSS if it used by component #10618

alkorlos opened this issue Feb 23, 2024 · 4 comments

Comments

@alkorlos
Copy link

alkorlos commented Feb 23, 2024

Describe the problem

In Svelte 5, it's easy to add the class prop to a component using runes:

<!-- Input.svelte -->
<script>
	const { 'class': className } = $props();
</script>

<input class={className} />

Thank you, very convenient!

However, in Svelte 5, component classes are still removed from CSS as unused:

<!-- App.svelte -->
<script>
	import Input from './Input.svelte';
</script>

<Input class="form__input" />

<style>
	.form__input {
		background: #cccccc;
	}
</style>

.form__input will be removed, even when it's actually used. One workaround is to replace .form__input to :global(.form__input), but this isn't convenient and it's generally preferable to avoid using global styles.

Expected behavior — not be removed .form__input and add to it unique HASH identifier.

Describe the proposed solution

Automatically detect usage class attribute in component and handle its behavior in a special way.

Alternatively, add $propsClass(); to work with class:

<script>
	const { 'class': className } = $propsClass();
</script>

Something like @Mooncat25 offered

There may be other approaches available, but this functionality has been very helpful.

Importance

would make my life easier

@alkorlos alkorlos changed the title Not removed "unused" class if it used by component Not removed "unused" class from CSS if it used by component Feb 23, 2024
@mwc
Copy link

mwc commented Feb 23, 2024

in fact, since [email protected], tailwindcss dark mode(based on class strategy) will also not work, because the .dark class will be removed.

for example:

<h3 class="text-black dark:text-white">title</h3>

the dark: variant will generate this css:

:is(.dark .dark\:text-white) {
    --tw-text-opacity: 1;
    color: rgb(255 255 255 / var(--tw-text-opacity));
}

but,,,, current component does not know what .dark is.

@Conduitry
Copy link
Member

A class prop on a component isn't a class on an element. I don't think we should have a special case for that prop name where we're treating it as though it were an element. There's also no guarantee what the child component is going to do with that prop, or that it's going to end up on a top-level element in that component so the CSS selectors would match the elements you want.

@alkorlos
Copy link
Author

And one more reason not to do this:

Components should be able to decide for themselves which styles can be controlled from 'outside'

Component styles

The proposed change I suggested will violate this idea. This issue has been discussed five years ago, but Svelte requires a different approach.

I've always made components as independent as possible, but things like hiding/showing components at different resolutions were left to the parent. Perhaps in this case, it's better to move this functionality to the class: directive and include it as a modifier, like this. class prop in Svelte not need.

Alright, thank you!

@Mooncat25
Copy link

Originally I have already given up replying the rejections on this type of proposal, especially when I see "class is just a normal prop and we don't treat it in a special case".

Come on, I have seen it long time ago, and this is the reason why we are here to make suggestion: please treat class as CSS class, or add the ability to tell Svelte to treat any prop as CSS class so the linked class won't be treated as unused and get removed.

Now I'm here because I just watched an interview of Rich Harris and the intro pretty much summarized my feeling on this ticket.

You know a lot of people candidly just don't have time for the front-end industries bullshit. Oh which library am I supposed to use for state management? No! It should be built-in!

To me, CSS handling in Svelte is always an unfinished feature. We can use it for simple cases. But when we want to go a bit advanced, we have to shift to another way, like using external stylesheet. Personally I use modular Scss. It works very well. But then it is not Svelte anymore.

The things suggested by @alkorlos works in intermediate cases, where you already know which properties the end users would like to override. It works if you are the component author and also the end user. But in reality, it doesn't work all the time and CSS class will always be preferred.

Svelte is a good framework. But this tiny bit of oversight makes it imperfect. Not saying I will give up using Svelte just because of this, but I wish someday we will see better component CSS class handling in the future. 🙏

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

No branches or pull requests

4 participants