diff --git a/.changeset/green-starfishes-shave.md b/.changeset/green-starfishes-shave.md new file mode 100644 index 000000000000..967bba753c74 --- /dev/null +++ b/.changeset/green-starfishes-shave.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: relax `:global` selector list validation diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index a8c39aaf9713..6196a85ade62 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -235,7 +235,31 @@ A top-level `:global {...}` block can only contain rules, not declarations ### css_global_block_invalid_list ``` -A `:global` selector cannot be part of a selector list with more than one item +A `:global` selector cannot be part of a selector list with entries that don't contain `:global` +``` + +The following CSS is invalid: + +```css +:global, x { + y { + color: red; + } +} +``` + +This is mixing a `:global` block, which means "everything in here is unscoped", with a scoped selector (`x` in this case). As a result it's not possible to transform the inner selector (`y` in this case) into something that satisfies both requirements. You therefore have to split this up into two selectors: + +```css +:global { + y { + color: red; + } +} + +x y { + color: red; +} ``` ### css_global_block_invalid_modifier diff --git a/packages/svelte/messages/compile-errors/style.md b/packages/svelte/messages/compile-errors/style.md index 1e1ab45e8cfc..f08a2156a35a 100644 --- a/packages/svelte/messages/compile-errors/style.md +++ b/packages/svelte/messages/compile-errors/style.md @@ -16,7 +16,31 @@ ## css_global_block_invalid_list -> A `:global` selector cannot be part of a selector list with more than one item +> A `:global` selector cannot be part of a selector list with entries that don't contain `:global` + +The following CSS is invalid: + +```css +:global, x { + y { + color: red; + } +} +``` + +This is mixing a `:global` block, which means "everything in here is unscoped", with a scoped selector (`x` in this case). As a result it's not possible to transform the inner selector (`y` in this case) into something that satisfies both requirements. You therefore have to split this up into two selectors: + +```css +:global { + y { + color: red; + } +} + +x y { + color: red; +} +``` ## css_global_block_invalid_modifier diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 6bf973948b92..aa328764e14a 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -555,12 +555,12 @@ export function css_global_block_invalid_declaration(node) { } /** - * A `:global` selector cannot be part of a selector list with more than one item + * A `:global` selector cannot be part of a selector list with entries that don't contain `:global` * @param {null | number | NodeLike} node * @returns {never} */ export function css_global_block_invalid_list(node) { - e(node, 'css_global_block_invalid_list', `A \`:global\` selector cannot be part of a selector list with more than one item\nhttps://svelte.dev/e/css_global_block_invalid_list`); + e(node, 'css_global_block_invalid_list', `A \`:global\` selector cannot be part of a selector list with entries that don't contain \`:global\`\nhttps://svelte.dev/e/css_global_block_invalid_list`); } /** diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-analyze.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-analyze.js index 76cb2f56e995..2dc343564831 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-analyze.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-analyze.js @@ -193,10 +193,12 @@ const css_visitors = { Rule(node, context) { node.metadata.parent_rule = context.state.rule; - node.metadata.is_global_block = node.prelude.children.some((selector) => { + // We gotta allow :global x, :global y because CSS preprocessors might generate that from :global { x, y {...} } + for (const complex_selector of node.prelude.children) { let is_global_block = false; - for (const child of selector.children) { + for (let selector_idx = 0; selector_idx < complex_selector.children.length; selector_idx++) { + const child = complex_selector.children[selector_idx]; const idx = child.selectors.findIndex(is_global_block_selector); if (is_global_block) { @@ -204,58 +206,56 @@ const css_visitors = { child.metadata.is_global_like = true; } - if (idx !== -1) { - is_global_block = true; + if (idx === 0) { + if ( + child.selectors.length > 1 && + selector_idx === 0 && + node.metadata.parent_rule === null + ) { + e.css_global_block_invalid_modifier_start(child.selectors[1]); + } else { + // `child` starts with `:global` + node.metadata.is_global_block = is_global_block = true; + + for (let i = 1; i < child.selectors.length; i++) { + walk(/** @type {AST.CSS.Node} */ (child.selectors[i]), null, { + ComplexSelector(node) { + node.metadata.used = true; + } + }); + } - for (let i = idx + 1; i < child.selectors.length; i++) { - walk(/** @type {AST.CSS.Node} */ (child.selectors[i]), null, { - ComplexSelector(node) { - node.metadata.used = true; - } - }); - } - } - } + if (child.combinator && child.combinator.name !== ' ') { + e.css_global_block_invalid_combinator(child, child.combinator.name); + } - return is_global_block; - }); + const declaration = node.block.children.find((child) => child.type === 'Declaration'); + const is_lone_global = + complex_selector.children.length === 1 && + complex_selector.children[0].selectors.length === 1; // just `:global`, not e.g. `:global x` - if (node.metadata.is_global_block) { - if (node.prelude.children.length > 1) { - e.css_global_block_invalid_list(node.prelude); - } + if (is_lone_global && node.prelude.children.length > 1) { + // `:global, :global x { z { ... } }` would become `x { z { ... } }` which means `z` is always + // constrained by `x`, which is not what the user intended + e.css_global_block_invalid_list(node.prelude); + } - const complex_selector = node.prelude.children[0]; - const global_selector = complex_selector.children.find((r, selector_idx) => { - const idx = r.selectors.findIndex(is_global_block_selector); - if (idx === 0) { - if (r.selectors.length > 1 && selector_idx === 0 && node.metadata.parent_rule === null) { - e.css_global_block_invalid_modifier_start(r.selectors[1]); + if ( + declaration && + // :global { color: red; } is invalid, but foo :global { color: red; } is valid + node.prelude.children.length === 1 && + is_lone_global + ) { + e.css_global_block_invalid_declaration(declaration); + } } - return true; } else if (idx !== -1) { - e.css_global_block_invalid_modifier(r.selectors[idx]); + e.css_global_block_invalid_modifier(child.selectors[idx]); } - }); - - if (!global_selector) { - throw new Error('Internal error: global block without :global selector'); - } - - if (global_selector.combinator && global_selector.combinator.name !== ' ') { - e.css_global_block_invalid_combinator(global_selector, global_selector.combinator.name); } - const declaration = node.block.children.find((child) => child.type === 'Declaration'); - - if ( - declaration && - // :global { color: red; } is invalid, but foo :global { color: red; } is valid - node.prelude.children.length === 1 && - node.prelude.children[0].children.length === 1 && - node.prelude.children[0].children[0].selectors.length === 1 - ) { - e.css_global_block_invalid_declaration(declaration); + if (node.metadata.is_global_block && !is_global_block) { + e.css_global_block_invalid_list(node.prelude); } } 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 dff034f8aad6..9f1142cce9a7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/css/index.js +++ b/packages/svelte/src/compiler/phases/3-transform/css/index.js @@ -170,7 +170,11 @@ const visitors = { if (node.metadata.is_global_block) { const selector = node.prelude.children[0]; - if (selector.children.length === 1 && selector.children[0].selectors.length === 1) { + if ( + node.prelude.children.length === 1 && + selector.children.length === 1 && + selector.children[0].selectors.length === 1 + ) { // `:global {...}` if (state.minify) { state.code.remove(node.start, node.block.start + 1); @@ -194,7 +198,7 @@ const visitors = { SelectorList(node, { state, next, path }) { // Only add comments if we're not inside a complex selector that itself is unused or a global block if ( - !is_in_global_block(path) && + (!is_in_global_block(path) || node.children.length > 1) && !path.find((n) => n.type === 'ComplexSelector' && !n.metadata.used) ) { const children = node.children; @@ -282,13 +286,24 @@ const visitors = { const global = /** @type {AST.CSS.PseudoClassSelector} */ (relative_selector.selectors[0]); remove_global_pseudo_class(global, relative_selector.combinator, context.state); - if ( - node.metadata.rule?.metadata.parent_rule && - global.args === null && - relative_selector.combinator === null - ) { - // div { :global.x { ... } } becomes div { &.x { ... } } - context.state.code.prependRight(global.start, '&'); + const parent_rule = node.metadata.rule?.metadata.parent_rule; + if (parent_rule && global.args === null) { + if (relative_selector.combinator === null) { + // div { :global.x { ... } } becomes div { &.x { ... } } + context.state.code.prependRight(global.start, '&'); + } + + // In case of multiple :global selectors in a selector list we gotta delete the comma, too, but only if + // the next selector is used; if it's unused then the comma deletion happens as part of removal of that next selector + if ( + parent_rule.prelude.children.length > 1 && + node.children.length === node.children.findIndex((s) => s === relative_selector) - 1 + ) { + const next_selector = parent_rule.prelude.children.find((s) => s.start > global.end); + if (next_selector && next_selector.metadata.used) { + context.state.code.update(global.end, next_selector.start, ''); + } + } } continue; } else { @@ -380,7 +395,9 @@ function remove_global_pseudo_class(selector, combinator, state) { // div :global.x becomes div.x while (/\s/.test(state.code.original[start - 1])) start--; } - state.code.remove(start, selector.start + ':global'.length); + + // update(...), not remove(...) because there could be a closing unused comment at the end + state.code.update(start, selector.start + ':global'.length, ''); } else { state.code .remove(selector.start, selector.start + ':global('.length) diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-1/_config.js b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-1/_config.js new file mode 100644 index 000000000000..85dedc801221 --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-1/_config.js @@ -0,0 +1,10 @@ +import { test } from '../../test'; + +export default test({ + error: { + code: 'css_global_block_invalid_list', + message: + "A `:global` selector cannot be part of a selector list with entries that don't contain `:global`", + position: [232, 246] + } +}); diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-1/main.svelte b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-1/main.svelte new file mode 100644 index 000000000000..260921f7048d --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-1/main.svelte @@ -0,0 +1,9 @@ + diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-2/_config.js b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-2/_config.js new file mode 100644 index 000000000000..f24095800a6d --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-2/_config.js @@ -0,0 +1,10 @@ +import { test } from '../../test'; + +export default test({ + error: { + code: 'css_global_block_invalid_list', + message: + "A `:global` selector cannot be part of a selector list with entries that don't contain `:global`", + position: [24, 43] + } +}); diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-2/main.svelte b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-2/main.svelte new file mode 100644 index 000000000000..2a09ec10cea5 --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple-2/main.svelte @@ -0,0 +1,6 @@ + diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple/_config.js b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple/_config.js deleted file mode 100644 index 9ae4e758c46f..000000000000 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple/_config.js +++ /dev/null @@ -1,9 +0,0 @@ -import { test } from '../../test'; - -export default test({ - error: { - code: 'css_global_block_invalid_list', - message: 'A `:global` selector cannot be part of a selector list with more than one item', - position: [9, 31] - } -}); diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple/main.svelte b/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple/main.svelte deleted file mode 100644 index 75178bc664eb..000000000000 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-multiple/main.svelte +++ /dev/null @@ -1,3 +0,0 @@ - diff --git a/packages/svelte/tests/css/samples/global-block/_config.js b/packages/svelte/tests/css/samples/global-block/_config.js index bee0d7204d00..a8b11a73ec14 100644 --- a/packages/svelte/tests/css/samples/global-block/_config.js +++ b/packages/svelte/tests/css/samples/global-block/_config.js @@ -16,6 +16,20 @@ export default test({ column: 16, character: 932 } + }, + { + code: 'css_unused_selector', + message: 'Unused CSS selector "unused :global"', + start: { + line: 100, + column: 29, + character: 1223 + }, + end: { + line: 100, + column: 43, + character: 1237 + } } ] }); diff --git a/packages/svelte/tests/css/samples/global-block/expected.css b/packages/svelte/tests/css/samples/global-block/expected.css index 438749224ba6..12f9a75032ff 100644 --- a/packages/svelte/tests/css/samples/global-block/expected.css +++ b/packages/svelte/tests/css/samples/global-block/expected.css @@ -90,3 +90,13 @@ opacity: 1; } } + + x, y { + color: green; + } + + div.svelte-xyz, div.svelte-xyz y /* (unused) unused*/ { + z { + color: green; + } + } diff --git a/packages/svelte/tests/css/samples/global-block/input.svelte b/packages/svelte/tests/css/samples/global-block/input.svelte index a1833636a13f..ee05205d67c3 100644 --- a/packages/svelte/tests/css/samples/global-block/input.svelte +++ b/packages/svelte/tests/css/samples/global-block/input.svelte @@ -92,4 +92,14 @@ opacity: 1; } } + + :global x, :global y { + color: green; + } + + div :global, div :global y, unused :global { + z { + color: green; + } + }