sveltejs/svelte

Unused css selector warning - disable specific parts?

maxmarkus opened this issue · 14 comments

Hello,
first of all, thanks for this great piece of software, appreciate working with it!

I wonder if it is possible to disable unused css checks for specific parts, similar to eslint:disable:nounsed

Adding some classes only programmatically throws a warning when bundling, as it expects all classes to exist in the template. Is there a way around it?

Best, Markus

The recommendation is not to add and remove classes programmatically, for a couple of reasons:

  • Svelte can only encapsulate CSS that it knows about
  • Mixing declarative and imperative DOM manipulation gets confusing, as it's no longer possible to determine the possible states of a piece of markup just by reading it. You have to read all the associated JavaScript and develop a mental model of the ways in which it could modify the DOM — and you don't have the same guarantees about state being consistently represented across the app because more than one thing 'owns' the DOM

Instead, it's more common to express dynamic classes in the template:

{#each things as thing}
  <div class="{thing === selected ? 'selected' : ''}>
    {thing.name}
  </div>
{/each}

<style>
  .selected {
    font-weight: bold;
  }
</style>

However if you really need to manipulate classes programmatically you can preserve the CSS with the :global(...) modifier. This will result in non-encapsulated CSS:

<div ref:things>
  {#each things as thing}
    <div data-id={thing.id}>
      {thing.name}
    </div>
  {/each}
</div>

<style>
  :global(.selected) {
    font-weight: bold;
  }
</style>

<script>
  export default {
    onupdate({ changed, current, previous }) {
      if (changed.selected) {
        if (previous.selected) {
          const div = this.refs.things.querySelector(`[data-id="${previous.selected.id}"]`);
          div.classList.remove('selected');
        }
        
        if (current.selected) {
          const div = this.refs.things.querySelector(`[data-id="${current.selected.id}"]`);
          div.classList.add('selected');
        }
      }
    }
  };
</script>

To preserve some encapsulation you can put the global selector inside a local one — it will still affect any child components, but it won't affect the rest of the page:

<style>
  ref:things :global(.selected) {
    font-weight: bold;
  }
</style>

Thanks for this great writeup, I definitely will put this to my bookmarks!
I also found a super clean solution for my issue, which was just for fading in an element, so I could do it as simple as:

{#if backdropActive}
    <div transition:fade="{duration: fadeDuration}" aria-hidden="false"></div>
{/if}

<script type="text/javascript">
  import { fade } from 'svelte-transitions';

  export default {
    transitions: { fade }
  }
</script>

Still we have now a different issue, same warning, but different usecase:

We're using some external scss framework that provides variables, functions, mixins, etc which are required currently to be in each <style>

<style type="text/scss">
@import 'node_modules/fundamental-ui/scss/icons';
@import 'node_modules/fundamental-ui/scss/core';
@import 'node_modules/fundamental-ui/scss/components/button';
// sometimes even more imports

Of course there are lots of css declarations unused with this type of usage.
Is there a better way, have I generally missed something?

Another valid use case for this is non-Svelte components. We've got an ag-grid implementation with renderers that apply classes for example.

And another use case is when rendering a recursive components (e.g. trees) with <svelte:self /> - I wanted to style nested levels (e.g. ul ul {}) and got the "Unused CSS" warning and the css gets removed.

I'd probably be somewhat against having ul ul {} be able to style ul => <svelte:self> => ul.

What happens when you have <Foo> => ul => <Bar> => ... => <Foo> => ul? The <Foo> component has no way of knowing whether <Bar> may have another instance of <Foo> inside it. It shouldn't keep the ul ul selector just because somewhere down in the tree eventually there might be another instance of <Foo> containing a ul.

I think the current rule of 'selectors refer to this instance of this component (unless they are wrapped in :global(...))' is much easier to reason about than 'selectors refer to this instance of this component or immediately nested ones via <svelte:self> but not any deeper ones'.

@Conduitry yep, I used :global to make it work.

Closing this - I think the current behavior is what we want.

So I use Sass mixins and would love an eslint-style ignore option. Here's an example mixin:

@mixin shape {
  background-image: linear-gradient(135deg, $pink-3, $violet-4 100%);

  &[shape="circle"],
  &.circle {
    clip-path: circle(50% at 50% 50%);
    -webkit-clip-path: circle(50% at 50% 50%);
  }

  &[shape="diamond"],
  &.diamond {
    clip-path: polygon(50% 0%, 100% 50%, 50% 100%, 0% 50%);
    -webkit-clip-path: polygon(50% 0%, 100% 50%, 50% 100%, 0% 50%);
  }

  &[shape="triangle"],
  &.triangle {
    clip-path: polygon(50% 0%, 0% 100%, 100% 100%);
    -webkit-clip-path: polygon(50% 0%, 0% 100%, 100% 100%);
  }

  &[shape="triangle-alt"],
  &.triangle-alt {
    clip-path: polygon(50% 100%, 0 0, 100% 0);
    -webkit-clip-path: polygon(50% 100%, 0 0, 100% 0);
  }

  img {
    width: 100%; height: 100%;
    object-fit: cover;
  }
}

When including this into my avatar class, obviously only one of these rules will be used. However, due to the nature of my app, there usage of the included styles will vary from page to page.

EDIT: It'd be neat if I could place a rule inside the mixin file itself but that's probably the purview of the preprocessor...

Here's how I "solved" the issue for the particular mixin mentioned above:

// in rollup.config.js
...,
svelte({
  preprocess,
  dev: isDevelopment,
  hydratable: true,
  emitCss: true,
  onwarn: (warning, handler) => {
    const { code, frame } = warning;

    if (code === "anchor-is-valid" || code === "a11y-autofocus")
      return;

    if (code === "css-unused-selector" && frame.includes("shape"))
      return;

    handler(warning);
  }
}),
...

I'm still getting the error on rules like header-navigation-section.active because .active is conditional but I'll figure that out some other time.

I ran into a similar problem where I had <section> in HTML and section + section in CSS.

aral commented

Another valid use case for this is if you’re using progressive enhancement and setting the ARIA role of an element dynamically only if JavaScript is on/loaded and then basing your styles on those ARIA roles to style components differently.

e.g., you have a list of links and sections that renders as a list of links and sections (e.g., using SSR in SvelteKit) and, on the client, is progressively enhanced into a tab interface. The CSS uses [role="tabpanel"], etc., selectors to apply only if the component is displaying as a tab panel (and not as a regular list of links and sections).

In this case, currently, there is no way to turn off the warnings that the styles are not used.

Use case (work-in-progress) from the Svelte version of Heydon Pickering’s Inclusive Tabbed Component: https://github.com/aral/heydons-accessible-tabs-in-svelte/blob/main/src/lib/TabbedInterface/TabList.svelte

Update: The closest I’ve been able to come to generating the CSS I need is:

* :global([role="tablist"]) {
    padding: 0;
  }

But that generates (e.g.):

.s-QrjSjQiqMQoF [role="tablist"]

Whereas what I need is a compound rule:

.s-QrjSjQiqMQoF[role="tablist"]

(The only difference is the lack of the space separator, making it a compound selector).

When I try to remove the space between * and :global() in hopes it would create a scoped compound rule, I get the following error:

:global(...) must be the first element in a compound selectorsvelte(css-invalid-global)

Basically, I think what’s lacking is a version of :global() that can create compound selectors. Or, to ease the restrictions on :global() so it isn’t limited to being the first rule in a compound selector. Or, a :scoped() -style modifier that acts like :global() but creates compound rules.

Will open a separate issue when I get a chance.

Opened a separate issue here: #6264

It's really sad when libraries decide for you how you should code and you have no say on it. There is (was?) a neverending discussion on Create-React-App exactly because it's hard to configure their ESLint rules... Why should we be forced to see warnings about our code which we can't force-ignore?

I'm another person using third-party code and I'm stuck with the longest build output ever, because "oh no this selector is unused" but it's actually in use. Why are we forced to code very specific warning ignores in some disconnected config file? Is it complex to have ignore comments as linters usually do?

Would be really awesome if we could just opt out of this automatic removal of CSS.

I strongly disagree with @Conduitry in that making styles global the stop Svelte from deleting our code is "what we want" and more than a (potentially dangerous) hack.

Style scoping works by Svelte being able to tell at compile time which selectors match which elements. The styles that are getting removed are styles that wouldn't match anything in the component anyway because they would lack the appropriate style scoping class. :global() is what you want, if you want to opt out of both style removal and style scoping. Selectors like foo :global(bar) let you have some of the benefits of scoping (foo needs to be part of this component, visible at compile time) with some of the benefits of not scoping (bar will not be scoped and can be anything the browser sees at runtime).