Skip to content

Commit

Permalink
fix: better sibling selector handling (#11096)
Browse files Browse the repository at this point in the history
Keep sibling selectors when dealing with slots/render tags/`svelte:element` tags
fixes #9274
  • Loading branch information
dummdidumm authored Apr 9, 2024
1 parent 3462c54 commit ed9bab9
Show file tree
Hide file tree
Showing 11 changed files with 199 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-plants-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: keep sibling selectors when dealing with slots/render tags/`svelte:element` tags
70 changes: 43 additions & 27 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,13 @@ function apply_selector(relative_selectors, rule, element, stylesheet) {
let sibling_matched = false;

for (const possible_sibling of siblings.keys()) {
if (apply_selector(parent_selectors, rule, possible_sibling, stylesheet)) {
if (possible_sibling.type === 'RenderTag' || possible_sibling.type === 'SlotElement') {
// `{@render foo()}<p>foo</p>` with `:global(.x) + p` is a match
if (parent_selectors.length === 1 && parent_selectors[0].metadata.is_global) {
mark(relative_selector, element);
sibling_matched = true;
}
} else if (apply_selector(parent_selectors, rule, possible_sibling, stylesheet)) {
mark(relative_selector, element);
sibling_matched = true;
}
Expand Down Expand Up @@ -564,38 +570,39 @@ function get_element_parent(node) {
function find_previous_sibling(node) {
/** @type {import('#compiler').SvelteNode} */
let current_node = node;
do {
if (current_node.type === 'SlotElement') {
const slot_children = current_node.fragment.nodes;
if (slot_children.length > 0) {
current_node = slot_children.slice(-1)[0]; // go to its last child first
continue;
}
}
while (
// @ts-expect-error TODO
!current_node.prev &&
// @ts-expect-error TODO
current_node.parent &&
// @ts-expect-error TODO
current_node.parent.type === 'SlotElement'
) {
// @ts-expect-error TODO
current_node = current_node.parent;

while (
// @ts-expect-error TODO
!current_node.prev &&
// @ts-expect-error TODO
current_node.parent?.type === 'SlotElement'
) {
// @ts-expect-error TODO
current_node = current_node.parent;
}

// @ts-expect-error
current_node = current_node.prev;

while (current_node?.type === 'SlotElement') {
const slot_children = current_node.fragment.nodes;
if (slot_children.length > 0) {
current_node = slot_children.slice(-1)[0];
} else {
break;
}
// @ts-expect-error
current_node = current_node.prev;
} while (current_node && current_node.type === 'SlotElement');
}

return current_node;
}

/**
* @param {import('#compiler').SvelteNode} node
* @param {boolean} adjacent_only
* @returns {Map<import('#compiler').RegularElement, NodeExistsValue>}
* @returns {Map<import('#compiler').RegularElement | import('#compiler').SvelteElement | import('#compiler').SlotElement | import('#compiler').RenderTag, NodeExistsValue>}
*/
function get_possible_element_siblings(node, adjacent_only) {
/** @type {Map<import('#compiler').RegularElement, NodeExistsValue>} */
/** @type {Map<import('#compiler').RegularElement | import('#compiler').SvelteElement | import('#compiler').SlotElement | import('#compiler').RenderTag, NodeExistsValue>} */
const result = new Map();

/** @type {import('#compiler').SvelteNode} */
Expand All @@ -618,6 +625,14 @@ function get_possible_element_siblings(node, adjacent_only) {
if (adjacent_only && has_definite_elements(possible_last_child)) {
return result;
}
} else if (
prev.type === 'SlotElement' ||
prev.type === 'RenderTag' ||
prev.type === 'SvelteElement'
) {
result.set(prev, NODE_PROBABLY_EXISTS);
// Special case: slots, render tags and svelte:element tags could resolve to no siblings,
// so we want to continue until we find a definite sibling even with the adjacent-only combinator
}
}

Expand Down Expand Up @@ -720,7 +735,7 @@ function get_possible_last_child(relative_selector, adjacent_only) {
}

/**
* @param {Map<import('#compiler').RegularElement, NodeExistsValue>} result
* @param {Map<unknown, NodeExistsValue>} result
* @returns {boolean}
*/
function has_definite_elements(result) {
Expand All @@ -734,8 +749,9 @@ function has_definite_elements(result) {
}

/**
* @param {Map<import('#compiler').RegularElement, NodeExistsValue>} from
* @param {Map<import('#compiler').RegularElement, NodeExistsValue>} to
* @template T
* @param {Map<T, NodeExistsValue>} from
* @param {Map<T, NodeExistsValue>} to
* @returns {void}
*/
function add_to_map(from, to) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { test } from '../../test';

export default test({
warnings: [
// TODO
// {
// code: 'css-unused-selector',
// message: 'Unused CSS selector ".a ~ .b"',
// start: { character: 111, column: 1, line: 10 },
// end: { character: 118, column: 8, line: 10 }
// },
]
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

.before.svelte-xyz + .foo:where(.svelte-xyz) { color: green; }
.before.svelte-xyz ~ .foo:where(.svelte-xyz) { color: green; }
.before.svelte-xyz ~ .bar:where(.svelte-xyz) { color: green; }

.x + .foo.svelte-xyz { color: green; }
.x + .foo.svelte-xyz span:where(.svelte-xyz) { color: green; }
.x ~ .foo.svelte-xyz { color: green; }
.x ~ .foo.svelte-xyz span:where(.svelte-xyz) { color: green; }
.x ~ .bar.svelte-xyz { color: green; }

/* no match */
/* (unused) :global(.x) + .bar { color: green; }*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<div>
<p class="before">before</p>
{@render children()}
<p class="foo">
<span>foo</span>
</p>
<p class="bar">bar</p>
</div>

<style>
.before + .foo { color: green; }
.before ~ .foo { color: green; }
.before ~ .bar { color: green; }
:global(.x) + .foo { color: green; }
:global(.x) + .foo span { color: green; }
:global(.x) ~ .foo { color: green; }
:global(.x) ~ .foo span { color: green; }
:global(.x) ~ .bar { color: green; }
/* no match */
:global(.x) + .bar { color: green; }
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { test } from '../../test';

export default test({
warnings: [
// TODO
// {
// code: 'css-unused-selector',
// message: 'Unused CSS selector ".a ~ .b"',
// start: { character: 111, column: 1, line: 10 },
// end: { character: 118, column: 8, line: 10 }
// },
]
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

.before.svelte-xyz + .foo:where(.svelte-xyz) { color: green; }
.before.svelte-xyz ~ .foo:where(.svelte-xyz) { color: green; }
.before.svelte-xyz ~ .bar:where(.svelte-xyz) { color: green; }

.x + .foo.svelte-xyz { color: green; }
.x + .foo.svelte-xyz span:where(.svelte-xyz) { color: green; }
.x ~ .foo.svelte-xyz { color: green; }
.x ~ .foo.svelte-xyz span:where(.svelte-xyz) { color: green; }
.x ~ .bar.svelte-xyz { color: green; }

/* no match */
/* (unused) :global(.x) + .bar { color: green; }*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<div>
<p class="before">before</p>
<slot></slot>
<p class="foo">
<span>foo</span>
</p>
<p class="bar">bar</p>
</div>

<style>
.before + .foo { color: green; }
.before ~ .foo { color: green; }
.before ~ .bar { color: green; }
:global(.x) + .foo { color: green; }
:global(.x) + .foo span { color: green; }
:global(.x) ~ .foo { color: green; }
:global(.x) ~ .foo span { color: green; }
:global(.x) ~ .bar { color: green; }
/* no match */
:global(.x) + .bar { color: green; }
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { test } from '../../test';

export default test({
warnings: [
// TODO
// {
// code: 'css-unused-selector',
// message: 'Unused CSS selector ".a ~ .b"',
// start: { character: 111, column: 1, line: 10 },
// end: { character: 118, column: 8, line: 10 }
// },
]
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

.before.svelte-xyz + .foo:where(.svelte-xyz) { color: green; }
.before.svelte-xyz ~ .foo:where(.svelte-xyz) { color: green; }
.before.svelte-xyz ~ .bar:where(.svelte-xyz) { color: green; }

.x.svelte-xyz + .foo:where(.svelte-xyz) { color: green; }
.x.svelte-xyz + .foo:where(.svelte-xyz) span:where(.svelte-xyz) { color: green; }
.x.svelte-xyz ~ .foo:where(.svelte-xyz) { color: green; }
.x.svelte-xyz ~ .foo:where(.svelte-xyz) span:where(.svelte-xyz) { color: green; }
.x.svelte-xyz ~ .bar:where(.svelte-xyz) { color: green; }

/* no match */
/* (unused) .x + .bar { color: green; }*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<script>
let tag = 'div'
</script>

<div>
<p class="before">before</p>
<svelte:element class="x" this={tag}></svelte:element>
<p class="foo">
<span>foo</span>
</p>
<p class="bar">bar</p>
</div>

<style>
.before + .foo { color: green; }
.before ~ .foo { color: green; }
.before ~ .bar { color: green; }
.x + .foo { color: green; }
.x + .foo span { color: green; }
.x ~ .foo { color: green; }
.x ~ .foo span { color: green; }
.x ~ .bar { color: green; }
/* no match */
.x + .bar { color: green; }
</style>

0 comments on commit ed9bab9

Please sign in to comment.