From 046900876f7951a9116688f936e61df69664c2c9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 22 Nov 2024 09:48:36 -0500 Subject: [PATCH] chore: remove references to node.parent (#14395) * 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 c0c7ab8bd10a812e84b267352b2afced4057e6d6. * 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 --- .../phases/2-analyze/css/css-prune.js | 108 ++++++++++-------- .../src/compiler/phases/2-analyze/index.js | 20 ++-- .../2-analyze/visitors/ExpressionTag.js | 4 +- .../2-analyze/visitors/RegularElement.js | 4 +- .../phases/2-analyze/visitors/RenderTag.js | 1 + .../2-analyze/visitors/SvelteElement.js | 4 +- .../phases/2-analyze/visitors/Text.js | 4 +- .../phases/2-analyze/visitors/shared/a11y.js | 19 +-- .../svelte/src/compiler/types/template.d.ts | 2 + 9 files changed, 90 insertions(+), 76 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js index bfb83d3e024d..9147e887e55b 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/css-prune.js @@ -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; } @@ -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)); @@ -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} */ - 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; } /** @@ -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; } /** @@ -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; } @@ -1118,8 +1121,12 @@ function mark_as_probably(result) { function loop_child(children, adjacent_only) { /** @type {Map} */ 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) { @@ -1137,5 +1144,6 @@ function loop_child(children, adjacent_only) { } } } + return result; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index a96652d60b0b..0a61fdbd88d1 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -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; @@ -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; @@ -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', @@ -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); } } } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js index 32c8d2ca3671..7e21d5ca1461 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js @@ -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); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js index 7d1c4aaeaaf0..fa6ca0f6e970 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js @@ -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