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

Svelte 5: Non-top-level :has() selector is considered unused #13717

Closed
hyunbinseo opened this issue Oct 20, 2024 · 2 comments · Fixed by #13750
Closed

Svelte 5: Non-top-level :has() selector is considered unused #13717

hyunbinseo opened this issue Oct 20, 2024 · 2 comments · Fixed by #13750
Assignees
Labels
bug css Stuff related to Svelte's built-in CSS handling
Milestone

Comments

@hyunbinseo
Copy link
Contributor

Describe the bug

The <legend> element appears as

<form action="">
  <fieldset>
    <legend></legend>
  </fieldset>
</form>

<style lang="postcss">
  fieldset:has(> legend) {
    background-color: yellow;
  }
  /* Unused CSS selector "form > fieldset:has(> legend)" */
  form > fieldset:has(> legend) {
    background-color: red;
  }
</style>

I don't think this is an intended change of the following PR, because <legend> clearly appears in the markup.

Reproduction

pnpm i [email protected]                                                       
pnpm check

# ====================================
# Loading svelte-check in workspace:
# Getting Svelte diagnostics...
# 
# /src/routes/+page.svelte:12:2
# Warn: Unused CSS selector "form > fieldset:has(> legend)" (svelte)
#         /* Unused CSS selector "form > fieldset:has(> legend)" */
#         form > fieldset:has(> legend) {
#                 background-color: red;
# 
# 
# ====================================
# svelte-check found 0 errors and 1 warning in 1 file
pnpm i [email protected]
pnpm check

# ====================================
# Loading svelte-check in workspace:
# Getting Svelte diagnostics...
# 
# ====================================
# svelte-check found 0 errors and 0 warnings

Logs

No response

System Info

System:
  OS: macOS 15.0.1
  CPU: (10) arm64 Apple M1 Pro
  Memory: 116.39 MB / 16.00 GB
  Shell: 5.9 - /bin/zsh
Binaries:
  Node: 22.10.0 - ~/.local/state/fnm_multishells/16744_1729417203997/bin/node
  npm: 10.9.0 - ~/.local/state/fnm_multishells/16744_1729417203997/bin/npm
  pnpm: 9.12.2 - ~/.local/state/fnm_multishells/16744_1729417203997/bin/pnpm
Browsers:
  Chrome: 129.0.6668.101
  Edge: 130.0.2849.46
  Safari: 18.0.1
npmPackages:
  svelte: 5.0.0-next.266 => 5.0.0-next.266

Severity

blocking an upgrade

@hyunbinseo
Copy link
Contributor Author

This is a huge deal-breaker for me.

Above workaround does not work in this example:

<!-- Unused CSS selector "fieldset:has(legend) > legend" -->
<style lang="postcss">
  fieldset:has(legend) {
    @apply border-2 p-4;
  }
  fieldset:has(legend) > legend {
    @apply -ml-3 px-3 font-bold;
  }
</style>

Considering this case, the issue title should be updated as well.

@dummdidumm dummdidumm added the css Stuff related to Svelte's built-in CSS handling label Oct 20, 2024
@dummdidumm dummdidumm added this to the 5.x milestone Oct 20, 2024
@dummdidumm dummdidumm added the bug label Oct 20, 2024
@dummdidumm dummdidumm self-assigned this Oct 21, 2024
dummdidumm added a commit that referenced this issue Oct 21, 2024
Fixes #13717

There are two parts to this:

1. the parent selectors weren't passed along for the check inside `:has`, which in case of a leading combinator would mean it would always count as unused
2. In case if a selector like `x > y:has(z)`, the prior logic would correctly determine that for element `z` there's a match for the `:has` selector, by first checking its contents and then walking up the tree. But after it did that, it would try to walk up the tree once more, which is a) wasteful b) buggy because the tree walking mechanism would no longer be adjusted for the `:has` special case, resulting in false negatives. To fix that, the `:has` will return a new value from the function, signaling that it already fully checked the upper selectors, and so the function calling it will skip doing that.
dummdidumm added a commit that referenced this issue Oct 21, 2024
…13750)

Fixes #13717

There are two parts to this:

1. the parent selectors weren't passed along for the check inside `:has`, which in case of a leading combinator would mean it would always count as unused
2. In case if a selector like `x > y:has(z)`, the prior logic would correctly determine that for element `z` there's a match for the `:has` selector, by first checking its contents and then walking up the tree. But after it did that, it would try to walk up the tree once more, which is a) wasteful b) buggy because the tree walking mechanism would no longer be adjusted for the `:has` special case, resulting in false negatives. To fix that, the `:has` will return a new value from the function, signaling that it already fully checked the upper selectors, and so the function calling it will skip doing that.
@vnphanquang
Copy link

vnphanquang commented Oct 22, 2024

@dummdidumm thank you for the fix. Unfortunately, as of 5.0.5 any :has followed by another combinator-ed selector is still incorrectly marked as unused. For example:

<label>
	<input type="checkbox" />
	<span>I am red when checked</span>
</label>

<style>
	label:has(input:checked) span {
		color: red;
	}
</style>

main:has(input:checked) p is pruned. Test it in this Svelte 5 REPL.

Happy to open a separate issue, just let me know if needed. Thanks!


UPDATE: not any sorry. I looked into the test cases for :has. The equivalence of the above issue is:

Input:

<x>
	<y></y>
	<w></w>
</x>

<style>
	x:has(y) w {
		color: green;
	}
</style>

Expected:

	x.svelte-xyz:has(y:where(.svelte-xyz)) w:where(.svelte-xyz) {
		color: green;
	}

which wasn't covered yet as of 5.0.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug css Stuff related to Svelte's built-in CSS handling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants