Skip to content
18 changes: 9 additions & 9 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ export function analyze_component(root, source, options) {

// mark nodes as scoped/unused/empty etc
for (const element of analysis.elements) {
prune(analysis.css.ast, element);
prune(analysis.css.ast, element.node);
}

const { comment } = analysis.css.ast.content;
Expand All @@ -724,18 +724,18 @@ export function analyze_component(root, source, options) {
warn_unused(analysis.css.ast);
}

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

if (element.metadata.scoped) {
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
if (element.type === 'SvelteElement' && options.generate === 'client') continue;
if (node.type === 'SvelteElement' && options.generate === 'client') continue;

/** @type {AST.Attribute | undefined} */
let class_attribute = undefined;

for (const attribute of element.attributes) {
for (const attribute of node.attributes) {
if (attribute.type === 'SpreadAttribute') {
// The spread method appends the hash to the end of the class attribute on its own
continue outer;
Expand Down Expand Up @@ -768,7 +768,7 @@ export function analyze_component(root, source, options) {
}
}
} else {
element.attributes.push(
node.attributes.push(
create_attribute('class', -1, -1, [
{
type: 'Text',
Expand All @@ -780,8 +780,8 @@ export function analyze_component(root, source, options) {
}
])
);
if (is_custom_element_node(element) && element.attributes.length === 1) {
mark_subtree_dynamic(element.metadata.path);
if (is_custom_element_node(node) && node.attributes.length === 1) {
mark_subtree_dynamic(node.metadata.path);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
* @param {Context} context
*/
export function ExpressionTag(node, context) {
if (node.parent && context.state.parent_element) {
const in_attribute = context.path.at(-1)?.type === 'Attribute';

if (!in_attribute && context.state.parent_element) {
if (!is_tag_valid_with_parent('#text', context.state.parent_element)) {
e.node_invalid_placement(node, '`{expression}`', context.state.parent_element);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
export function RegularElement(node, context) {
validate_element(node, context);

check_element(node, context.state);
check_element(node, context);

node.metadata.path = [...context.path];

context.state.analysis.elements.push(node);
context.state.analysis.elements.push({ node, path: context.path });
Copy link
Member

Choose a reason for hiding this comment

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

We already have path as metadata on the node, would be better to either reuse that or to replace usages of metadata.path where possible.
(having metadata.path might make it easier for the CSS prune stuff)

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can see we're using metadata.path in only one place. Seems like we could probably get rid of it. Using metadata.path in pruning is a possibility but it seems easier to just use context.path everywhere instead

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I take that back, I misunderstood — reverting most recent change

Copy link
Member

Choose a reason for hiding this comment

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

but it seems easier to just use context.path everywhere instead

But during pruning context.path will relate to the css tree and not the element's parent tree

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant the context.path being stored on analysis.elements. But on reflection it does make sense to use metadata.path everywhere — you probably missed that reply because GitHub's UI is a piece of shit


// Special case: Move the children of <textarea> into a value attribute if they are dynamic
if (node.name === 'textarea' && node.fragment.nodes.length > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
export function RenderTag(node, context) {
validate_opening_tag(node, context.state, '@');

context.state.analysis.elements.push(node);
context.state.analysis.elements.push({ node, path: context.path });

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
export function SvelteElement(node, context) {
validate_element(node, context);

check_element(node, context.state);
check_element(node, context);

context.state.analysis.elements.push(node);
context.state.analysis.elements.push({ node, path: context.path });

const xmlns = /** @type {AST.Attribute & { value: [AST.Text] } | undefined} */ (
node.attributes.find(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import * as e from '../../../errors.js';
* @param {Context} context
*/
export function Text(node, context) {
if (node.parent && context.state.parent_element && regex_not_whitespace.test(node.data)) {
const in_attribute = context.path.at(-1)?.type === 'Attribute';

if (!in_attribute && context.state.parent_element && regex_not_whitespace.test(node.data)) {
if (!is_tag_valid_with_parent('#text', context.state.parent_element)) {
e.node_invalid_placement(node, 'Text node', context.state.parent_element);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @import { AnalysisState } from '../../types.js' */
/** @import { Context } from '../../types.js' */
/** @import { AST, SvelteNode, TemplateNode } from '#compiler' */
/** @import { ARIARoleDefinitionKey, ARIARoleRelationConcept, ARIAProperty, ARIAPropertyDefinition, ARIARoleDefinition } from 'aria-query' */
import { roles as roles_map, aria, elementRoles } from 'aria-query';
Expand Down Expand Up @@ -582,16 +582,17 @@ function get_implicit_role(name, attribute_map) {
const invisible_elements = ['meta', 'html', 'script', 'style'];

/**
* @param {SvelteNode | null} parent
* @param {SvelteNode[]} path
* @param {string[]} elements
*/
function is_parent(parent, elements) {
while (parent) {
function is_parent(path, elements) {
let i = path.length;
while (i--) {
const parent = path[i];
if (parent.type === 'SvelteElement') return true; // unknown, play it safe, so we don't warn
if (parent.type === 'RegularElement') {
return elements.includes(parent.name);
}
parent = /** @type {TemplateNode} */ (parent).parent;
}
return false;
}
Expand Down Expand Up @@ -683,9 +684,9 @@ function get_static_text_value(attribute) {

/**
* @param {AST.RegularElement | AST.SvelteElement} node
* @param {AnalysisState} state
* @param {Context} context
*/
export function check_element(node, state) {
export function check_element(node, context) {
/** @type {Map<string, AST.Attribute>} */
const attribute_map = new Map();

Expand Down Expand Up @@ -792,7 +793,7 @@ export function check_element(node, state) {
}

// Footers and headers are special cases, and should not have redundant roles unless they are the children of sections or articles.
const is_parent_section_or_article = is_parent(node.parent, ['section', 'article']);
const is_parent_section_or_article = is_parent(context.path, ['section', 'article']);
if (!is_parent_section_or_article) {
const has_nested_redundant_role =
current_role === a11y_nested_implicit_semantics.get(node.name);
Expand Down Expand Up @@ -1114,7 +1115,7 @@ export function check_element(node, state) {
}

if (node.name === 'figcaption') {
if (!is_parent(node.parent, ['figure'])) {
if (!is_parent(context.path, ['figure'])) {
w.a11y_figcaption_parent(node);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/phases/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { AST, Binding, Css, SvelteNode } from '#compiler';
import type { AST, Binding, Css, ElementWithPath, SvelteNode } from '#compiler';
import type { Identifier, LabeledStatement, Program, VariableDeclaration } from 'estree';
import type { Scope, ScopeRoot } from './scope.js';

Expand Down 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: ElementWithPath[];
runes: boolean;
exports: Array<{ name: string; alias: string | null }>;
/** Whether the component uses `$$props` */
Expand Down
5 changes: 5 additions & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,11 @@ export type TemplateNode =

export type SvelteNode = Node | TemplateNode | AST.Fragment | Css.Node;

export interface ElementWithPath {
node: AST.RegularElement | AST.SvelteElement | AST.RenderTag;
path: SvelteNode[];
}

declare module 'estree' {
export interface BaseNode {
/** Added by the Svelte parser */
Expand Down
Loading