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

Wrong CSS "Unused CSS selector" #14204

Open
erwanvivien opened this issue Nov 7, 2024 · 8 comments · May be fixed by #14276 or #14271
Open

Wrong CSS "Unused CSS selector" #14204

erwanvivien opened this issue Nov 7, 2024 · 8 comments · May be fixed by #14276 or #14271
Labels
awaiting submitter needs a reproduction, or clarification css Stuff related to Svelte's built-in CSS handling

Comments

@erwanvivien
Copy link

erwanvivien commented Nov 7, 2024

Describe the bug

I think we should relax the CSS verification for attributes

In my code I had to add data-active={false as true} to not trigger the first CSS selector
Then I added the second CSS selector, but this one is not correctly picked up

Reproduction

<script lang="ts">
  //
</script>

<div class="floating-menu" data-active={false as true}>
  <div class="command-menu-tooltip"></div>
</div>

<style>
  /* This one is correctly picked up */
  .floating-menu[data-active='true'] > .command-menu-tooltip {
    background-color: red;
  }

  /* This one is not */
  .floating-menu[data-active='true'] {
    background-color: red;
  }
</style>

REPL

Logs

No response

System Info

System:
    OS: macOS 14.2.1
    CPU: (12) arm64 Apple M3 Pro
    Memory: 71.31 MB / 18.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.10.0 - ~/.nvm/versions/node/v22.10.0/bin/node
    npm: 10.9.0 - ~/.nvm/versions/node/v22.10.0/bin/npm
  Browsers:
    Chrome: 130.0.6723.93
    Safari: 17.2.1
  npmPackages:
    svelte: ^5.1.12 => 5.1.12

Severity

annoyance

@Leonidaz
Copy link

Leonidaz commented Nov 7, 2024

If a variable is used for the data attribute then the css is good. Not sure how intentional this is but it makes sense to me as the css parser seems to be quite literal about the hard-coded values.

Use a variable instead

@dummdidumm
Copy link
Member

I too do not fully understand what you're trying to show here. If the variable is false, then it's ok that it's marked as unused? Or is it about the weird discrepancy of one being marked as used and the other not?

@dummdidumm dummdidumm added awaiting submitter needs a reproduction, or clarification css Stuff related to Svelte's built-in CSS handling labels Nov 8, 2024
@Conduitry
Copy link
Member

I don't know what the false as true is supposed to do, but it does seem weird that the as true affects the scoping.

To me, the issue is that the first one isn't recognized as unused, not that the second one is.

@erwanvivien
Copy link
Author

Thanks everyone for the comments

@Conduitry said

To me, the issue is that the first one isn't recognized as unused, not that the second one is.

Which is what I kinda want to point out, I would indeed skip the as true when checking the literal values (meaning trusting values instead of typing)

At the end we'd have two unused CSS selector

@erwanvivien
Copy link
Author

I had this use case (which I changed later to using a state variable) but meanwhile I had this inner component using the old DOM API to add and remove data-attributes which meant I didn't care about the initial value

@dummdidumm
Copy link
Member

I dug into this and it turns out that this goes back to our TS stripping algorithm not working correctly. While it removes the TS nodes, the parent and prev nodes become out of date - they point to the old nodes prior to stripping. This goes back to zimmerframe automatically creating new objects where needed so everything stays immutable.
The most pragmatic solution IMO is to allow zimmerframe to mutate the AST instead, which solves this issue.

@erwanvivien
Copy link
Author

I've added comments on the zimmerframe PR

My guts are telling me that mutating things is usually not the right way of doing things though

@Rich-Harris Rich-Harris linked a pull request Nov 12, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification css Stuff related to Svelte's built-in CSS handling
Projects
None yet
5 participants