diff --git a/.changeset/quick-eels-occur.md b/.changeset/quick-eels-occur.md new file mode 100644 index 000000000000..e5eb153fbf5d --- /dev/null +++ b/.changeset/quick-eels-occur.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: remove scoping for `:not` selectors diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 8e8670c8243e..387b6a5485e0 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -50,9 +50,9 @@ function migrate_css(state) { while (code) { if ( code.startsWith(':has') || - code.startsWith(':not') || code.startsWith(':is') || - code.startsWith(':where') + code.startsWith(':where') || + code.startsWith(':not') ) { let start = code.indexOf('(') + 1; let is_global = false; @@ -74,7 +74,7 @@ function migrate_css(state) { char = code[end]; } if (start && end) { - if (!is_global) { + if (!is_global && !code.startsWith(':not')) { str.prependLeft(starting + start, ':global('); str.appendRight(starting + end - 1, ')'); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index fae89f11edd8..4306d49d818f 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -10,7 +10,6 @@ import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js'; * stylesheet: Compiler.Css.StyleSheet; * element: Compiler.AST.RegularElement | Compiler.AST.SvelteElement; * from_render_tag: boolean; - * inside_not: boolean; * }} State */ /** @typedef {NODE_PROBABLY_EXISTS | NODE_DEFINITELY_EXISTS} NodeExistsValue */ @@ -62,13 +61,9 @@ export function prune(stylesheet, element) { const parent = get_element_parent(element); if (!parent) return; - walk( - stylesheet, - { stylesheet, element: parent, from_render_tag: true, inside_not: false }, - visitors - ); + walk(stylesheet, { stylesheet, element: parent, from_render_tag: true }, visitors); } else { - walk(stylesheet, { stylesheet, element, from_render_tag: false, inside_not: false }, visitors); + walk(stylesheet, { stylesheet, element, from_render_tag: false }, visitors); } } @@ -179,9 +174,9 @@ function truncate(node) { selector.type === 'PseudoClassSelector' && selector.args !== null && (selector.name === 'has' || - selector.name === 'not' || selector.name === 'is' || - selector.name === 'where') + selector.name === 'where' || + selector.name === 'not') )) ); }); @@ -227,7 +222,7 @@ function apply_selector(relative_selectors, rule, element, state) { // if this is the left-most non-global selector, mark it — we want // `x y z {...}` to become `x.blah y z.blah {...}` const parent = parent_selectors[parent_selectors.length - 1]; - if (!state.inside_not && (!parent || is_global(parent, rule))) { + if (!parent || is_global(parent, rule)) { mark(relative_selector, element); } @@ -269,7 +264,7 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule, if (parent.type === 'RegularElement' || parent.type === 'SvelteElement') { if (apply_selector(parent_selectors, rule, parent, state)) { // TODO the `name === ' '` causes false positives, but removing it causes false negatives... - if (!state.inside_not && (name === ' ' || crossed_component_boundary)) { + if (name === ' ' || crossed_component_boundary) { mark(parent_selectors[parent_selectors.length - 1], parent); } @@ -295,15 +290,11 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule, if (possible_sibling.type === 'RenderTag' || possible_sibling.type === 'SlotElement') { // `{@render foo()}

foo

` with `:global(.x) + p` is a match if (parent_selectors.length === 1 && parent_selectors[0].metadata.is_global) { - if (!state.inside_not) { - mark(relative_selector, element); - } + mark(relative_selector, element); sibling_matched = true; } } else if (apply_selector(parent_selectors, rule, possible_sibling, state)) { - if (!state.inside_not) { - mark(relative_selector, element); - } + mark(relative_selector, element); sibling_matched = true; } } @@ -512,7 +503,35 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element, // We came across a :global, everything beyond it is global and therefore a potential match if (name === 'global' && selector.args === null) return true; - if ((name === 'is' || name === 'where' || name === 'not') && selector.args) { + // :not(...) contents should stay unscoped. Scoping them would achieve the opposite of what we want, + // because they are then _more_ likely to bleed out of the component. The exception is complex selectors + // with descendants, in which case we scope them all. + if (name === 'not' && selector.args) { + for (const complex_selector of selector.args.children) { + complex_selector.metadata.used = true; + const relative = truncate(complex_selector); + + if (complex_selector.children.length > 1) { + // 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). + // We can't fully check if that actually matches with our current algorithm, so we just assume it does. + // The result may not match a real element, so the only drawback is the missing prune. + for (const selector of relative) { + selector.metadata.scoped = true; + } + + /** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null} */ + let el = element; + while (el) { + el.metadata.scoped = true; + el = get_element_parent(el); + } + } + } + + break; + } + + if ((name === 'is' || name === 'where') && selector.args) { let matched = false; for (const complex_selector of selector.args.children) { @@ -522,32 +541,9 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element, if (is_global) { complex_selector.metadata.used = true; matched = true; - } else if (name !== 'not' && apply_selector(relative, rule, element, state)) { + } else if (apply_selector(relative, rule, element, state)) { complex_selector.metadata.used = true; matched = true; - } else if ( - name === 'not' && - !apply_selector(relative, rule, element, { ...state, inside_not: true }) - ) { - // For `:not(...)` we gotta do the inverse: If it did not match, mark the element and possibly - // everything above (if the selector is written is a such) as scoped (because they matched by not matching). - element.metadata.scoped = true; - complex_selector.metadata.used = true; - matched = true; - - for (const r of relative) { - r.metadata.scoped = true; - } - - // bar:not(foo bar) means that foo is an ancestor of bar - if (complex_selector.children.length > 1) { - /** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null} */ - let el = get_element_parent(element); - while (el) { - el.metadata.scoped = true; - el = get_element_parent(el); - } - } } else if (complex_selector.children.length > 1 && (name == 'is' || name == 'where')) { // foo :is(bar baz) can also mean that bar is an ancestor of foo, and baz a descendant. // We can't fully check if that actually matches with our current algorithm, so we just assume it does. diff --git a/packages/svelte/src/compiler/phases/3-transform/css/index.js b/packages/svelte/src/compiler/phases/3-transform/css/index.js index 62c520e99487..350eca35d8a6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/css/index.js +++ b/packages/svelte/src/compiler/phases/3-transform/css/index.js @@ -278,29 +278,10 @@ const visitors = { ComplexSelector(node, context) { const before_bumped = context.state.specificity.bumped; - /** - * @param {Css.PseudoClassSelector} selector - * @param {Css.Combinator | null} combinator - */ - function remove_global_pseudo_class(selector, combinator) { - if (selector.args === null) { - let start = selector.start; - if (combinator?.name === ' ') { - // div :global.x becomes div.x - while (/\s/.test(context.state.code.original[start - 1])) start--; - } - context.state.code.remove(start, selector.start + ':global'.length); - } else { - context.state.code - .remove(selector.start, selector.start + ':global('.length) - .remove(selector.end - 1, selector.end); - } - } - for (const relative_selector of node.children) { if (relative_selector.metadata.is_global) { const global = /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]); - remove_global_pseudo_class(global, relative_selector.combinator); + remove_global_pseudo_class(global, relative_selector.combinator, context.state); if ( node.metadata.rule?.metadata.parent_rule && @@ -328,7 +309,7 @@ const visitors = { // for any :global() or :global at the middle of compound selector for (const selector of relative_selector.selectors) { if (selector.type === 'PseudoClassSelector' && selector.name === 'global') { - remove_global_pseudo_class(selector, null); + remove_global_pseudo_class(selector, null, context.state); } } @@ -380,6 +361,26 @@ const visitors = { } }; +/** + * @param {Css.PseudoClassSelector} selector + * @param {Css.Combinator | null} combinator + * @param {State} state + */ +function remove_global_pseudo_class(selector, combinator, state) { + if (selector.args === null) { + let start = selector.start; + if (combinator?.name === ' ') { + // div :global.x becomes div.x + while (/\s/.test(state.code.original[start - 1])) start--; + } + state.code.remove(start, selector.start + ':global'.length); + } else { + state.code + .remove(selector.start, selector.start + ':global('.length) + .remove(selector.end - 1, selector.end); + } +} + /** * Walk backwards until we find a non-whitespace character * @param {number} end diff --git a/packages/svelte/tests/css/samples/not-selector-global/_config.js b/packages/svelte/tests/css/samples/not-selector-global/_config.js index 2776ac227af7..292c6c49ac9d 100644 --- a/packages/svelte/tests/css/samples/not-selector-global/_config.js +++ b/packages/svelte/tests/css/samples/not-selector-global/_config.js @@ -1,20 +1,5 @@ import { test } from '../../test'; export default test({ - warnings: [ - { - code: 'css_unused_selector', - message: 'Unused CSS selector ":global(.x) :not(p)"', - start: { - line: 14, - column: 1, - character: 197 - }, - end: { - line: 14, - column: 20, - character: 216 - } - } - ] + warnings: [] }); diff --git a/packages/svelte/tests/css/samples/not-selector-global/expected.css b/packages/svelte/tests/css/samples/not-selector-global/expected.css index f90edd0c8801..ea9ff3948653 100644 --- a/packages/svelte/tests/css/samples/not-selector-global/expected.css +++ b/packages/svelte/tests/css/samples/not-selector-global/expected.css @@ -2,18 +2,28 @@ .svelte-xyz:not(.foo) { color: green; } - .svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) { + .svelte-xyz:not(.foo):not(.unused) { color: green; } - .x:not(.foo.svelte-xyz) { + .x:not(.foo) { color: green; } - /* (unused) :global(.x) :not(p) { - color: red; - }*/ - .x:not(p.svelte-xyz) { - color: red; /* TODO would be nice to prune this one day */ + .x .svelte-xyz:not(p) { + color: green; + } + .x:not(p) { + color: green; + } + .x .svelte-xyz:not(.unused) { + color: green; + } + + span:not(p.svelte-xyz span:where(.svelte-xyz)) { + color: green; + } + span.svelte-xyz:not(p span) { + color: green; } - .x .svelte-xyz:not(.unused:where(.svelte-xyz)) { + span:not(p span) { color: green; } diff --git a/packages/svelte/tests/css/samples/not-selector-global/expected.html b/packages/svelte/tests/css/samples/not-selector-global/expected.html index b4834bb8189e..5eeb2be29429 100644 --- a/packages/svelte/tests/css/samples/not-selector-global/expected.html +++ b/packages/svelte/tests/css/samples/not-selector-global/expected.html @@ -1 +1,3 @@ -

foo

bar

\ No newline at end of file +

foo

+

bar baz

+buzz \ No newline at end of file diff --git a/packages/svelte/tests/css/samples/not-selector-global/input.svelte b/packages/svelte/tests/css/samples/not-selector-global/input.svelte index 578052d943ff..1f0e6cd6db1d 100644 --- a/packages/svelte/tests/css/samples/not-selector-global/input.svelte +++ b/packages/svelte/tests/css/samples/not-selector-global/input.svelte @@ -1,5 +1,9 @@

foo

-

bar

+

+ bar + baz +

+buzz \ No newline at end of file diff --git a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/_config.js b/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/_config.js deleted file mode 100644 index 292c6c49ac9d..000000000000 --- a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/_config.js +++ /dev/null @@ -1,5 +0,0 @@ -import { test } from '../../test'; - -export default test({ - warnings: [] -}); diff --git a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.css b/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.css deleted file mode 100644 index 8fe09de6dc63..000000000000 --- a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.css +++ /dev/null @@ -1,4 +0,0 @@ - - .svelte-xyz:not(.bar:where(.svelte-xyz)) { - color: green; - } diff --git a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.html b/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.html deleted file mode 100644 index fa4ca95c605c..000000000000 --- a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/expected.html +++ /dev/null @@ -1 +0,0 @@ -

foo

bar

\ No newline at end of file diff --git a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/input.svelte b/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/input.svelte deleted file mode 100644 index 6599a21ca61f..000000000000 --- a/packages/svelte/tests/css/samples/not-selector-hash-on-right-elements/input.svelte +++ /dev/null @@ -1,8 +0,0 @@ -

foo

-

bar

- - \ No newline at end of file diff --git a/packages/svelte/tests/css/samples/not-selector/_config.js b/packages/svelte/tests/css/samples/not-selector/_config.js index 27b4f91a6ac3..9c91a2a4c079 100644 --- a/packages/svelte/tests/css/samples/not-selector/_config.js +++ b/packages/svelte/tests/css/samples/not-selector/_config.js @@ -4,44 +4,30 @@ export default test({ warnings: [ { code: 'css_unused_selector', - message: 'Unused CSS selector ":not(p)"', + message: 'Unused CSS selector "span :not(.foo)"', start: { - line: 11, + line: 26, column: 1, - character: 125 + character: 276 }, end: { - line: 11, - column: 8, - character: 132 - } - }, - { - code: 'css_unused_selector', - message: 'Unused CSS selector "p :not(.foo)"', - start: { - line: 22, - column: 1, - character: 235 - }, - end: { - line: 22, - column: 13, - character: 247 + line: 26, + column: 16, + character: 291 } }, { code: 'css_unused_selector', - message: 'Unused CSS selector "p :not(.unused)"', + message: 'Unused CSS selector "span :not(.unused)"', start: { - line: 25, + line: 29, column: 1, - character: 268 + character: 312 }, end: { - line: 25, - column: 16, - character: 283 + line: 29, + column: 19, + character: 330 } } ] diff --git a/packages/svelte/tests/css/samples/not-selector/expected.css b/packages/svelte/tests/css/samples/not-selector/expected.css index b4d51bf269d8..ea0b3aacacd9 100644 --- a/packages/svelte/tests/css/samples/not-selector/expected.css +++ b/packages/svelte/tests/css/samples/not-selector/expected.css @@ -1,24 +1,31 @@ - .svelte-xyz:not(.foo:where(.svelte-xyz)) { + .svelte-xyz:not(.foo) { color: green; } - .svelte-xyz:not(.unused:where(.svelte-xyz)) { + .svelte-xyz:not(.unused) { + color: green; + } + .svelte-xyz:not(p) { color: green; } - /* (unused) :not(p) { - color: red; - }*/ - .svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused:where(.svelte-xyz)) { + .svelte-xyz:not(.foo):not(.unused) { color: green; } - p.svelte-xyz:not(.foo:where(.svelte-xyz)) { + p.svelte-xyz:not(.foo) { color: green; } - /* (unused) p :not(.foo) { + /* (unused) span :not(.foo) { color: red; }*/ - /* (unused) p :not(.unused) { + /* (unused) span :not(.unused) { color: red; }*/ + + span.svelte-xyz:not(p:where(.svelte-xyz) span:where(.svelte-xyz)) { + color: green; + } + span.svelte-xyz:not(:focus) { + color: green; + } diff --git a/packages/svelte/tests/css/samples/not-selector/expected.html b/packages/svelte/tests/css/samples/not-selector/expected.html index b4834bb8189e..5eeb2be29429 100644 --- a/packages/svelte/tests/css/samples/not-selector/expected.html +++ b/packages/svelte/tests/css/samples/not-selector/expected.html @@ -1 +1,3 @@ -

foo

bar

\ No newline at end of file +

foo

+

bar baz

+buzz \ No newline at end of file diff --git a/packages/svelte/tests/css/samples/not-selector/input.svelte b/packages/svelte/tests/css/samples/not-selector/input.svelte index a0e72c7be8e9..3a786530a2ee 100644 --- a/packages/svelte/tests/css/samples/not-selector/input.svelte +++ b/packages/svelte/tests/css/samples/not-selector/input.svelte @@ -1,5 +1,9 @@

foo

-

bar

+

+ bar + baz +

+buzz \ No newline at end of file diff --git a/packages/svelte/tests/css/test.ts b/packages/svelte/tests/css/test.ts index 30ad96369a1f..443ee0295809 100644 --- a/packages/svelte/tests/css/test.ts +++ b/packages/svelte/tests/css/test.ts @@ -32,30 +32,17 @@ const { test, run } = suite(async (config, cwd) => { await compile_directory(cwd, 'client', { cssHash: () => 'svelte-xyz', ...config.compileOptions }); await compile_directory(cwd, 'server', { cssHash: () => 'svelte-xyz', ...config.compileOptions }); - const dom_css = fs.readFileSync(`${cwd}/_output/client/input.svelte.css`, 'utf-8').trim(); - const ssr_css = fs.readFileSync(`${cwd}/_output/server/input.svelte.css`, 'utf-8').trim(); - - assert.equal(dom_css, ssr_css); - - const dom_warnings = load_warnings(`${cwd}/_output/client/input.svelte.warnings.json`); - const ssr_warnings = load_warnings(`${cwd}/_output/server/input.svelte.warnings.json`); - const expected_warnings = (config.warnings || []).map(normalize_warning); - assert.deepEqual(dom_warnings, ssr_warnings); - assert.deepEqual(dom_warnings.map(normalize_warning), expected_warnings); - const expected = { html: try_read_file(`${cwd}/expected.html`), css: try_read_file(`${cwd}/expected.css`) }; - assert.equal(dom_css.trim().replace(/\r\n/g, '\n'), (expected.css ?? '').trim()); - // we do this here, rather than in the expected.html !== null // block, to verify that valid code was generated const ClientComponent = (await import(`${cwd}/_output/client/input.svelte.js`)).default; const ServerComponent = (await import(`${cwd}/_output/server/input.svelte.js`)).default; - // verify that the right elements have scoping selectors + // verify that the right elements have scoping selectors (do this first to ensure all actual files are written to disk) if (expected.html !== null) { const target = window.document.createElement('main'); @@ -75,6 +62,19 @@ const { test, run } = suite(async (config, cwd) => { // const actual_ssr = ServerComponent.render(config.props).html; // assert_html_equal(actual_ssr, expected.html); } + + const dom_css = fs.readFileSync(`${cwd}/_output/client/input.svelte.css`, 'utf-8').trim(); + const ssr_css = fs.readFileSync(`${cwd}/_output/server/input.svelte.css`, 'utf-8').trim(); + + assert.equal(dom_css, ssr_css); + + const dom_warnings = load_warnings(`${cwd}/_output/client/input.svelte.warnings.json`); + const ssr_warnings = load_warnings(`${cwd}/_output/server/input.svelte.warnings.json`); + const expected_warnings = (config.warnings || []).map(normalize_warning); + assert.deepEqual(dom_warnings, ssr_warnings); + assert.deepEqual(dom_warnings.map(normalize_warning), expected_warnings); + + assert.equal(dom_css.trim().replace(/\r\n/g, '\n'), (expected.css ?? '').trim()); }); export { test }; diff --git a/packages/svelte/tests/migrate/samples/is-not-where-has/output.svelte b/packages/svelte/tests/migrate/samples/is-not-where-has/output.svelte index 1e02e9bdfdc8..d8bb703ac53a 100644 --- a/packages/svelte/tests/migrate/samples/is-not-where-has/output.svelte +++ b/packages/svelte/tests/migrate/samples/is-not-where-has/output.svelte @@ -21,22 +21,22 @@ what if i'm talking about `:has()` in my blog?