Skip to content

Commit

Permalink
fix: ensure explicit nesting selector is always applied (#14193)
Browse files Browse the repository at this point in the history
Previously, we were applying an explicit nesting selector to the start of a relative selector chain only when starting the traversal. Prepending the selector is important because it ensures we traverse upwards to the parent rule when the current selectors all matched and there's still more to do. But we forgot to do the prepend for parent rules, which meant that if we were nested two levels deep, we would stop too early. This fix ensures we prepend in that case, too.

Fixes #14178
  • Loading branch information
dummdidumm authored Nov 7, 2024
1 parent d7caf08 commit 1eed645
Show file tree
Hide file tree
Showing 10 changed files with 179 additions and 54 deletions.
5 changes: 5 additions & 0 deletions .changeset/nice-numbers-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: ensure explicit nesting selector is always applied
77 changes: 46 additions & 31 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,38 +77,9 @@ const visitors = {
}
},
ComplexSelector(node, context) {
const selectors = truncate(node);
const selectors = get_relative_selectors(node);
const inner = selectors[selectors.length - 1];

if (node.metadata.rule?.metadata.parent_rule && selectors.length > 0) {
let has_explicit_nesting_selector = false;

// nesting could be inside pseudo classes like :is, :has or :where
for (let selector of selectors) {
walk(
selector,
{},
{
// @ts-ignore
NestingSelector() {
has_explicit_nesting_selector = true;
}
}
);
// if we found one we can break from the others
if (has_explicit_nesting_selector) break;
}

if (!has_explicit_nesting_selector) {
selectors[0] = {
...selectors[0],
combinator: descendant_combinator
};

selectors.unshift(nesting_selector);
}
}

if (context.state.from_render_tag) {
// We're searching for a match that crosses a render tag boundary. That means we have to both traverse up
// the element tree (to see if we find an entry point) but also remove selectors from the end (assuming
Expand Down Expand Up @@ -156,6 +127,50 @@ const visitors = {
}
};

/**
* Retrieves the relative selectors (minus the trailing globals) from a complex selector.
* Also searches them for any existing `&` selectors and adds one if none are found.
* This ensures we traverse up to the parent rule when the inner selectors match and we're
* trying to see if the parent rule also matches.
* @param {Compiler.Css.ComplexSelector} node
*/
function get_relative_selectors(node) {
const selectors = truncate(node);

if (node.metadata.rule?.metadata.parent_rule && selectors.length > 0) {
let has_explicit_nesting_selector = false;

// nesting could be inside pseudo classes like :is, :has or :where
for (let selector of selectors) {
walk(
selector,
{},
{
// @ts-ignore
NestingSelector() {
has_explicit_nesting_selector = true;
}
}
);
// if we found one we can break from the others
if (has_explicit_nesting_selector) break;
}

if (!has_explicit_nesting_selector) {
if (selectors[0].combinator === null) {
selectors[0] = {
...selectors[0],
combinator: descendant_combinator
};
}

selectors.unshift(nesting_selector);
}
}

return selectors;
}

/**
* Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes
* @param {Compiler.Css.ComplexSelector} node
Expand Down Expand Up @@ -641,7 +656,7 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
const parent = /** @type {Compiler.Css.Rule} */ (rule.metadata.parent_rule);

for (const complex_selector of parent.prelude.children) {
if (apply_selector(truncate(complex_selector), parent, element, state)) {
if (apply_selector(get_relative_selectors(complex_selector), parent, element, state)) {
complex_selector.metadata.used = true;
matched = true;
}
Expand Down
5 changes: 0 additions & 5 deletions packages/svelte/tests/css/samples/nested-in-pseudo/_config.js

This file was deleted.

This file was deleted.

This file was deleted.

11 changes: 0 additions & 11 deletions packages/svelte/tests/css/samples/nested-in-pseudo/input.svelte

This file was deleted.

48 changes: 48 additions & 0 deletions packages/svelte/tests/css/samples/nesting-selectors/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { test } from '../../test';

export default test({
warnings: [
{
code: 'css_unused_selector',
message: 'Unused CSS selector ".unused:has(&)"',
start: {
line: 10,
column: 2,
character: 105
},
end: {
line: 10,
column: 16,
character: 119
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector "&.unused"',
start: {
line: 23,
column: 3,
character: 223
},
end: {
line: 23,
column: 11,
character: 231
}
},
{
code: 'css_unused_selector',
message: 'Unused CSS selector "&.unused"',
start: {
line: 37,
column: 3,
character: 344
},
end: {
line: 37,
column: 11,
character: 352
}
}
]
});
37 changes: 37 additions & 0 deletions packages/svelte/tests/css/samples/nesting-selectors/expected.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@

nav.svelte-xyz {
header:where(.svelte-xyz):has(&){
color: green;
}
/* (unused) .unused:has(&){
color: red;
}*/
}

header.svelte-xyz {
> nav:where(.svelte-xyz) {
color: green;

&.active {
color: green;
}

/* (unused) &.unused {
color: red;
}*/
}
}

header.svelte-xyz {
& > nav:where(.svelte-xyz) {
color: green;

&.active {
color: green;
}

/* (unused) &.unused {
color: red;
}*/
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<header class="svelte-xyz"><nav class="active svelte-xyz"></nav></header>
42 changes: 42 additions & 0 deletions packages/svelte/tests/css/samples/nesting-selectors/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<header>
<nav class="active"></nav>
</header>

<style>
nav {
header:has(&){
color: green;
}
.unused:has(&){
color: red;
}
}
header {
> nav {
color: green;
&.active {
color: green;
}
&.unused {
color: red;
}
}
}
header {
& > nav {
color: green;
&.active {
color: green;
}
&.unused {
color: red;
}
}
}
</style>

0 comments on commit 1eed645

Please sign in to comment.