Skip to content

Commit

Permalink
chore: remove references to node.parent (#14395)
Browse files Browse the repository at this point in the history
* chore: remove references to node.parent

* types

* add ElementWithPath interface

* put path on analysis.elements instead of metadata

* Revert "put path on analysis.elements instead of metadata"

This reverts commit c0c7ab8.

* use node.metadata.path

* remove a node.parent usage

* another

* and another, albeit by replacing some bewildering code with some more bewildering code

* make loop idiomatic

* replace some more weirdo code

* simplify
  • Loading branch information
Rich-Harris authored Nov 22, 2024
1 parent dd9abea commit 0469008
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 76 deletions.
108 changes: 58 additions & 50 deletions packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,15 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule,
switch (name) {
case ' ':
case '>': {
let parent = /** @type {Compiler.TemplateNode | null} */ (element.parent);

let parent_matched = false;
let crossed_component_boundary = false;

while (parent) {
const path = element.metadata.path;
let i = path.length;

while (i--) {
const parent = path[i];

if (parent.type === 'Component' || parent.type === 'SvelteComponent') {
crossed_component_boundary = true;
}
Expand All @@ -289,8 +292,6 @@ function apply_combinator(combinator, relative_selector, parent_selectors, rule,

if (name === '>') return parent_matched;
}

parent = /** @type {Compiler.TemplateNode | null} */ (parent.parent);
}

return parent_matched || parent_selectors.every((selector) => is_global(selector, rule));
Expand Down Expand Up @@ -679,51 +680,50 @@ function relative_selector_might_apply_to_node(relative_selector, rule, element,
* @param {boolean} include_self
*/
function get_following_sibling_elements(element, include_self) {
/** @type {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | Compiler.AST.Root | null} */
let parent = get_element_parent(element);
const path = element.metadata.path;
let i = path.length;

if (!parent) {
parent = element;
while (parent?.type !== 'Root') {
parent = /** @type {any} */ (parent).parent;
/** @type {Compiler.SvelteNode} */
let start = element;
let nodes = /** @type {Compiler.SvelteNode[]} */ (
/** @type {Compiler.AST.Fragment} */ (path[0]).nodes
);

// find the set of nodes to walk...
while (i--) {
const node = path[i];

if (node.type === 'RegularElement' || node.type === 'SvelteElement') {
nodes = node.fragment.nodes;
break;
}

if (node.type !== 'Fragment') {
start = node;
}
}

/** @type {Array<Compiler.AST.RegularElement | Compiler.AST.SvelteElement>} */
const sibling_elements = [];
let found_parent = false;

for (const el of parent.fragment.nodes) {
if (found_parent) {
walk(
el,
{},
{
RegularElement(node) {
sibling_elements.push(node);
},
SvelteElement(node) {
sibling_elements.push(node);
}
}
);
} else {
/** @type {any} */
let child = element;
while (child !== el && child !== parent) {
child = child.parent;
const siblings = [];

// ...then walk them, starting from the node after the one
// containing the element in question
for (const node of nodes.slice(nodes.indexOf(start) + 1)) {
walk(node, null, {
RegularElement(node) {
siblings.push(node);
},
SvelteElement(node) {
siblings.push(node);
}
if (child === el) {
found_parent = true;
}
}
});
}

if (include_self) {
sibling_elements.push(element);
siblings.push(element);
}

return sibling_elements;
return siblings;
}

/**
Expand Down Expand Up @@ -867,15 +867,18 @@ function unquote(str) {
* @returns {Compiler.AST.RegularElement | Compiler.AST.SvelteElement | null}
*/
function get_element_parent(node) {
/** @type {Compiler.SvelteNode | null} */
let parent = node;
while (
// @ts-expect-error TODO figure out a more elegant solution
(parent = parent.parent) &&
parent.type !== 'RegularElement' &&
parent.type !== 'SvelteElement'
);
return parent ?? null;
let path = node.metadata.path;
let i = path.length;

while (i--) {
const parent = path[i];

if (parent.type === 'RegularElement' || parent.type === 'SvelteElement') {
return parent;
}
}

return null;
}

/**
Expand Down Expand Up @@ -920,7 +923,7 @@ function find_previous_sibling(node) {
while (current_node?.type === 'SlotElement') {
const slot_children = current_node.fragment.nodes;
if (slot_children.length > 0) {
current_node = slot_children.slice(-1)[0];
current_node = slot_children[slot_children.length - 1];
} else {
break;
}
Expand Down Expand Up @@ -1118,8 +1121,12 @@ function mark_as_probably(result) {
function loop_child(children, adjacent_only) {
/** @type {Map<Compiler.AST.RegularElement, NodeExistsValue>} */
const result = new Map();
for (let i = children.length - 1; i >= 0; i--) {

let i = children.length;

while (i--) {
const child = children[i];

if (child.type === 'RegularElement') {
result.set(child, NODE_DEFINITELY_EXISTS);
if (adjacent_only) {
Expand All @@ -1137,5 +1144,6 @@ function loop_child(children, adjacent_only) {
}
}
}

return result;
}
20 changes: 10 additions & 10 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -709,8 +709,8 @@ export function analyze_component(root, source, options) {
analyze_css(analysis.css.ast, analysis);

// mark nodes as scoped/unused/empty etc
for (const element of analysis.elements) {
prune(analysis.css.ast, element);
for (const node of analysis.elements) {
prune(analysis.css.ast, 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 @@ -19,11 +19,9 @@ 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);

// Special case: Move the children of <textarea> into a value attribute if they are dynamic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
*/
export function SvelteElement(node, context) {
validate_element(node, context);
check_element(node, context);

check_element(node, context.state);

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

const xmlns = /** @type {AST.Attribute & { value: [AST.Text] } | undefined} */ (
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
2 changes: 2 additions & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export namespace AST {
metadata: {
dynamic: boolean;
args_with_call_expression: Set<number>;
path: SvelteNode[];
};
}

Expand Down Expand Up @@ -344,6 +345,7 @@ export namespace AST {
*/
mathml: boolean;
scoped: boolean;
path: SvelteNode[];
};
}

Expand Down

0 comments on commit 0469008

Please sign in to comment.