diff --git a/.changeset/nervous-squids-drop.md b/.changeset/nervous-squids-drop.md new file mode 100644 index 000000000000..3e2ad4485b70 --- /dev/null +++ b/.changeset/nervous-squids-drop.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: scope `:not(...)` selectors 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 c51de75fd626..2159c33afca7 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 @@ -121,16 +121,26 @@ const visitors = { }; /** - * Discard trailing `:global(...)` selectors without a `:has(...)` modifier, these are unused for scoping purposes + * Discard trailing `:global(...)` selectors without a `:has/is/where/not(...)` modifier, these are unused for scoping purposes * @param {Compiler.Css.ComplexSelector} node */ function truncate(node) { const i = node.children.findLastIndex(({ metadata, selectors }) => { + const first = selectors[0]; return ( + // not after a :global selector !metadata.is_global_like && + !(first.type === 'PseudoClassSelector' && first.name === 'global' && first.args === null) && + // not a :global(...) without a :has/is/where/not(...) modifier (!metadata.is_global || selectors.some( - (selector) => selector.type === 'PseudoClassSelector' && selector.name === 'has' + (selector) => + selector.type === 'PseudoClassSelector' && + selector.args !== null && + (selector.name === 'has' || + selector.name === 'not' || + selector.name === 'is' || + selector.name === 'where') )) ); }); @@ -440,17 +450,47 @@ function relative_selector_might_apply_to_node( // 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') && selector.args) { + if ((name === 'is' || name === 'where' || name === 'not') && selector.args) { let matched = false; for (const complex_selector of selector.args.children) { - if (apply_selector(truncate(complex_selector), rule, element, stylesheet, check_has)) { + const relative = truncate(complex_selector); + if ( + relative.length === 0 /* is :global(...) */ || + apply_selector(relative, rule, element, stylesheet, check_has) + ) { complex_selector.metadata.used = true; matched = true; + } 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. + // The result may not match a real element, so the only drawback is the missing prune. + complex_selector.metadata.used = true; + matched = true; + for (const selector of relative) { + selector.metadata.scoped = true; + } } } if (!matched) { + if ( + relative_selector.metadata.is_global && + !relative_selector.metadata.is_global_like + ) { + // Edge case: `:global(.x):is(.y)` where `.x` is global but `.y` doesn't match. + // Since `used` is set to `true` for `:global(.x)` in css-analyze beforehand, and + // we have no way of knowing if it's safe to set it back to `false`, we'll mark + // the inner selector as used and scoped to prevent it from being pruned, which could + // result in a invalid CSS output (e.g. `.x:is(/* unused .y */)`). The result + // can't match a real element, so the only drawback is the missing prune. + // TODO clean this up some day + selector.args.children[0].metadata.used = true; + selector.args.children[0].children.forEach((selector) => { + selector.metadata.scoped = true; + }); + } + return false; } } 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 31a8f472b2bb..e1da4eca9679 100644 --- a/packages/svelte/src/compiler/phases/3-transform/css/index.js +++ b/packages/svelte/src/compiler/phases/3-transform/css/index.js @@ -311,7 +311,7 @@ const visitors = { context.state.specificity.bumped = before_bumped; }, PseudoClassSelector(node, context) { - if (node.name === 'is' || node.name === 'where' || node.name === 'has') { + if (node.name === 'is' || node.name === 'where' || node.name === 'has' || node.name === 'not') { context.next(); } } diff --git a/packages/svelte/src/compiler/types/css.d.ts b/packages/svelte/src/compiler/types/css.d.ts index 287e551aaa0b..97ac7fc0d3a7 100644 --- a/packages/svelte/src/compiler/types/css.d.ts +++ b/packages/svelte/src/compiler/types/css.d.ts @@ -87,6 +87,8 @@ export namespace Css { /** * `true` if the whole selector is unscoped, e.g. `:global(...)` or `:global` or `:global.x`. * Selectors like `:global(...).x` are not considered global, because they still need scoping. + * Selectors like `:global(...):is/where/not/has(...)` are considered global even if they aren't + * strictly speaking (we should consolidate the logic around this at some point). */ is_global: boolean; /** `:root`, `:host`, `::view-transition`, or selectors after a `:global` */ diff --git a/packages/svelte/tests/css/samples/is/_config.js b/packages/svelte/tests/css/samples/is/_config.js index cb3a31ce2061..e4c24eb75681 100644 --- a/packages/svelte/tests/css/samples/is/_config.js +++ b/packages/svelte/tests/css/samples/is/_config.js @@ -4,16 +4,72 @@ export default test({ warnings: [ { code: 'css_unused_selector', + message: 'Unused CSS selector ".unused"', + start: { + line: 11, + column: 10, + character: 80 + }, end: { - character: 38, - column: 11, - line: 6 + line: 11, + column: 17, + character: 87 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector "x :is(.unused)"', + start: { + line: 14, + column: 1, + character: 111 }, - message: 'Unused CSS selector "z"', + end: { + line: 14, + column: 15, + character: 125 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector ".unused"', start: { - character: 37, - column: 10, - line: 6 + line: 14, + column: 7, + character: 117 + }, + end: { + line: 14, + column: 14, + character: 124 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector ":global(.foo) :is(.unused)"', + start: { + line: 28, + column: 1, + character: 274 + }, + end: { + line: 28, + column: 27, + character: 300 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector ".unused"', + start: { + line: 28, + column: 19, + character: 292 + }, + end: { + line: 28, + column: 26, + character: 299 } } ] diff --git a/packages/svelte/tests/css/samples/is/expected.css b/packages/svelte/tests/css/samples/is/expected.css index 2f30feedc230..be8deeff2892 100644 --- a/packages/svelte/tests/css/samples/is/expected.css +++ b/packages/svelte/tests/css/samples/is/expected.css @@ -1,4 +1,34 @@ - x.svelte-xyz :is(y:where(.svelte-xyz) /* (unused) z*/) { - color: purple; + x.svelte-xyz :is(y:where(.svelte-xyz)) { + color: green; + } + x.svelte-xyz :is(y:where(.svelte-xyz) /* (unused) .unused*/) { + color: green; + } + /* (unused) x :is(.unused) { + color: red; + }*/ + + x.svelte-xyz :is(y) { + color: green; + } + x.svelte-xyz :is(.foo) { + color: green; + } + + .foo :is(x.svelte-xyz) { + color: green; + } + /* (unused) :global(.foo) :is(.unused) { + color: red; + }*/ + + x.svelte-xyz :is(html *) { + color: green; + } + x.svelte-xyz :is(html:where(.svelte-xyz) :where(.svelte-xyz)) { + color: red; /* TODO would be nice to prune this one day */ + } + y.svelte-xyz :is(x:where(.svelte-xyz) :where(.svelte-xyz)) { + color: green; /* matches z */ } diff --git a/packages/svelte/tests/css/samples/is/input.svelte b/packages/svelte/tests/css/samples/is/input.svelte index de84526d67ef..06aa0669b90f 100644 --- a/packages/svelte/tests/css/samples/is/input.svelte +++ b/packages/svelte/tests/css/samples/is/input.svelte @@ -1,9 +1,41 @@ - + + + diff --git a/packages/svelte/tests/css/samples/not-selector/_config.js b/packages/svelte/tests/css/samples/not-selector/_config.js new file mode 100644 index 000000000000..6ab49a8809fb --- /dev/null +++ b/packages/svelte/tests/css/samples/not-selector/_config.js @@ -0,0 +1,62 @@ +import { test } from '../../test'; + +export default test({ + warnings: [ + { + code: 'css_unused_selector', + message: 'Unused CSS selector ":not(.unused)"', + start: { + line: 8, + column: 1, + character: 89 + }, + end: { + line: 8, + column: 14, + character: 102 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector ":not(.foo):not(.unused)"', + start: { + line: 18, + column: 1, + character: 221 + }, + end: { + line: 18, + column: 24, + character: 244 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector "p :not(.foo)"', + start: { + line: 25, + column: 1, + character: 300 + }, + end: { + line: 25, + column: 13, + character: 312 + } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector ":global(.x) :not(.unused)"', + start: { + line: 34, + column: 1, + character: 469 + }, + end: { + line: 34, + column: 26, + character: 494 + } + } + ] +}); diff --git a/packages/svelte/tests/css/samples/not-selector/expected.css b/packages/svelte/tests/css/samples/not-selector/expected.css index 1e5f9478db79..45e9ad69c051 100644 --- a/packages/svelte/tests/css/samples/not-selector/expected.css +++ b/packages/svelte/tests/css/samples/not-selector/expected.css @@ -1,3 +1,33 @@ + + .svelte-xyz:not(.foo:where(.svelte-xyz)) { + color: green; + } + /* (unused) :not(.unused) { + color: red; + }*/ .svelte-xyz:not(.foo) { + color: green; + } + + .svelte-xyz:not(.foo:where(.svelte-xyz)):not(.unused) { + color: green; + } + /* (unused) :not(.foo):not(.unused) { color: red; + }*/ + + p.svelte-xyz:not(.foo:where(.svelte-xyz)) { + color: green; } + /* (unused) p :not(.foo) { + color: red; + }*/ + .x:not(.foo.svelte-xyz) { + color: green; + } + .x:not(.unused.svelte-xyz) { + color: red; /* TODO would be nice to prune this one day */ + } + /* (unused) :global(.x) :not(.unused) { + color: red; + }*/ diff --git a/packages/svelte/tests/css/samples/not-selector/input.svelte b/packages/svelte/tests/css/samples/not-selector/input.svelte index 31a880f2bbf2..57d062cf4993 100644 --- a/packages/svelte/tests/css/samples/not-selector/input.svelte +++ b/packages/svelte/tests/css/samples/not-selector/input.svelte @@ -3,6 +3,35 @@ \ No newline at end of file diff --git a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md index eff6a7c7438c..f45b2e041692 100644 --- a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md +++ b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md @@ -293,9 +293,9 @@ Svelte 5 is more strict about the HTML structure and will throw a compiler error Assignments to destructured parts of a `@const` declaration are no longer allowed. It was an oversight that this was ever allowed. -### :is(...), :where(...) and :has(...) are scoped +### :is(...), :where(...), :has(...) and :not(...) are scoped -Previously, Svelte did not analyse selectors inside `:is(...)`, `:where(...)` and `:has(...)`, effectively treating them as global. Svelte 5 analyses them in the context of the current component. As such, some selectors may now be treated as unused if they were relying on this treatment. To fix this, use `:global(...)` inside the `:is(...)/:where(...)/:has(...)` selectors. +Previously, Svelte did not analyse selectors inside `:is(...)`, `:where(...)`, `:has(...)` and `:not(...)`, effectively treating them as global. Svelte 5 analyses them in the context of the current component. As such, some selectors may now be treated as unused if they were relying on this treatment. To fix this, use `:global(...)` inside the `:is(...)/:where(...)/:has(...)/:not(...)` selectors. When using Tailwind's `@apply` directive, add a `:global` selector to preserve rules that use Tailwind-generated `:is(...)` selectors: