From 45137f0064f4720341bbd9a545284e68c6fa595c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 19 Jul 2024 18:21:55 +0200 Subject: [PATCH] `div { :global { &.x { ... } } }` is equivalent to `div:global.x { ... }`, so the latter should be allowed, too --- .../svelte/messages/compile-errors/style.md | 8 --- packages/svelte/src/compiler/errors.js | 18 ----- .../phases/2-analyze/css/css-analyze.js | 68 ++++++++++++------- .../compiler/phases/3-transform/css/index.js | 29 ++++++-- packages/svelte/src/compiler/types/css.d.ts | 2 +- .../css-global-block-combinator/_config.js | 2 +- .../css-global-block-combinator/main.svelte | 10 ++- .../css-global-block-declaration/_config.js | 2 +- .../css-global-block-declaration/main.svelte | 10 +++ .../_config.js | 10 --- .../main.svelte | 8 --- .../css-global-block-modifier/_config.js | 9 --- .../css-global-block-modifier/main.svelte | 3 - .../tests/css/samples/global-block/_config.js | 8 +-- .../css/samples/global-block/expected.css | 28 +++++++- .../css/samples/global-block/input.svelte | 25 +++++++ .../samples/global-nested-block/expected.css | 2 +- 17 files changed, 147 insertions(+), 95 deletions(-) delete mode 100644 packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/_config.js delete mode 100644 packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/main.svelte delete mode 100644 packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/_config.js delete mode 100644 packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/main.svelte diff --git a/packages/svelte/messages/compile-errors/style.md b/packages/svelte/messages/compile-errors/style.md index 49a0d5a08d..f2a1c61e64 100644 --- a/packages/svelte/messages/compile-errors/style.md +++ b/packages/svelte/messages/compile-errors/style.md @@ -18,14 +18,6 @@ > A :global {...} block cannot be part of a selector list with more than one item -## css_global_block_invalid_modifier - -> A :global {...} block cannot modify an existing selector - -## css_global_block_invalid_placement - -> A :global {...} block can only appear at the end of a selector sequence (did you mean to use :global(...) instead?) - ## css_global_invalid_placement > :global(...) can be at the start or end of a selector sequence, but not in the middle diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 040356df25..3e229870b4 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -461,24 +461,6 @@ export function css_global_block_invalid_list(node) { e(node, "css_global_block_invalid_list", "A :global {...} block cannot be part of a selector list with more than one item"); } -/** - * A :global {...} block cannot modify an existing selector - * @param {null | number | NodeLike} node - * @returns {never} - */ -export function css_global_block_invalid_modifier(node) { - e(node, "css_global_block_invalid_modifier", "A :global {...} block cannot modify an existing selector"); -} - -/** - * A :global {...} block can only appear at the end of a selector sequence (did you mean to use :global(...) instead?) - * @param {null | number | NodeLike} node - * @returns {never} - */ -export function css_global_block_invalid_placement(node) { - e(node, "css_global_block_invalid_placement", "A :global {...} block can only appear at the end of a selector sequence (did you mean to use :global(...) instead?)"); -} - /** * :global(...) can be at the start or end of a selector sequence, but not in the middle * @param {null | number | NodeLike} node 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 e8b570b92d..28273e428f 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 @@ -48,7 +48,7 @@ const analysis_visitors = { node.metadata.rule = context.state.rule; - node.metadata.used = node.children.every( + node.metadata.used ||= node.children.every( ({ metadata }) => metadata.is_global || metadata.is_global_like ); }, @@ -57,6 +57,7 @@ const analysis_visitors = { node.selectors.length >= 1 && node.selectors[0].type === 'PseudoClassSelector' && node.selectors[0].name === 'global' && + node.selectors[0].args !== null && // we want :global(...), not :global node.selectors.every( (selector) => selector.type === 'PseudoClassSelector' || selector.type === 'PseudoElementSelector' @@ -85,15 +86,33 @@ const analysis_visitors = { Rule(node, context) { node.metadata.parent_rule = context.state.rule; - // `:global {...}` or `div :global {...}` + // `:global {...}` or `div :global {...}` or `:global div` node.metadata.is_global_block = node.prelude.children.some((selector) => { - const last = selector.children[selector.children.length - 1]; + let is_global_block = false; - const s = last.selectors[last.selectors.length - 1]; + for (const child of selector.children) { + const idx = child.selectors.findIndex( + (s) => s.type === 'PseudoClassSelector' && s.name === 'global' && s.args === null + ); - if (s.type === 'PseudoClassSelector' && s.name === 'global' && s.args === null) { - return true; + if (idx !== -1) { + is_global_block = true; + for (let i = idx + 1; i < child.selectors.length; i++) { + walk(/** @type {Css.Node} */ (child.selectors[i]), null, { + ComplexSelector(node) { + node.metadata.used = true; + } + }); + } + } + + if (is_global_block) { + // TODO this needs to be more granular, the stuff before the :global selector should be scoped + child.metadata.is_global_like = true; + } } + + return is_global_block; }); context.next({ @@ -120,19 +139,29 @@ const validation_visitors = { const complex_selector = node.prelude.children[0]; const relative_selector = complex_selector.children[complex_selector.children.length - 1]; - if (relative_selector.selectors.length > 1) { - e.css_global_block_invalid_modifier( - relative_selector.selectors[relative_selector.selectors.length - 1] - ); - } - - if (relative_selector.combinator && relative_selector.combinator.name !== ' ') { - e.css_global_block_invalid_combinator(relative_selector, relative_selector.combinator.name); + if ( + relative_selector.combinator && + relative_selector.combinator.name !== ' ' && + relative_selector.selectors.length === 1 + ) { + const s = relative_selector.selectors[0]; + if (s.type === 'PseudoClassSelector' && s.name === 'global' && s.args === null) { + e.css_global_block_invalid_combinator( + relative_selector, + relative_selector.combinator.name + ); + } } const declaration = node.block.children.find((child) => child.type === 'Declaration'); - if (declaration) { + if ( + declaration && + // :global { color: red; } is invalid, but foo :global { color: red; } or 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); } } @@ -146,14 +175,7 @@ const validation_visitors = { if (global) { const idx = node.children.indexOf(global); - if (global.selectors[0].args === null && idx !== node.children.length - 1) { - // ensure `:global` is only at the end of a selector - e.css_global_block_invalid_placement(global.selectors[0]); - } else if ( - global.selectors[0].args !== null && - idx !== 0 && - idx !== node.children.length - 1 - ) { + if (global.selectors[0].args !== null && idx !== 0 && idx !== node.children.length - 1) { // ensure `:global(...)` is not used in the middle of a selector (but multiple `global(...)` in sequence are ok) for (let i = idx + 1; i < node.children.length; i++) { if (!is_global(node.children[i])) { 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 bc3952eacc..3e25795dd3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/css/index.js +++ b/packages/svelte/src/compiler/phases/3-transform/css/index.js @@ -140,7 +140,7 @@ const visitors = { if (node.metadata.is_global_block) { const selector = node.prelude.children[0]; - if (selector.children.length === 1) { + if (selector.children.length === 1 && selector.children[0].selectors.length === 1) { // `:global {...}` state.code.prependRight(node.start, '/* '); state.code.appendLeft(node.block.start + 1, '*/'); @@ -216,19 +216,36 @@ const visitors = { ComplexSelector(node, context) { const before_bumped = context.state.specificity.bumped; - /** @param {Css.SimpleSelector} selector */ + /** @param {Css.PseudoClassSelector} selector */ function remove_global_pseudo_class(selector) { - context.state.code - .remove(selector.start, selector.start + ':global('.length) - .remove(selector.end - 1, selector.end); + if (selector.args === null) { + context.state.code.remove(selector.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) { - remove_global_pseudo_class(relative_selector.selectors[0]); + remove_global_pseudo_class( + /** @type {Css.PseudoClassSelector} */ (relative_selector.selectors[0]) + ); continue; } + // TODO make more efficient? + for (const selector of relative_selector.selectors) { + if ( + selector.type === 'PseudoClassSelector' && + selector.name === 'global' && + selector.args === null + ) { + remove_global_pseudo_class(selector); + } + } + if (relative_selector.metadata.scoped) { if (relative_selector.selectors.length === 1) { // skip standalone :is/:where/& selectors diff --git a/packages/svelte/src/compiler/types/css.d.ts b/packages/svelte/src/compiler/types/css.d.ts index 89a739713a..d15f73aef1 100644 --- a/packages/svelte/src/compiler/types/css.d.ts +++ b/packages/svelte/src/compiler/types/css.d.ts @@ -58,7 +58,7 @@ export namespace Css { metadata: { /** :global(..) */ is_global: boolean; - /** :root, :host, ::view-transition */ + /** `:root`, `:host`, `::view-transition`, or selectors after a `:global` */ is_global_like: boolean; scoped: boolean; }; diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/_config.js b/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/_config.js index 1014cd34c2..317bd554d7 100644 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/_config.js +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/_config.js @@ -4,6 +4,6 @@ export default test({ error: { code: 'css_global_block_invalid_combinator', message: 'A :global {...} block cannot follow a > combinator', - position: [12, 21] + position: [79, 88] } }); diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/main.svelte b/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/main.svelte index b3b6ebe6ac..a538823b9a 100644 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/main.svelte +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-combinator/main.svelte @@ -1,3 +1,11 @@ diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/_config.js b/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/_config.js index 3f7a82b54c..37ac580e0a 100644 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/_config.js +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/_config.js @@ -4,6 +4,6 @@ export default test({ error: { code: 'css_global_block_invalid_declaration', message: 'A :global {...} block can only contain rules, not declarations', - position: [24, 34] + position: [109, 119] } }); diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/main.svelte b/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/main.svelte index 6a1891ca29..d3374f2aa0 100644 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/main.svelte +++ b/packages/svelte/tests/compiler-errors/samples/css-global-block-declaration/main.svelte @@ -1,5 +1,15 @@ diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/_config.js b/packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/_config.js deleted file mode 100644 index 0eb061e41e..0000000000 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/_config.js +++ /dev/null @@ -1,10 +0,0 @@ -import { test } from '../../test'; - -export default test({ - error: { - code: 'css_global_block_invalid_placement', - message: - 'A :global {...} block can only appear at the end of a selector sequence (did you mean to use :global(...) instead?)', - position: [50, 57] - } -}); diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/main.svelte b/packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/main.svelte deleted file mode 100644 index 15d721f54f..0000000000 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-invalid-selector/main.svelte +++ /dev/null @@ -1,8 +0,0 @@ - diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/_config.js b/packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/_config.js deleted file mode 100644 index 00843fa217..0000000000 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/_config.js +++ /dev/null @@ -1,9 +0,0 @@ -import { test } from '../../test'; - -export default test({ - error: { - code: 'css_global_block_invalid_modifier', - message: 'A :global {...} block cannot modify an existing selector', - position: [14, 21] - } -}); diff --git a/packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/main.svelte b/packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/main.svelte deleted file mode 100644 index 7d274e3894..0000000000 --- a/packages/svelte/tests/compiler-errors/samples/css-global-block-modifier/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 430b45fe37..652d95260d 100644 --- a/packages/svelte/tests/css/samples/global-block/_config.js +++ b/packages/svelte/tests/css/samples/global-block/_config.js @@ -7,14 +7,14 @@ export default test({ code: 'css_unused_selector', message: 'Unused CSS selector ".unused :global"', start: { - line: 16, + line: 31, column: 1, - character: 128 + character: 400 }, end: { - line: 16, + line: 31, column: 16, - character: 143 + character: 415 } } ] diff --git a/packages/svelte/tests/css/samples/global-block/expected.css b/packages/svelte/tests/css/samples/global-block/expected.css index 8bf0f0e596..e50d8207a7 100644 --- a/packages/svelte/tests/css/samples/global-block/expected.css +++ b/packages/svelte/tests/css/samples/global-block/expected.css @@ -1,15 +1,41 @@ + /* :global {*/ .x { color: green; } /*}*/ - div.svelte-xyz { + div.svelte-xyz { + .y { + color: green; + } + } + + /* some css preprocessors de-nest :global blocks with a single child + (e.g turn `:global { div { ... } }` into `:global div { ... }`), + so we need to support it, too */ + div { .y { color: green; } } + div.svelte-xyz p { + .y { + color: green; + } + } + + /* div :global { &.x { ...} } is allowed, so div :global.x must be, too */ + div.svelte-xyz .x { + color: green; + } + + /* div { :global { &.x { ...} } } is allowed, so div:global.x must be, too */ + div.svelte-xyz:is(html.dark-mode *) { + color: green; + } + /* (unused) .unused :global { .z { color: red; diff --git a/packages/svelte/tests/css/samples/global-block/input.svelte b/packages/svelte/tests/css/samples/global-block/input.svelte index d05a6b7edc..3aaa63849d 100644 --- a/packages/svelte/tests/css/samples/global-block/input.svelte +++ b/packages/svelte/tests/css/samples/global-block/input.svelte @@ -13,6 +13,31 @@ } } + /* some css preprocessors de-nest :global blocks with a single child + (e.g turn `:global { div { ... } }` into `:global div { ... }`), + so we need to support it, too */ + :global div { + .y { + color: green; + } + } + + div :global p { + .y { + color: green; + } + } + + /* div :global { &.x { ...} } is allowed, so div :global.x must be, too */ + div :global.x { + color: green; + } + + /* div { :global { &.x { ...} } } is allowed, so div:global.x must be, too */ + div:global:is(html.dark-mode *) { + color: green; + } + .unused :global { .z { color: red; diff --git a/packages/svelte/tests/css/samples/global-nested-block/expected.css b/packages/svelte/tests/css/samples/global-nested-block/expected.css index 22d855b4c6..fd76204493 100644 --- a/packages/svelte/tests/css/samples/global-nested-block/expected.css +++ b/packages/svelte/tests/css/samples/global-nested-block/expected.css @@ -10,7 +10,7 @@ color: green; } - p:where(.svelte-xyz) { + p:where(.svelte-xyz) { .y { color: green; }