From 72963d462f627a4240c9eb1586fe9377bd74556b Mon Sep 17 00:00:00 2001 From: Edoardo Cavazza Date: Wed, 22 Jan 2025 10:26:28 +0100 Subject: [PATCH 1/6] fix: add check for is attribute --- .../phases/3-transform/client/visitors/RegularElement.js | 2 +- .../phases/3-transform/client/visitors/shared/fragment.js | 3 ++- packages/svelte/src/compiler/phases/nodes.js | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 21a78de032c4..59d2d7fd28de 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -227,7 +227,7 @@ export function RegularElement(node, context) { node_id, attributes_id, (node.metadata.svg || node.metadata.mathml || is_custom_element_node(node)) && b.true, - node.name.includes('-') && b.true, + is_custom_element_node(node) && b.true, context.state ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js index 0b4ac873428e..5bc3041ca453 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js @@ -4,6 +4,7 @@ import { cannot_be_set_statically } from '../../../../../../utils.js'; import { is_event_attribute, is_text_attribute } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; +import { is_custom_element_node } from '../../../../nodes.js'; import { build_template_chunk } from './utils.js'; /** @@ -128,7 +129,7 @@ export function process_children(nodes, initial, is_element, { visit, state }) { function is_static_element(node, state) { if (node.type !== 'RegularElement') return false; if (node.fragment.metadata.dynamic) return false; - if (node.name.includes('-')) return false; // we're setting all attributes on custom elements through properties + if (is_custom_element_node(node)) return false; // we're setting all attributes on custom elements through properties for (const attribute of node.attributes) { if (attribute.type !== 'Attribute') { diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index 5ca4ce3380a3..dc8f6b03d65b 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -23,10 +23,10 @@ export function is_element_node(node) { /** * @param {AST.RegularElement | AST.SvelteElement} node - * @returns {node is AST.RegularElement} + * @returns {boolean} */ export function is_custom_element_node(node) { - return node.type === 'RegularElement' && node.name.includes('-'); + return node.type === 'RegularElement' && (node.name.includes('-') || node.attributes.some((attr) => attr.type === 'Attribute' && attr.name === 'is')); } /** From 37e869494dbb6920942b4f7ac2984f610ac950f4 Mon Sep 17 00:00:00 2001 From: Edoardo Cavazza Date: Wed, 22 Jan 2025 11:05:23 +0100 Subject: [PATCH 2/6] chore: add tests for custom elements properties --- .../samples/custom-element-attributes/_config.js | 15 +++++++++++++++ .../samples/custom-element-attributes/main.svelte | 2 ++ 2 files changed, 17 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/custom-element-attributes/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js b/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js new file mode 100644 index 000000000000..4e550abe8843 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js @@ -0,0 +1,15 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client', 'server'], + async test({ assert, target }) { + const my_element = /** @type HTMLElement & { object: { test: true }; } */ (target.querySelector('my-element')); + const my_link = /** @type HTMLAnchorElement & { object: { test: true }; } */ (target.querySelector('a')); + assert.equal(my_element.getAttribute('string'), 'test'); + assert.equal(my_element.hasAttribute('object'), false); + assert.deepEqual(my_element.object, { test: true }); + assert.equal(my_link.getAttribute('string'), 'test'); + assert.equal(my_link.hasAttribute('object'), false); + assert.deepEqual(my_link.object, { test: true }); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/main.svelte b/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/main.svelte new file mode 100644 index 000000000000..ff94a9484c9a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/main.svelte @@ -0,0 +1,2 @@ + + From dc727e588ab5626524a382598a6a3567bb8e468b Mon Sep 17 00:00:00 2001 From: Edoardo Cavazza Date: Wed, 22 Jan 2025 12:33:59 +0100 Subject: [PATCH 3/6] Create two-doors-exercise.md --- .changeset/two-doors-exercise.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/two-doors-exercise.md diff --git a/.changeset/two-doors-exercise.md b/.changeset/two-doors-exercise.md new file mode 100644 index 000000000000..b41e582ae79a --- /dev/null +++ b/.changeset/two-doors-exercise.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: add check for `is` attribute to correctly detect custom elements From ba37bbcd1113f770ec1807145a76e28c28a7cb42 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 22 Jan 2025 12:57:33 +0100 Subject: [PATCH 4/6] chore: fix lint --- packages/svelte/src/compiler/phases/nodes.js | 6 +++++- .../samples/custom-element-attributes/_config.js | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index dc8f6b03d65b..003c3a2c4945 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -26,7 +26,11 @@ export function is_element_node(node) { * @returns {boolean} */ export function is_custom_element_node(node) { - return node.type === 'RegularElement' && (node.name.includes('-') || node.attributes.some((attr) => attr.type === 'Attribute' && attr.name === 'is')); + return ( + node.type === 'RegularElement' && + (node.name.includes('-') || + node.attributes.some((attr) => attr.type === 'Attribute' && attr.name === 'is')) + ); } /** diff --git a/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js b/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js index 4e550abe8843..118a51157eb9 100644 --- a/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/custom-element-attributes/_config.js @@ -3,8 +3,12 @@ import { test } from '../../test'; export default test({ mode: ['client', 'server'], async test({ assert, target }) { - const my_element = /** @type HTMLElement & { object: { test: true }; } */ (target.querySelector('my-element')); - const my_link = /** @type HTMLAnchorElement & { object: { test: true }; } */ (target.querySelector('a')); + const my_element = /** @type HTMLElement & { object: { test: true }; } */ ( + target.querySelector('my-element') + ); + const my_link = /** @type HTMLAnchorElement & { object: { test: true }; } */ ( + target.querySelector('a') + ); assert.equal(my_element.getAttribute('string'), 'test'); assert.equal(my_element.hasAttribute('object'), false); assert.deepEqual(my_element.object, { test: true }); From 72fab2bd4602ac423da64aec67dc7f875dc34fb1 Mon Sep 17 00:00:00 2001 From: Edoardo Cavazza Date: Wed, 22 Jan 2025 13:43:25 +0100 Subject: [PATCH 5/6] fix: improve custom element checks in html validation --- .../src/compiler/phases/2-analyze/types.d.ts | 4 +- .../2-analyze/visitors/ExpressionTag.js | 12 ++++- .../2-analyze/visitors/RegularElement.js | 40 +++++++++++--- .../phases/2-analyze/visitors/Text.js | 12 ++++- .../server/visitors/RegularElement.js | 2 + .../server/visitors/SvelteElement.js | 1 + packages/svelte/src/html-tree-validation.js | 53 +++++++++++-------- packages/svelte/src/internal/server/dev.js | 30 ++++++++--- 8 files changed, 115 insertions(+), 39 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index b4ca4dc26278..587a24f55032 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -10,10 +10,10 @@ export interface AnalysisState { options: ValidatedCompileOptions; ast_type: 'instance' | 'template' | 'module'; /** - * Tag name of the parent element. `null` if the parent is `svelte:element`, `#snippet`, a component or the root. + * The parent element. `null` if the parent is `svelte:element`, `#snippet`, a component or the root. * Parent doesn't necessarily mean direct path predecessor because there could be `#each`, `#if` etc in-between. */ - parent_element: string | null; + parent_element: AST.RegularElement | null; has_props_rune: boolean; /** Which slots the current parent component has */ component_slots: Set; 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 88fe4e6afaee..e2c322b79516 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js @@ -2,6 +2,7 @@ /** @import { Context } from '../types' */ import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js'; import * as e from '../../../errors.js'; +import { is_custom_element_node } from '../../nodes.js'; import { mark_subtree_dynamic } from './shared/fragment.js'; /** @@ -12,7 +13,16 @@ export function ExpressionTag(node, context) { const in_template = context.path.at(-1)?.type === 'Fragment'; if (in_template && context.state.parent_element) { - const message = is_tag_valid_with_parent('#text', context.state.parent_element); + const message = is_tag_valid_with_parent( + { + tag: '#text', + custom_element: false + }, + { + tag: context.state.parent_element.name, + custom_element: is_custom_element_node(context.state.parent_element) + } + ); if (message) { e.node_invalid_placement(node, message); } 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 03dfaebcb793..f9b1829f43a3 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js @@ -116,7 +116,12 @@ export function RegularElement(node, context) { if (context.state.parent_element) { let past_parent = false; let only_warn = false; - const ancestors = [context.state.parent_element]; + const ancestors = [ + { + tag: context.state.parent_element.name, + custom_element: is_custom_element_node(context.state.parent_element) + } + ]; for (let i = context.path.length - 1; i >= 0; i--) { const ancestor = context.path[i]; @@ -132,8 +137,20 @@ export function RegularElement(node, context) { } if (!past_parent) { - if (ancestor.type === 'RegularElement' && ancestor.name === context.state.parent_element) { - const message = is_tag_valid_with_parent(node.name, context.state.parent_element); + if ( + ancestor.type === 'RegularElement' && + ancestor.name === context.state.parent_element.name + ) { + const message = is_tag_valid_with_parent( + { + tag: node.name, + custom_element: is_custom_element_node(node) + }, + { + tag: context.state.parent_element.name, + custom_element: is_custom_element_node(context.state.parent_element) + } + ); if (message) { if (only_warn) { w.node_invalid_placement_ssr(node, message); @@ -145,9 +162,18 @@ export function RegularElement(node, context) { past_parent = true; } } else if (ancestor.type === 'RegularElement') { - ancestors.push(ancestor.name); - - const message = is_tag_valid_with_ancestor(node.name, ancestors); + ancestors.push({ + tag: ancestor.name, + custom_element: is_custom_element_node(ancestor) + }); + + const message = is_tag_valid_with_ancestor( + { + tag: node.name, + custom_element: is_custom_element_node(node) + }, + ancestors + ); if (message) { if (only_warn) { w.node_invalid_placement_ssr(node, message); @@ -178,7 +204,7 @@ export function RegularElement(node, context) { w.element_invalid_self_closing_tag(node, node.name); } - context.next({ ...context.state, parent_element: node.name }); + context.next({ ...context.state, parent_element: node }); // Special case: tags are valid in both the SVG and HTML namespace. // If there's no parent, look downwards to see if it's the parent of a SVG or HTML element. diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js index 363a111b7dc6..4b683ebb09cd 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js @@ -3,6 +3,7 @@ import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js'; import { regex_not_whitespace } from '../../patterns.js'; import * as e from '../../../errors.js'; +import { is_custom_element_node } from '../../nodes.js'; /** * @param {AST.Text} node @@ -12,7 +13,16 @@ export function Text(node, context) { const in_template = context.path.at(-1)?.type === 'Fragment'; if (in_template && context.state.parent_element && regex_not_whitespace.test(node.data)) { - const message = is_tag_valid_with_parent('#text', context.state.parent_element); + const message = is_tag_valid_with_parent( + { + tag: '#text', + custom_element: false + }, + { + tag: context.state.parent_element.name, + custom_element: is_custom_element_node(context.state.parent_element) + } + ); if (message) { e.node_invalid_placement(node, message); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js index af50695efa62..0fa7f8810846 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js @@ -5,6 +5,7 @@ import { is_void } from '../../../../../utils.js'; import { dev, locator } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; +import { is_custom_element_node } from '../../../nodes.js'; import { clean_nodes, determine_namespace_for_children } from '../../utils.js'; import { build_element_attributes } from './shared/element.js'; import { process_children, build_template } from './shared/utils.js'; @@ -62,6 +63,7 @@ export function RegularElement(node, context) { '$.push_element', b.id('$$payload'), b.literal(node.name), + b.literal(is_custom_element_node(node)), b.literal(location.line), b.literal(location.column) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js index 9f6faa33c8eb..0e5f3fec81c6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js @@ -47,6 +47,7 @@ export function SvelteElement(node, context) { '$.push_element', b.id('$$payload'), tag, + b.literal(false), b.literal(location.line), b.literal(location.column) ) diff --git a/packages/svelte/src/html-tree-validation.js b/packages/svelte/src/html-tree-validation.js index 4ec4ecf26f45..21555dfa9920 100644 --- a/packages/svelte/src/html-tree-validation.js +++ b/packages/svelte/src/html-tree-validation.js @@ -1,3 +1,10 @@ +/** + * @typedef {{ + * tag: string; + * custom_element: boolean; + * }} Element + */ + /** * Map of elements that have certain elements that are not allowed inside them, in the sense that they will auto-close the parent/ancestor element. * Theoretically one could take advantage of it but most of the time it will just result in confusing behavior and break when SSR'd. @@ -137,33 +144,33 @@ const disallowed_children = { /** * Returns an error message if the tag is not allowed inside the ancestor tag (which is grandparent and above) such that it will result * in the browser repairing the HTML, which will likely result in an error during hydration. - * @param {string} child_tag - * @param {string[]} ancestors All nodes starting with the parent, up until the ancestor, which means two entries minimum + * @param {Element} child_node + * @param {Element[]} ancestors All nodes starting with the parent, up until the ancestor, which means two entries minimum * @param {string} [child_loc] * @param {string} [ancestor_loc] * @returns {string | null} */ -export function is_tag_valid_with_ancestor(child_tag, ancestors, child_loc, ancestor_loc) { - if (child_tag.includes('-')) return null; // custom elements can be anything +export function is_tag_valid_with_ancestor(child_node, ancestors, child_loc, ancestor_loc) { + if (child_node.custom_element) return null; // custom elements can be anything - const ancestor_tag = ancestors[ancestors.length - 1]; + const ancestor_tag = ancestors[ancestors.length - 1].tag; const disallowed = disallowed_children[ancestor_tag]; if (!disallowed) return null; if ('reset_by' in disallowed && disallowed.reset_by) { for (let i = ancestors.length - 2; i >= 0; i--) { const ancestor = ancestors[i]; - if (ancestor.includes('-')) return null; // custom elements can be anything + if (ancestor.custom_element) return null; // custom elements can be anything // A reset means that forbidden descendants are allowed again - if (disallowed.reset_by.includes(ancestors[i])) { + if (disallowed.reset_by.includes(ancestors[i].tag)) { return null; } } } - if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) { - const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``; + if ('descendant' in disallowed && disallowed.descendant.includes(child_node.tag)) { + const child = child_loc ? `\`<${child_node.tag}>\` (${child_loc})` : `\`<${child_node.tag}>\``; const ancestor = ancestor_loc ? `\`<${ancestor_tag}>\` (${ancestor_loc})` : `\`<${ancestor_tag}>\``; @@ -177,36 +184,38 @@ export function is_tag_valid_with_ancestor(child_tag, ancestors, child_loc, ance /** * Returns an error message if the tag is not allowed inside the parent tag such that it will result * in the browser repairing the HTML, which will likely result in an error during hydration. - * @param {string} child_tag - * @param {string} parent_tag + * @param {Element} child_node + * @param {Element} parent_node * @param {string} [child_loc] * @param {string} [parent_loc] * @returns {string | null} */ -export function is_tag_valid_with_parent(child_tag, parent_tag, child_loc, parent_loc) { - if (child_tag.includes('-') || parent_tag?.includes('-')) return null; // custom elements can be anything +export function is_tag_valid_with_parent(child_node, parent_node, child_loc, parent_loc) { + if (child_node.custom_element || parent_node?.custom_element) return null; // custom elements can be anything - if (parent_tag === 'template') return null; // no errors or warning should be thrown in immediate children of template tags + if (parent_node.tag === 'template') return null; // no errors or warning should be thrown in immediate children of template tags - const disallowed = disallowed_children[parent_tag]; + const disallowed = disallowed_children[parent_node.tag]; - const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``; - const parent = parent_loc ? `\`<${parent_tag}>\` (${parent_loc})` : `\`<${parent_tag}>\``; + const child = child_loc ? `\`<${child_node.tag}>\` (${child_loc})` : `\`<${child_node.tag}>\``; + const parent = parent_loc + ? `\`<${parent_node.tag}>\` (${parent_loc})` + : `\`<${parent_node.tag}>\``; if (disallowed) { - if ('direct' in disallowed && disallowed.direct.includes(child_tag)) { + if ('direct' in disallowed && disallowed.direct.includes(child_node.tag)) { return `${child} cannot be a direct child of ${parent}`; } - if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) { + if ('descendant' in disallowed && disallowed.descendant.includes(child_node.tag)) { return `${child} cannot be a child of ${parent}`; } if ('only' in disallowed && disallowed.only) { - if (disallowed.only.includes(child_tag)) { + if (disallowed.only.includes(child_node.tag)) { return null; } else { - return `${child} cannot be a child of ${parent}. \`<${parent_tag}>\` only allows these children: ${disallowed.only.map((d) => `\`<${d}>\``).join(', ')}`; + return `${child} cannot be a child of ${parent}. \`<${parent_node.tag}>\` only allows these children: ${disallowed.only.map((d) => `\`<${d}>\``).join(', ')}`; } } } @@ -215,7 +224,7 @@ export function is_tag_valid_with_parent(child_tag, parent_tag, child_loc, paren // parsing rules - if we're down here, then none of those matched and // so we allow it only if we don't know what the parent is, as all other // cases are invalid (and we only get into this function if we know the parent). - switch (child_tag) { + switch (child_node.tag) { case 'body': case 'caption': case 'col': diff --git a/packages/svelte/src/internal/server/dev.js b/packages/svelte/src/internal/server/dev.js index ecf4e67429ac..adf68950da8a 100644 --- a/packages/svelte/src/internal/server/dev.js +++ b/packages/svelte/src/internal/server/dev.js @@ -9,6 +9,7 @@ import { current_component } from './context.js'; /** * @typedef {{ * tag: string; + * custom_element: boolean; * parent: null | Element; * filename: null | string; * line: number; @@ -60,32 +61,49 @@ export function reset_elements() { /** * @param {Payload} payload * @param {string} tag + * @param {boolean} custom_element * @param {number} line * @param {number} column */ -export function push_element(payload, tag, line, column) { +export function push_element(payload, tag, custom_element, line, column) { var filename = /** @type {Component} */ (current_component).function[FILENAME]; - var child = { tag, parent, filename, line, column }; + var child = { tag, custom_element, parent, filename, line, column }; if (parent !== null) { var ancestor = parent.parent; - var ancestors = [parent.tag]; + var ancestors = [parent]; const child_loc = filename ? `${filename}:${line}:${column}` : undefined; const parent_loc = parent.filename ? `${parent.filename}:${parent.line}:${parent.column}` : undefined; - const message = is_tag_valid_with_parent(tag, parent.tag, child_loc, parent_loc); + const message = is_tag_valid_with_parent( + { + tag, + custom_element + }, + parent, + child_loc, + parent_loc + ); if (message) print_error(payload, message); while (ancestor != null) { - ancestors.push(ancestor.tag); + ancestors.push(ancestor); const ancestor_loc = ancestor.filename ? `${ancestor.filename}:${ancestor.line}:${ancestor.column}` : undefined; - const message = is_tag_valid_with_ancestor(tag, ancestors, child_loc, ancestor_loc); + const message = is_tag_valid_with_ancestor( + { + tag, + custom_element + }, + ancestors, + child_loc, + ancestor_loc + ); if (message) print_error(payload, message); ancestor = ancestor.parent; From 57137e66184b618ffdb085e9153e0ae5d0d878ab Mon Sep 17 00:00:00 2001 From: Edoardo Cavazza Date: Wed, 22 Jan 2025 15:26:20 +0100 Subject: [PATCH 6/6] Revert "fix: improve custom element checks in html validation" This reverts commit 72fab2bd4602ac423da64aec67dc7f875dc34fb1. --- .../src/compiler/phases/2-analyze/types.d.ts | 4 +- .../2-analyze/visitors/ExpressionTag.js | 12 +---- .../2-analyze/visitors/RegularElement.js | 40 +++----------- .../phases/2-analyze/visitors/Text.js | 12 +---- .../server/visitors/RegularElement.js | 2 - .../server/visitors/SvelteElement.js | 1 - packages/svelte/src/html-tree-validation.js | 53 ++++++++----------- packages/svelte/src/internal/server/dev.js | 30 +++-------- 8 files changed, 39 insertions(+), 115 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index 587a24f55032..b4ca4dc26278 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -10,10 +10,10 @@ export interface AnalysisState { options: ValidatedCompileOptions; ast_type: 'instance' | 'template' | 'module'; /** - * The parent element. `null` if the parent is `svelte:element`, `#snippet`, a component or the root. + * Tag name of the parent element. `null` if the parent is `svelte:element`, `#snippet`, a component or the root. * Parent doesn't necessarily mean direct path predecessor because there could be `#each`, `#if` etc in-between. */ - parent_element: AST.RegularElement | null; + parent_element: string | null; has_props_rune: boolean; /** Which slots the current parent component has */ component_slots: Set; 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 e2c322b79516..88fe4e6afaee 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js @@ -2,7 +2,6 @@ /** @import { Context } from '../types' */ import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js'; import * as e from '../../../errors.js'; -import { is_custom_element_node } from '../../nodes.js'; import { mark_subtree_dynamic } from './shared/fragment.js'; /** @@ -13,16 +12,7 @@ export function ExpressionTag(node, context) { const in_template = context.path.at(-1)?.type === 'Fragment'; if (in_template && context.state.parent_element) { - const message = is_tag_valid_with_parent( - { - tag: '#text', - custom_element: false - }, - { - tag: context.state.parent_element.name, - custom_element: is_custom_element_node(context.state.parent_element) - } - ); + const message = is_tag_valid_with_parent('#text', context.state.parent_element); if (message) { e.node_invalid_placement(node, message); } 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 f9b1829f43a3..03dfaebcb793 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js @@ -116,12 +116,7 @@ export function RegularElement(node, context) { if (context.state.parent_element) { let past_parent = false; let only_warn = false; - const ancestors = [ - { - tag: context.state.parent_element.name, - custom_element: is_custom_element_node(context.state.parent_element) - } - ]; + const ancestors = [context.state.parent_element]; for (let i = context.path.length - 1; i >= 0; i--) { const ancestor = context.path[i]; @@ -137,20 +132,8 @@ export function RegularElement(node, context) { } if (!past_parent) { - if ( - ancestor.type === 'RegularElement' && - ancestor.name === context.state.parent_element.name - ) { - const message = is_tag_valid_with_parent( - { - tag: node.name, - custom_element: is_custom_element_node(node) - }, - { - tag: context.state.parent_element.name, - custom_element: is_custom_element_node(context.state.parent_element) - } - ); + if (ancestor.type === 'RegularElement' && ancestor.name === context.state.parent_element) { + const message = is_tag_valid_with_parent(node.name, context.state.parent_element); if (message) { if (only_warn) { w.node_invalid_placement_ssr(node, message); @@ -162,18 +145,9 @@ export function RegularElement(node, context) { past_parent = true; } } else if (ancestor.type === 'RegularElement') { - ancestors.push({ - tag: ancestor.name, - custom_element: is_custom_element_node(ancestor) - }); - - const message = is_tag_valid_with_ancestor( - { - tag: node.name, - custom_element: is_custom_element_node(node) - }, - ancestors - ); + ancestors.push(ancestor.name); + + const message = is_tag_valid_with_ancestor(node.name, ancestors); if (message) { if (only_warn) { w.node_invalid_placement_ssr(node, message); @@ -204,7 +178,7 @@ export function RegularElement(node, context) { w.element_invalid_self_closing_tag(node, node.name); } - context.next({ ...context.state, parent_element: node }); + context.next({ ...context.state, parent_element: node.name }); // Special case: tags are valid in both the SVG and HTML namespace. // If there's no parent, look downwards to see if it's the parent of a SVG or HTML element. diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js index 4b683ebb09cd..363a111b7dc6 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js @@ -3,7 +3,6 @@ import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js'; import { regex_not_whitespace } from '../../patterns.js'; import * as e from '../../../errors.js'; -import { is_custom_element_node } from '../../nodes.js'; /** * @param {AST.Text} node @@ -13,16 +12,7 @@ export function Text(node, context) { const in_template = context.path.at(-1)?.type === 'Fragment'; if (in_template && context.state.parent_element && regex_not_whitespace.test(node.data)) { - const message = is_tag_valid_with_parent( - { - tag: '#text', - custom_element: false - }, - { - tag: context.state.parent_element.name, - custom_element: is_custom_element_node(context.state.parent_element) - } - ); + const message = is_tag_valid_with_parent('#text', context.state.parent_element); if (message) { e.node_invalid_placement(node, message); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js index 0fa7f8810846..af50695efa62 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js @@ -5,7 +5,6 @@ import { is_void } from '../../../../../utils.js'; import { dev, locator } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; -import { is_custom_element_node } from '../../../nodes.js'; import { clean_nodes, determine_namespace_for_children } from '../../utils.js'; import { build_element_attributes } from './shared/element.js'; import { process_children, build_template } from './shared/utils.js'; @@ -63,7 +62,6 @@ export function RegularElement(node, context) { '$.push_element', b.id('$$payload'), b.literal(node.name), - b.literal(is_custom_element_node(node)), b.literal(location.line), b.literal(location.column) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js index 0e5f3fec81c6..9f6faa33c8eb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js @@ -47,7 +47,6 @@ export function SvelteElement(node, context) { '$.push_element', b.id('$$payload'), tag, - b.literal(false), b.literal(location.line), b.literal(location.column) ) diff --git a/packages/svelte/src/html-tree-validation.js b/packages/svelte/src/html-tree-validation.js index 21555dfa9920..4ec4ecf26f45 100644 --- a/packages/svelte/src/html-tree-validation.js +++ b/packages/svelte/src/html-tree-validation.js @@ -1,10 +1,3 @@ -/** - * @typedef {{ - * tag: string; - * custom_element: boolean; - * }} Element - */ - /** * Map of elements that have certain elements that are not allowed inside them, in the sense that they will auto-close the parent/ancestor element. * Theoretically one could take advantage of it but most of the time it will just result in confusing behavior and break when SSR'd. @@ -144,33 +137,33 @@ const disallowed_children = { /** * Returns an error message if the tag is not allowed inside the ancestor tag (which is grandparent and above) such that it will result * in the browser repairing the HTML, which will likely result in an error during hydration. - * @param {Element} child_node - * @param {Element[]} ancestors All nodes starting with the parent, up until the ancestor, which means two entries minimum + * @param {string} child_tag + * @param {string[]} ancestors All nodes starting with the parent, up until the ancestor, which means two entries minimum * @param {string} [child_loc] * @param {string} [ancestor_loc] * @returns {string | null} */ -export function is_tag_valid_with_ancestor(child_node, ancestors, child_loc, ancestor_loc) { - if (child_node.custom_element) return null; // custom elements can be anything +export function is_tag_valid_with_ancestor(child_tag, ancestors, child_loc, ancestor_loc) { + if (child_tag.includes('-')) return null; // custom elements can be anything - const ancestor_tag = ancestors[ancestors.length - 1].tag; + const ancestor_tag = ancestors[ancestors.length - 1]; const disallowed = disallowed_children[ancestor_tag]; if (!disallowed) return null; if ('reset_by' in disallowed && disallowed.reset_by) { for (let i = ancestors.length - 2; i >= 0; i--) { const ancestor = ancestors[i]; - if (ancestor.custom_element) return null; // custom elements can be anything + if (ancestor.includes('-')) return null; // custom elements can be anything // A reset means that forbidden descendants are allowed again - if (disallowed.reset_by.includes(ancestors[i].tag)) { + if (disallowed.reset_by.includes(ancestors[i])) { return null; } } } - if ('descendant' in disallowed && disallowed.descendant.includes(child_node.tag)) { - const child = child_loc ? `\`<${child_node.tag}>\` (${child_loc})` : `\`<${child_node.tag}>\``; + if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) { + const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``; const ancestor = ancestor_loc ? `\`<${ancestor_tag}>\` (${ancestor_loc})` : `\`<${ancestor_tag}>\``; @@ -184,38 +177,36 @@ export function is_tag_valid_with_ancestor(child_node, ancestors, child_loc, anc /** * Returns an error message if the tag is not allowed inside the parent tag such that it will result * in the browser repairing the HTML, which will likely result in an error during hydration. - * @param {Element} child_node - * @param {Element} parent_node + * @param {string} child_tag + * @param {string} parent_tag * @param {string} [child_loc] * @param {string} [parent_loc] * @returns {string | null} */ -export function is_tag_valid_with_parent(child_node, parent_node, child_loc, parent_loc) { - if (child_node.custom_element || parent_node?.custom_element) return null; // custom elements can be anything +export function is_tag_valid_with_parent(child_tag, parent_tag, child_loc, parent_loc) { + if (child_tag.includes('-') || parent_tag?.includes('-')) return null; // custom elements can be anything - if (parent_node.tag === 'template') return null; // no errors or warning should be thrown in immediate children of template tags + if (parent_tag === 'template') return null; // no errors or warning should be thrown in immediate children of template tags - const disallowed = disallowed_children[parent_node.tag]; + const disallowed = disallowed_children[parent_tag]; - const child = child_loc ? `\`<${child_node.tag}>\` (${child_loc})` : `\`<${child_node.tag}>\``; - const parent = parent_loc - ? `\`<${parent_node.tag}>\` (${parent_loc})` - : `\`<${parent_node.tag}>\``; + const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``; + const parent = parent_loc ? `\`<${parent_tag}>\` (${parent_loc})` : `\`<${parent_tag}>\``; if (disallowed) { - if ('direct' in disallowed && disallowed.direct.includes(child_node.tag)) { + if ('direct' in disallowed && disallowed.direct.includes(child_tag)) { return `${child} cannot be a direct child of ${parent}`; } - if ('descendant' in disallowed && disallowed.descendant.includes(child_node.tag)) { + if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) { return `${child} cannot be a child of ${parent}`; } if ('only' in disallowed && disallowed.only) { - if (disallowed.only.includes(child_node.tag)) { + if (disallowed.only.includes(child_tag)) { return null; } else { - return `${child} cannot be a child of ${parent}. \`<${parent_node.tag}>\` only allows these children: ${disallowed.only.map((d) => `\`<${d}>\``).join(', ')}`; + return `${child} cannot be a child of ${parent}. \`<${parent_tag}>\` only allows these children: ${disallowed.only.map((d) => `\`<${d}>\``).join(', ')}`; } } } @@ -224,7 +215,7 @@ export function is_tag_valid_with_parent(child_node, parent_node, child_loc, par // parsing rules - if we're down here, then none of those matched and // so we allow it only if we don't know what the parent is, as all other // cases are invalid (and we only get into this function if we know the parent). - switch (child_node.tag) { + switch (child_tag) { case 'body': case 'caption': case 'col': diff --git a/packages/svelte/src/internal/server/dev.js b/packages/svelte/src/internal/server/dev.js index adf68950da8a..ecf4e67429ac 100644 --- a/packages/svelte/src/internal/server/dev.js +++ b/packages/svelte/src/internal/server/dev.js @@ -9,7 +9,6 @@ import { current_component } from './context.js'; /** * @typedef {{ * tag: string; - * custom_element: boolean; * parent: null | Element; * filename: null | string; * line: number; @@ -61,49 +60,32 @@ export function reset_elements() { /** * @param {Payload} payload * @param {string} tag - * @param {boolean} custom_element * @param {number} line * @param {number} column */ -export function push_element(payload, tag, custom_element, line, column) { +export function push_element(payload, tag, line, column) { var filename = /** @type {Component} */ (current_component).function[FILENAME]; - var child = { tag, custom_element, parent, filename, line, column }; + var child = { tag, parent, filename, line, column }; if (parent !== null) { var ancestor = parent.parent; - var ancestors = [parent]; + var ancestors = [parent.tag]; const child_loc = filename ? `${filename}:${line}:${column}` : undefined; const parent_loc = parent.filename ? `${parent.filename}:${parent.line}:${parent.column}` : undefined; - const message = is_tag_valid_with_parent( - { - tag, - custom_element - }, - parent, - child_loc, - parent_loc - ); + const message = is_tag_valid_with_parent(tag, parent.tag, child_loc, parent_loc); if (message) print_error(payload, message); while (ancestor != null) { - ancestors.push(ancestor); + ancestors.push(ancestor.tag); const ancestor_loc = ancestor.filename ? `${ancestor.filename}:${ancestor.line}:${ancestor.column}` : undefined; - const message = is_tag_valid_with_ancestor( - { - tag, - custom_element - }, - ancestors, - child_loc, - ancestor_loc - ); + const message = is_tag_valid_with_ancestor(tag, ancestors, child_loc, ancestor_loc); if (message) print_error(payload, message); ancestor = ancestor.parent;