Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: take snippets into account when scoping css selectors #10208

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-actors-grow.md
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: take snippets into account when scoping css selectors
54 changes: 44 additions & 10 deletions packages/svelte/src/compiler/phases/2-analyze/css/Selector.js
Expand Up @@ -58,7 +58,7 @@ export default class Selector {
apply(node) {
/** @type {Array<{ node: import('#compiler').RegularElement | import('#compiler').SvelteElement; block: Block }>} */
const to_encapsulate = [];
apply_selector(this.local_blocks.slice(), node, to_encapsulate);
apply_selector(this.local_blocks, node, to_encapsulate);
if (to_encapsulate.length > 0) {
to_encapsulate.forEach(({ node, block }) => {
this.stylesheet.nodes_with_css_class.add(node);
Expand Down Expand Up @@ -203,20 +203,36 @@ export default class Selector {
* @param {Block[]} blocks
* @param {import('#compiler').RegularElement | import('#compiler').SvelteElement | null} node
* @param {Array<{ node: import('#compiler').RegularElement | import('#compiler').SvelteElement; block: Block }>} to_encapsulate
* @param {boolean} [has_render_tag]
* @returns {boolean}
*/
function apply_selector(blocks, node, to_encapsulate) {
function apply_selector(
blocks,
node,
to_encapsulate,
has_render_tag = node?.fragment.nodes.some((n) => n.type === 'RenderTag')
) {
blocks = blocks.slice();
const block = blocks.pop();
if (!block) return false;
if (!node) {
return (
(block.global && blocks.every((block) => block.global)) || (block.host && blocks.length === 0)
);
}
const applies = block_might_apply_to_node(block, node);

let applies = block_might_apply_to_node(block, node);

if (applies === NO_MATCH) {
return false;
if (has_render_tag) {
// If the element contains a render tag then we assume the selector might match something inside the rendered snippet
// and traverse the blocks upwards to see if the present blocks match our node further upwards.
// We could do more static analysis and check the render tag reference to see if this snippet block continues
// with elements that actually match the selector, but that would be a lot of work for little gain
return apply_selector(blocks, node, to_encapsulate, true);
} else {
return false;
}
}

if (applies === UNKNOWN_SELECTOR) {
Expand All @@ -225,7 +241,7 @@ function apply_selector(blocks, node, to_encapsulate) {
}

if (block.combinator) {
if (block.combinator.type === 'Combinator' && block.combinator.name === ' ') {
if (block.combinator.name === ' ') {
for (const ancestor_block of blocks) {
if (ancestor_block.global) {
continue;
Expand All @@ -234,7 +250,7 @@ function apply_selector(blocks, node, to_encapsulate) {
to_encapsulate.push({ node, block });
return true;
}
/** @type {import('#compiler').RegularElement | import('#compiler').SvelteElement | null} */
/** @type {ReturnType<typeof get_element_parent>} */
let parent = node;
while ((parent = get_element_parent(parent))) {
if (block_might_apply_to_node(ancestor_block, parent) !== NO_MATCH) {
Expand All @@ -250,10 +266,27 @@ function apply_selector(blocks, node, to_encapsulate) {
to_encapsulate.push({ node, block });
return true;
}
// The inverse of the render tag logic above: mark the node as encapsulated if it's inside a snippet block.
// May result in false positives just like the render tag logic for the same reasons.
// TODO try to get rid of .parent in favor of path in the long run
if (node.parent?.type === 'SnippetBlock') {
to_encapsulate.push({ node, block });
return true;
}
return false;
} else if (block.combinator.name === '>') {
const has_global_parent = blocks.every((block) => block.global);
if (has_global_parent || apply_selector(blocks, get_element_parent(node), to_encapsulate)) {
if (
has_global_parent ||
apply_selector(blocks, get_element_parent(node), to_encapsulate, has_render_tag)
) {
to_encapsulate.push({ node, block });
return true;
}
// The inverse of the render tag logic above: mark the node as encapsulated if it's inside a snippet block.
// May result in false positives just like the render tag logic for the same reasons.
// TODO try to get rid of .parent in favor of path in the long run
if (node.parent?.type === 'SnippetBlock') {
to_encapsulate.push({ node, block });
return true;
}
Expand All @@ -273,7 +306,7 @@ function apply_selector(blocks, node, to_encapsulate) {
return true;
}
for (const possible_sibling of siblings.keys()) {
if (apply_selector(blocks.slice(), possible_sibling, to_encapsulate)) {
if (apply_selector(blocks, possible_sibling, to_encapsulate, has_render_tag)) {
to_encapsulate.push({ node, block });
has_match = true;
}
Expand Down Expand Up @@ -514,9 +547,10 @@ function get_element_parent(node) {
// @ts-expect-error TODO figure out a more elegant solution
(parent = parent.parent) &&
parent.type !== 'RegularElement' &&
parent.type !== 'SvelteElement'
parent.type !== 'SvelteElement' &&
parent.type !== 'SnippetBlock'
);
return parent ?? null;
return parent?.type !== 'SnippetBlock' ? parent ?? null : null;
}

/**
Expand Down
@@ -0,0 +1,16 @@

div.svelte-xyz > span.svelte-xyz {
background-color: red;
}

div.svelte-xyz span.svelte-xyz {
letter-spacing: 10px;
}

div.svelte-xyz span {
text-decoration: underline;
}

p.svelte-xyz span.svelte-xyz.svelte-xyz {
background: black;
}
@@ -0,0 +1,2 @@
<div class="svelte-xyz"><span class="svelte-xyz">Hello world</span></div>
<p class="svelte-xyz"><strong><span class="svelte-xyz">Hello world</span></strong></p>
@@ -0,0 +1,31 @@
{#snippet my_snippet()}
<span>Hello world</span>
{/snippet}

<div>{@render my_snippet()}</div>

<p>
{#snippet my_snippet()}
<span>Hello world</span>
{/snippet}

<strong>{@render my_snippet()}</strong>
</p>

<style>
div > span {
background-color: red;
}

div span {
letter-spacing: 10px;
}

div :global(span) {
text-decoration: underline;
}

p span {
background: black;
}
</style>