Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2867cc1
WIP
Rich-Harris Nov 25, 2024
b84ed1e
Merge branch 'main' into better-pruning
Rich-Harris Nov 25, 2024
33026a7
link snippets to render tags and vice versa
Rich-Harris Nov 25, 2024
fdce504
WIP
Rich-Harris Nov 25, 2024
bc46771
Merge branch 'main' into better-pruning
Rich-Harris Nov 25, 2024
af7ebb0
tidy up
Rich-Harris Nov 26, 2024
22d56e8
tidy up
Rich-Harris Nov 26, 2024
e3b6e21
merge main
Rich-Harris Nov 27, 2024
344c132
update test
Rich-Harris Nov 27, 2024
1dd8772
more accurate pruning
Rich-Harris Nov 27, 2024
7b6aae1
failing test
Rich-Harris Nov 27, 2024
3707cbb
sibling stuff
Rich-Harris Nov 27, 2024
7e38ae6
merge main
Rich-Harris Nov 27, 2024
947e763
add test
Rich-Harris Nov 27, 2024
7520988
add failing test
Rich-Harris Nov 27, 2024
0dad618
another failing test
Rich-Harris Nov 27, 2024
bef6c15
more tests
Rich-Harris Nov 27, 2024
3d961f5
fix some stuff
Rich-Harris Nov 27, 2024
21fff48
skip test for now
Rich-Harris Nov 27, 2024
c7273b3
changeset
Rich-Harris Nov 27, 2024
d1d99d2
check
Rich-Harris Nov 27, 2024
9fb9881
use existing helpers
Rich-Harris Nov 27, 2024
e43baca
better comments
Rich-Harris Nov 27, 2024
300c15d
Update packages/svelte/src/compiler/phases/2-analyze/visitors/RenderT…
Rich-Harris Nov 27, 2024
a22f9d5
DRY out
Rich-Harris Nov 27, 2024
d907155
this appears to be unnecessary
Rich-Harris Nov 27, 2024
122d824
no need to pass state around
Rich-Harris Nov 27, 2024
fefc4e9
fix and simplify
Rich-Harris Nov 27, 2024
301c402
move code to where it is used
Rich-Harris Nov 27, 2024
16f28a1
this bit is nonsensical, parent cannot exist if there was no combinator
Rich-Harris Nov 27, 2024
bb5c311
simplify
Rich-Harris Nov 27, 2024
95418a6
simplify
Rich-Harris Nov 27, 2024
2a16fca
bit more
Rich-Harris Nov 27, 2024
8e3a529
simplify
Rich-Harris Nov 28, 2024
01ea8c2
handle `:has`
dummdidumm Nov 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/twelve-squids-wink.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: better account for render tags when pruning CSS
8 changes: 6 additions & 2 deletions packages/svelte/src/compiler/phases/1-parse/state/tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,10 @@ function open(parser) {
name
},
parameters: function_expression.params,
body: create_fragment()
body: create_fragment(),
metadata: {
sites: new Set()
}
});
parser.stack.push(block);
parser.fragments.push(block.body);
Expand Down Expand Up @@ -605,7 +608,8 @@ function special(parser) {
metadata: {
dynamic: false,
args_with_call_expression: new Set(),
path: []
path: [],
snippets: new Set()
}
});
}
Expand Down
138 changes: 63 additions & 75 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { get_attribute_chunks, is_text_attribute } from '../../../utils/ast.js';
/**
* @typedef {{
* element: Compiler.AST.RegularElement | Compiler.AST.SvelteElement;
* from_render_tag: boolean;
* }} State
*/
/** @typedef {NODE_PROBABLY_EXISTS | NODE_DEFINITELY_EXISTS} NodeExistsValue */
Expand Down Expand Up @@ -53,78 +52,40 @@ const nesting_selector = {
/**
*
* @param {Compiler.Css.StyleSheet} stylesheet
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.RenderTag} element
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} element
*/
export function prune(stylesheet, element) {
if (element.type === 'RenderTag') {
const parent = get_element_parent(element);
if (!parent) return;

walk(stylesheet, { element: parent, from_render_tag: true }, visitors);
} else {
walk(stylesheet, { element, from_render_tag: false }, visitors);
}
}

/** @type {Visitors<Compiler.Css.Node, State>} */
const visitors = {
Rule(node, context) {
if (node.metadata.is_global_block) {
context.visit(node.prelude);
} else {
context.next();
}
},
ComplexSelector(node, context) {
const selectors = get_relative_selectors(node);
const inner = selectors[selectors.length - 1];

if (context.state.from_render_tag) {
// We're searching for a match that crosses a render tag boundary. That means we have to both traverse up
// the element tree (to see if we find an entry point) but also remove selectors from the end (assuming
// they are part of the render tag we don't see). We do all possible combinations of both until we find a match.
/** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null} */
let element = context.state.element;

while (element) {
const selectors_to_check = selectors.slice();

while (selectors_to_check.length > 0) {
selectors_to_check.pop();

if (
apply_selector(
selectors_to_check,
/** @type {Compiler.Css.Rule} */ (node.metadata.rule),
element,
context.state
)
) {
mark(inner, element);
node.metadata.used = true;
return;
}
}

element = get_element_parent(element);
const state = { element };

walk(/** @type {Compiler.Css.Node} */ (stylesheet), state, {
Rule(node, context) {
if (node.metadata.is_global_block) {
context.visit(node.prelude);
} else {
context.next();
}
},
ComplexSelector(node, context) {
const selectors = get_relative_selectors(node);

if (
apply_selector(
selectors,
/** @type {Compiler.Css.Rule} */ (node.metadata.rule),
context.state.element,
context.state
)
) {
mark(selectors[selectors.length - 1], context.state.element);
node.metadata.used = true;
}
} else if (
apply_selector(
selectors,
/** @type {Compiler.Css.Rule} */ (node.metadata.rule),
context.state.element,
context.state
)
) {
mark(inner, context.state.element);
node.metadata.used = true;
}

// note: we don't call context.next() here, we only recurse into
// selectors that don't belong to rules (i.e. inside `:is(...)` etc)
// when we encounter them below
}
};
// note: we don't call context.next() here, we only recurse into
// selectors that don't belong to rules (i.e. inside `:is(...)` etc)
// when we encounter them below
}
});
}

/**
* Retrieves the relative selectors (minus the trailing globals) from a complex selector.
Expand All @@ -147,6 +108,7 @@ function get_relative_selectors(node) {
has_explicit_nesting_selector = true;
}
});

// if we found one we can break from the others
if (has_explicit_nesting_selector) break;
}
Expand Down Expand Up @@ -269,10 +231,15 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule,
}

if (parent.type === 'SnippetBlock') {
// We assume the snippet might be rendered in a place where the parent selectors match.
// (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 true;
for (const site of parent.metadata.sites) {
if (
apply_combinator(combinator, relative_selector, parent_selectors, rule, site, state)
) {
return true;
}
}

return false;
}

if (parent.type === 'RegularElement' || parent.type === 'SvelteElement') {
Expand Down Expand Up @@ -877,7 +844,7 @@ function get_element_parent(node) {
}

/**
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement} node
* @param {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.RenderTag | Compiler.AST.Component | Compiler.AST.SvelteComponent | Compiler.AST.SvelteSelf} node
* @param {boolean} adjacent_only
* @returns {Map<Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.SlotElement | Compiler.AST.RenderTag, NodeExistsValue>}
*/
Expand Down Expand Up @@ -929,7 +896,28 @@ function get_possible_element_siblings(node, adjacent_only) {

current = path[i];

if (!current || !is_block(current)) break;
if (!current) break;

if (
current.type === 'Component' ||
current.type === 'SvelteComponent' ||
current.type === 'SvelteSelf'
) {
continue;
}

if (current.type === 'SnippetBlock') {
for (const site of current.metadata.sites) {
const siblings = get_possible_element_siblings(site, adjacent_only);
add_to_map(siblings, result);

if (adjacent_only && current.metadata.sites.size === 1 && has_definite_elements(siblings)) {
return result;
}
}
}

if (!is_block(current)) break;

if (current.type === 'EachBlock' && fragment === current.body) {
// `{#each ...}<a /><b />{/each}` — `<b>` can be previous sibling of `<a />`
Expand Down
16 changes: 13 additions & 3 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,9 @@ export function analyze_component(root, source, options) {
: '',
keyframes: []
},
source
source,
snippet_renderers: new Map(),
snippets: new Set()
};

if (!runes) {
Expand Down Expand Up @@ -698,6 +700,16 @@ export function analyze_component(root, source, options) {
);
}

for (const [node, resolved] of analysis.snippet_renderers) {
if (!resolved) {
node.metadata.snippets = analysis.snippets;
}

for (const snippet of node.metadata.snippets) {
snippet.metadata.sites.add(node);
}
}

if (
analysis.uses_render_tags &&
(analysis.uses_slots || (!analysis.custom_element && analysis.slot_names.size > 0))
Expand Down Expand Up @@ -726,8 +738,6 @@ export function analyze_component(root, source, options) {
}

outer: for (const node of analysis.elements) {
if (node.type === 'RenderTag') continue;

if (node.metadata.scoped) {
// Dynamic elements in dom mode always use spread for attributes and therefore shouldn't have a class attribute added to them
// TODO this happens during the analysis phase, which shouldn't know anything about client vs server
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,31 @@ export function RenderTag(node, context) {
validate_opening_tag(node, context.state, '@');

node.metadata.path = [...context.path];
context.state.analysis.elements.push(node);

const callee = unwrap_optional(node.expression).callee;

node.metadata.dynamic =
callee.type !== 'Identifier' || context.state.scope.get(callee.name)?.kind !== 'normal';
const binding = callee.type === 'Identifier' ? context.state.scope.get(callee.name) : undefined;

node.metadata.dynamic = binding?.kind !== 'normal';

/** If we can't unambiguously resolve this to a declaration, we
* must assume the worst and link the render tag to every snippet
*/
let resolved =
callee.type === 'Identifier' &&
(!binding ||
binding.declaration_kind === 'import' ||
binding.kind === 'prop' ||
binding.kind === 'rest_prop' ||
binding.kind === 'bindable_prop' ||
binding?.initial?.type === 'SnippetBlock');

if (binding?.initial?.type === 'SnippetBlock') {
// if this render tag unambiguously references a local snippet, our job is easy
node.metadata.snippets.add(binding.initial);
}

context.state.analysis.snippet_renderers.set(node, resolved);
context.state.analysis.uses_render_tags = true;

const raw_args = unwrap_optional(node.expression).arguments;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import * as e from '../../../errors.js';
* @param {Context} context
*/
export function SnippetBlock(node, context) {
context.state.analysis.snippets.add(node);

validate_block_not_empty(node.body, context);

if (context.state.analysis.runes) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/** @import { AST } from '#compiler' */
/** @import { Expression } from 'estree' */
/** @import { AnalysisState, Context } from '../../types' */
import * as e from '../../../../errors.js';
import { get_attribute_expression, is_expression_attribute } from '../../../../utils/ast.js';
Expand All @@ -15,6 +16,70 @@ import { mark_subtree_dynamic } from './fragment.js';
* @param {Context} context
*/
export function visit_component(node, context) {
node.metadata.path = [...context.path];

// link this node to all the snippets that it could render, so that we can prune CSS correctly
node.metadata.snippets = new Set();

let resolved = true;

for (const attribute of node.attributes) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not really relevant, but this iterates over the attributes, and below we're iterating over them again, feels a bit wasteful - but merging them into one probably makes everything harder to reason about

/** @type {Expression | undefined} */
let expression;

if (attribute.type === 'SpreadAttribute' || attribute.type === 'BindDirective') {
resolved = false;
continue;
}

if (attribute.type !== 'Attribute' || attribute.value === true) {
continue;
}

if (Array.isArray(attribute.value)) {
if (attribute.value.length === 1 && attribute.value[0].type === 'ExpressionTag') {
expression = attribute.value[0].expression;
}
} else {
expression = attribute.value.expression;
}

if (!expression) continue;

if (expression.type === 'Identifier') {
const binding = context.state.scope.get(expression.name);

if (
binding &&
binding.declaration_kind !== 'import' &&
binding.kind !== 'prop' &&
binding.kind !== 'rest_prop' &&
binding.kind !== 'bindable_prop' &&
binding.initial?.type !== 'SnippetBlock'
) {
resolved = false;
}

if (binding?.initial?.type === 'SnippetBlock') {
node.metadata.snippets.add(binding.initial);
}
} else {
// we can't safely know which snippets this component could render,
// so we deopt. this _could_ result in unused CSS not being discarded
resolved = false;
}
}

if (resolved) {
for (const child of node.fragment.nodes) {
if (child.type === 'SnippetBlock') {
node.metadata.snippets.add(child);
}
}
}

context.state.analysis.snippet_renderers.set(node, resolved);

mark_subtree_dynamic(context.path);

for (const attribute of node.attributes) {
Expand Down
13 changes: 12 additions & 1 deletion packages/svelte/src/compiler/phases/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export interface ComponentAnalysis extends Analysis {
instance: Js;
template: Template;
/** Used for CSS pruning and scoping */
elements: Array<AST.RegularElement | AST.SvelteElement | AST.RenderTag>;
elements: Array<AST.RegularElement | AST.SvelteElement>;
runes: boolean;
exports: Array<{ name: string; alias: string | null }>;
/** Whether the component uses `$$props` */
Expand Down Expand Up @@ -72,6 +72,17 @@ export interface ComponentAnalysis extends Analysis {
keyframes: string[];
};
source: string;
/**
* Every render tag/component, and whether it could be definitively resolved or not
*/
snippet_renderers: Map<
AST.RenderTag | AST.Component | AST.SvelteComponent | AST.SvelteSelf,
boolean
>;
/**
* Every snippet that is declared locally
*/
snippets: Set<AST.SnippetBlock>;
}

declare module 'estree' {
Expand Down
Loading
Loading