Skip to content

Commit 32304b2

Browse files
committed
The exception are :not selectors with descendant selectors, as that means "look up the tree" and we need to scope all ancestor elements in that case.
1 parent 1089c7b commit 32304b2

File tree

10 files changed

+115
-59
lines changed

10 files changed

+115
-59
lines changed

packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,7 @@ const visitors = {
162162
};
163163

164164
/**
165-
* Discard trailing `:global(...)` selectors without a `:has/is/where(...)` modifier, these are unused for scoping purposes
166-
* `:not(...)` modifiers are not considered, because they should stay unscoped, because scoping them would achieve the
167-
* opposite of what we want, because they are then _more_ likely to bleed out of the component.
165+
* Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes
168166
* @param {Compiler.Css.ComplexSelector} node
169167
*/
170168
function truncate(node) {
@@ -174,13 +172,16 @@ function truncate(node) {
174172
// not after a :global selector
175173
!metadata.is_global_like &&
176174
!(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) &&
177-
// not a :global(...) without a :has/is/where(...) modifier
175+
// not a :global(...) without a :has/is/where/not(...) modifier
178176
(!metadata.is_global ||
179177
selectors.some(
180178
(selector) =>
181179
selector.type === 'PseudoClassSelector' &&
182180
selector.args !== null &&
183-
(selector.name === 'has' || selector.name === 'is' || selector.name === 'where')
181+
(selector.name === 'has' ||
182+
selector.name === 'is' ||
183+
selector.name === 'where' ||
184+
selector.name === 'not')
184185
))
185186
);
186187
});
@@ -511,9 +512,34 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
511512
// We came across a :global, everything beyond it is global and therefore a potential match
512513
if (name === 'global' && selector.args === null) return true;
513514

514-
// We ignore :not(...) because its contents should stay unscoped. Scoping them would achieve the
515-
// opposite of what we want, because they are then _more_ likely to bleed out of the component,
516-
// because there would be more chances of the inner selector not matching, which means `:not` matches.
515+
// :not(...) contents should stay unscoped. Scoping them would achieve the opposite of what we want,
516+
// because they are then _more_ likely to bleed out of the component. The exception is complex selectors
517+
// with descendants, in which case we scope them all.
518+
if (name === 'not' && selector.args) {
519+
for (const complex_selector of selector.args.children) {
520+
complex_selector.metadata.used = true;
521+
const relative = truncate(complex_selector);
522+
523+
if (complex_selector.children.length > 1) {
524+
// foo:not(bar foo) means that bar is an ancestor of foo (side note: ending with foo is the only way the selector make sense).
525+
// We can't fully check if that actually matches with our current algorithm, so we just assume it does.
526+
// The result may not match a real element, so the only drawback is the missing prune.
527+
for (const selector of relative) {
528+
selector.metadata.scoped = true;
529+
}
530+
531+
/** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null} */
532+
let el = element;
533+
while (el) {
534+
el.metadata.scoped = true;
535+
el = get_element_parent(el);
536+
}
537+
}
538+
}
539+
540+
break;
541+
}
542+
517543
if ((name === 'is' || name === 'where') && selector.args) {
518544
let matched = false;
519545

packages/svelte/src/compiler/phases/3-transform/css/index.js

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -355,19 +355,9 @@ const visitors = {
355355
context.state.specificity.bumped = before_bumped;
356356
},
357357
PseudoClassSelector(node, context) {
358-
if (node.name === 'is' || node.name === 'where' || node.name === 'has') {
358+
if (node.name === 'is' || node.name === 'where' || node.name === 'has' || node.name === 'not') {
359359
context.next();
360360
}
361-
if (node.name === 'not' && node.args) {
362-
for (const complex_selector of node.args.children) {
363-
for (const relative_selector of complex_selector.children) {
364-
if (relative_selector.metadata.is_global) {
365-
const global = /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]);
366-
remove_global_pseudo_class(global, relative_selector.combinator, context.state);
367-
}
368-
}
369-
}
370-
}
371361
}
372362
};
373363

packages/svelte/tests/css/samples/not-selector-global/expected.css

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,21 @@
99
color: green;
1010
}
1111
.x .svelte-xyz:not(p) {
12-
color: red; /* TODO would be nice to prune this one day */
12+
color: green;
1313
}
1414
.x:not(p) {
15-
color: red; /* TODO would be nice to prune this one day */
15+
color: green;
1616
}
1717
.x .svelte-xyz:not(.unused) {
1818
color: green;
1919
}
20+
21+
span:not(p.svelte-xyz span:where(.svelte-xyz)) {
22+
color: green;
23+
}
24+
span.svelte-xyz:not(p span) {
25+
color: green;
26+
}
27+
span:not(p span) {
28+
color: green;
29+
}
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1-
<p class="foo svelte-xyz">foo</p> <p class="bar svelte-xyz">bar</p>
1+
<p class="foo svelte-xyz">foo</p>
2+
<p class="bar svelte-xyz">bar <span class="svelte-xyz">baz</span></p>
3+
<span class="svelte-xyz">buzz</span>

packages/svelte/tests/css/samples/not-selector-global/input.svelte

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<p class="foo">foo</p>
2-
<p class="bar">bar</p>
2+
<p class="bar">
3+
bar
4+
<span>baz</span>
5+
</p>
6+
<span>buzz</span>
37

48
<style>
59
:not(:global(.foo)) {
@@ -12,12 +16,22 @@
1216
color: green;
1317
}
1418
:global(.x) :not(p) {
15-
color: red; /* TODO would be nice to prune this one day */
19+
color: green;
1620
}
1721
:global(.x):not(p) {
18-
color: red; /* TODO would be nice to prune this one day */
22+
color: green;
1923
}
2024
:global(.x) :not(.unused) {
2125
color: green;
2226
}
27+
28+
:global(span):not(p span) {
29+
color: green;
30+
}
31+
span:not(:global(p span)) {
32+
color: green;
33+
}
34+
:global(span:not(p span)) {
35+
color: green;
36+
}
2337
</style>

packages/svelte/tests/css/samples/not-selector/_config.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,30 @@ export default test({
44
warnings: [
55
{
66
code: 'css_unused_selector',
7-
message: 'Unused CSS selector "p :not(.foo)"',
7+
message: 'Unused CSS selector "span :not(.foo)"',
88
start: {
9-
line: 22,
9+
line: 26,
1010
column: 1,
11-
character: 291
11+
character: 276
1212
},
1313
end: {
14-
line: 22,
15-
column: 13,
16-
character: 303
14+
line: 26,
15+
column: 16,
16+
character: 291
1717
}
1818
},
1919
{
2020
code: 'css_unused_selector',
21-
message: 'Unused CSS selector "p :not(.unused)"',
21+
message: 'Unused CSS selector "span :not(.unused)"',
2222
start: {
23-
line: 25,
23+
line: 29,
2424
column: 1,
25-
character: 324
25+
character: 312
2626
},
2727
end: {
28-
line: 25,
29-
column: 16,
30-
character: 339
28+
line: 29,
29+
column: 19,
30+
character: 330
3131
}
3232
}
3333
]

packages/svelte/tests/css/samples/not-selector/expected.css

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
color: green;
77
}
88
.svelte-xyz:not(p) {
9-
color: red; /* TODO would be nice to mark this as unused someday */
9+
color: green;
1010
}
1111

1212
.svelte-xyz:not(.foo):not(.unused) {
@@ -16,9 +16,13 @@
1616
p.svelte-xyz:not(.foo) {
1717
color: green;
1818
}
19-
/* (unused) p :not(.foo) {
19+
/* (unused) span :not(.foo) {
2020
color: red;
2121
}*/
22-
/* (unused) p :not(.unused) {
22+
/* (unused) span :not(.unused) {
2323
color: red;
2424
}*/
25+
26+
span.svelte-xyz:not(p:where(.svelte-xyz) span:where(.svelte-xyz)) {
27+
color: green;
28+
}
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
1-
<p class="foo svelte-xyz">foo</p> <p class="bar svelte-xyz">bar</p>
1+
<p class="foo svelte-xyz">foo</p>
2+
<p class="bar svelte-xyz">bar <span class="svelte-xyz">baz</span></p>
3+
<span class="svelte-xyz">buzz</span>
Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<p class="foo">foo</p>
2-
<p class="bar">bar</p>
2+
<p class="bar">
3+
bar
4+
<span>baz</span>
5+
</p>
6+
<span>buzz</span>
37

48
<style>
59
:not(.foo) {
@@ -9,7 +13,7 @@
913
color: green;
1014
}
1115
:not(p) {
12-
color: red; /* TODO would be nice to mark this as unused someday */
16+
color: green;
1317
}
1418
1519
:not(.foo):not(.unused) {
@@ -19,10 +23,14 @@
1923
p:not(.foo) {
2024
color: green;
2125
}
22-
p :not(.foo) {
26+
span :not(.foo) {
2327
color: red;
2428
}
25-
p :not(.unused) {
29+
span :not(.unused) {
2630
color: red;
2731
}
32+
33+
span:not(p span) {
34+
color: green;
35+
}
2836
</style>

packages/svelte/tests/css/test.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,30 +32,17 @@ const { test, run } = suite<CssTest>(async (config, cwd) => {
3232
await compile_directory(cwd, 'client', { cssHash: () => 'svelte-xyz', ...config.compileOptions });
3333
await compile_directory(cwd, 'server', { cssHash: () => 'svelte-xyz', ...config.compileOptions });
3434

35-
const dom_css = fs.readFileSync(`${cwd}/_output/client/input.svelte.css`, 'utf-8').trim();
36-
const ssr_css = fs.readFileSync(`${cwd}/_output/server/input.svelte.css`, 'utf-8').trim();
37-
38-
assert.equal(dom_css, ssr_css);
39-
40-
const dom_warnings = load_warnings(`${cwd}/_output/client/input.svelte.warnings.json`);
41-
const ssr_warnings = load_warnings(`${cwd}/_output/server/input.svelte.warnings.json`);
42-
const expected_warnings = (config.warnings || []).map(normalize_warning);
43-
assert.deepEqual(dom_warnings, ssr_warnings);
44-
assert.deepEqual(dom_warnings.map(normalize_warning), expected_warnings);
45-
4635
const expected = {
4736
html: try_read_file(`${cwd}/expected.html`),
4837
css: try_read_file(`${cwd}/expected.css`)
4938
};
5039

51-
assert.equal(dom_css.trim().replace(/\r\n/g, '\n'), (expected.css ?? '').trim());
52-
5340
// we do this here, rather than in the expected.html !== null
5441
// block, to verify that valid code was generated
5542
const ClientComponent = (await import(`${cwd}/_output/client/input.svelte.js`)).default;
5643
const ServerComponent = (await import(`${cwd}/_output/server/input.svelte.js`)).default;
5744

58-
// verify that the right elements have scoping selectors
45+
// verify that the right elements have scoping selectors (do this first to ensure all actual files are written to disk)
5946
if (expected.html !== null) {
6047
const target = window.document.createElement('main');
6148

@@ -75,6 +62,19 @@ const { test, run } = suite<CssTest>(async (config, cwd) => {
7562
// const actual_ssr = ServerComponent.render(config.props).html;
7663
// assert_html_equal(actual_ssr, expected.html);
7764
}
65+
66+
const dom_css = fs.readFileSync(`${cwd}/_output/client/input.svelte.css`, 'utf-8').trim();
67+
const ssr_css = fs.readFileSync(`${cwd}/_output/server/input.svelte.css`, 'utf-8').trim();
68+
69+
assert.equal(dom_css, ssr_css);
70+
71+
const dom_warnings = load_warnings(`${cwd}/_output/client/input.svelte.warnings.json`);
72+
const ssr_warnings = load_warnings(`${cwd}/_output/server/input.svelte.warnings.json`);
73+
const expected_warnings = (config.warnings || []).map(normalize_warning);
74+
assert.deepEqual(dom_warnings, ssr_warnings);
75+
assert.deepEqual(dom_warnings.map(normalize_warning), expected_warnings);
76+
77+
assert.equal(dom_css.trim().replace(/\r\n/g, '\n'), (expected.css ?? '').trim());
7878
});
7979

8080
export { test };

0 commit comments

Comments
 (0)