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

Scoped styles for <svelte:element> #7443

Closed
sherifsalah opened this issue Apr 11, 2022 · 17 comments · Fixed by #7652
Closed

Scoped styles for <svelte:element> #7443

sherifsalah opened this issue Apr 11, 2022 · 17 comments · Fixed by #7652

Comments

@sherifsalah
Copy link

Describe the problem

Currenlty scoped styling for any dynamic tags generated with svelte:element give the generic warning "Unused CSS selector", not really sure if that's a bug or it should be added as a feature!

Describe the proposed solution

add support for scoped styles with svelte:element

Alternatives considered

currently the only solution to fix that problem is using global styles

Importance

would make my life easier

@baseballyama
Copy link
Member

Could you provide us REPL?
I tried to reproduce it on REPL but I couldn't.
https://svelte.dev/repl/7e4c14c9a5ce430488e8bdc331a27976?version=3.47.0

@SarcevicAntonio
Copy link
Member

The issue is, that can't use the generic tag selectors like this:
https://svelte.dev/repl/23d982fc6f4f4f06a6aa227860fa2d84?version=3.47.0

Your workaround with using classes works for me right now tho :D

@baseballyama
Copy link
Member

Ah I got it. Thank you!

I think this workaround also useful.
https://svelte.dev/repl/3832a541ea24479581c97bfd0a934b7b?version=3.47.0

@tanhauhau
Copy link
Member

I guess when we try to see if the selectors match any element, we can make svelte:element to match all element selector ?

<div>
  <svelte:element class="a" this={...}>
     <span />
  </svelte:element>
</div>

<style>
   /* used selectors */
   h1 { }
   p span { }
   ul.a { }
   div > b > span { }
   div > i { }

   /* unused selectors */
   h2 div { }
   h3.b { }
</style>

@baseballyama
Copy link
Member

I think the compiler disable CSS removal process if using dynamic class name.
Should we do a similar process for <svelte:element>?
(If <svelte:element> is there, we don't remove css which specified tag name like h1 { color: blue })

image

@baseballyama
Copy link
Member

@tanhauhau

oops, we are posting at the exact same time lol.
Yes, I think we can implement such a process.

Can I try?

@gtm-nayan
Copy link
Contributor

A thought,
Making special considerations for things like svelte:element might get out of hand as more features are added.

How about introducing a way for users to tell the compiler to not purge certain selectors? Essentially what people do with :global() but it'd still keep scoping. This would solve the problem at hand right? While also unlocking more possibilities in userland. An example possibility

There's already an issue proposing something similar, will link it and carry over the discussion there.

@sherifsalah
Copy link
Author

A thought, Making special considerations for things like svelte:element might get out of hand as more features are added.

How about introducing a way for users to tell the compiler to not purge certain selectors? Essentially what people do with :global() but it'd still keep scoping. This would solve the problem at hand right? While also unlocking more possibilities in userland. An example possibility

There's already an issue proposing something similar, will link it and carry over the discussion there.

I love this idea, maybe making a special keyword like :global() something like :keep() or whatever will fix all the issues of purging what the compiler sees unused css

@baseballyama
Copy link
Member

There's already an issue proposing something similar, will link it and carry over the discussion there.

Could you please tell me where is that?

And personally, it sounds like it is transferring the responsibilities of the Svelte compiler to userland regarding purging CSS.
It seems to me that as long as Svelte has the ability to purge CSS, it has the responsibility to handle all cases.

@SarcevicAntonio
Copy link
Member

I'd also love to opt out of purging (while keeping scoping).

@gtm-nayan
Copy link
Contributor

Found it #5804

And personally, it sounds like it is transferring the responsibilities of the Svelte compiler to userland regarding purging CSS.

It is kinda, (technically it'd be the responsibility of not purging CSS), IMO it's fine in this case, we can't expect Svelte to bend for every possible scenario that people can conjure.

Forgive me if I misunderstood the current proposed solution to this issue, my interpretation was that if the component has a dynamic class name somewhere then Svelte will ignore unused CSS in that component? Which I think opens up more possibilities for pitfalls, and an explicit opt in to keep styles would be a lot better here.

@tanhauhau
Copy link
Member

Can I try?

@baseballyama yup go for it 🚀

@baseballyama
Copy link
Member

There is actual problem here regarding CSS purging, so I think we need to discuss about it.
And I think I should wait to handle this until then.

#5804 (comment)

@sherifsalah
Copy link
Author

@tanhauhau what about the suggestion of making something like :global keyword to keep classes that we don't wanna purge, this will solve another issue of keeping conditional classes as well as example:
repl
condtional class red will be purged so when the condition changes class red won't be found, should we create a new issue?

@tanhauhau
Copy link
Member

the repl didnt use <svelte:element> at all, is this a similar issue or a different issue?

@sherifsalah
Copy link
Author

the repl didnt use <svelte:element> at all, is this a similar issue or a different issue?

Different issue i guess, but it just popped up with this discussion, the idea of preventing svelte from purging certian classes (conditional classes), same as using :global but keeping it scoped as example: :keep(.class) {color: red}

@Conduitry
Copy link
Member

This is supported now in 3.51.0 - https://svelte.dev/repl/23d982fc6f4f4f06a6aa227860fa2d84?version=3.51.0

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 a pull request may close this issue.

6 participants