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

Unused css selector warning - disable specific parts? #1594

Closed
maxmarkus opened this issue Jul 16, 2018 · 14 comments
Closed

Unused css selector warning - disable specific parts? #1594

maxmarkus opened this issue Jul 16, 2018 · 14 comments

Comments

@maxmarkus
Copy link

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

@Rich-Harris
Copy link
Member

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>

@maxmarkus
Copy link
Author

maxmarkus commented Jul 18, 2018

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?

@alindsay55661
Copy link

alindsay55661 commented Sep 19, 2018

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

@tborychowski
Copy link

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.

@Conduitry
Copy link
Member

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'.

@tborychowski
Copy link

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

@Conduitry
Copy link
Member

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

@NetOpWibby
Copy link

NetOpWibby commented Aug 9, 2019

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...

@NetOpWibby
Copy link

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.

@paulschreiber
Copy link

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

@aral
Copy link

aral commented Apr 30, 2021

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

@igorsantos07
Copy link

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?

@Florinstruct
Copy link

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.

@Conduitry
Copy link
Member

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).

@sveltejs sveltejs locked and limited conversation to collaborators Oct 13, 2022
@Conduitry Conduitry closed this as not planned Won't fix, can't repro, duplicate, stale Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants