From 9e40f9a809a77f8d140916b6af28569a0f6145a4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 25 Nov 2023 16:03:45 -0500 Subject: [PATCH 01/16] failing test for #9645 --- .../_config.js | 12 ++++++++++++ .../main.svelte | 10 ++++++++++ 2 files changed, 22 insertions(+) create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/main.svelte diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js new file mode 100644 index 000000000000..1a4df30e9bcf --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js @@ -0,0 +1,12 @@ +import { test } from '../../test'; + +export default test({ + html: '', + + test({ assert, target }) { + const svg = target.querySelector('svg'); + const rect = target.querySelector('path'); + assert.equal(svg?.namespaceURI, 'http://www.w3.org/2000/svg'); + assert.equal(rect?.namespaceURI, 'http://www.w3.org/2000/svg'); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/main.svelte b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/main.svelte new file mode 100644 index 000000000000..00069f711c82 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/main.svelte @@ -0,0 +1,10 @@ + + + + + From 290b75e99fe5b0866d8a98c4b8b2591f1dd2eee0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 25 Nov 2023 16:26:00 -0500 Subject: [PATCH 02/16] WIP --- .../compiler/phases/1-parse/read/options.js | 3 ++- .../3-transform/client/visitors/template.js | 4 +-- packages/svelte/src/constants.js | 3 +++ packages/svelte/src/internal/client/render.js | 27 +++++++++++++------ 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/compiler/phases/1-parse/read/options.js b/packages/svelte/src/compiler/phases/1-parse/read/options.js index 31e81c7c71b9..69ca025f589e 100644 --- a/packages/svelte/src/compiler/phases/1-parse/read/options.js +++ b/packages/svelte/src/compiler/phases/1-parse/read/options.js @@ -1,3 +1,4 @@ +import { namespace_svg } from '../../../../constants.js'; import { error } from '../../../errors.js'; const regex_valid_tag_name = /^[a-zA-Z][a-zA-Z0-9]*-[a-zA-Z0-9-]+$/; @@ -156,7 +157,7 @@ export default function read_options(node) { error(attribute, 'invalid-svelte-option-namespace'); } - if (value === 'http://www.w3.org/2000/svg') { + if (value === namespace_svg) { component_options.namespace = 'svg'; } else if (value === 'html' || value === 'svg' || value === 'foreign') { component_options.namespace = value; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 6ad1026cc744..7b1d0465a580 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -2134,9 +2134,7 @@ export const template_visitors = { inner.length === 0 ? /** @type {any} */ (undefined) : b.arrow([element_id, b.id('$$anchor')], b.block(inner)), - namespace === 'http://www.w3.org/2000/svg' - ? b.literal(true) - : /** @type {any} */ (undefined) + namespace ? b.literal(namespace) : /** @type {any} */ (undefined) ) ) ); diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index 3df03ded08fd..eba78d616ab1 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -83,3 +83,6 @@ export const DOMBooleanAttributes = [ 'seamless', 'selected' ]; + +export const namespace_svg = 'http://www.w3.org/2000/svg'; +export const namespace_html = 'http://www.w3.org/1999/xhtml'; diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index a963c832c381..7840e2dfcdc3 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -19,8 +19,17 @@ import { create_dynamic_component_block, create_snippet_block } from './block.js'; -import { PassiveDelegatedEvents, DelegatedEvents, AttributeAliases } from '../../constants.js'; -import { create_fragment_from_html, insert, reconcile_html, remove } from './reconciler.js'; +import { + PassiveDelegatedEvents, + DelegatedEvents, + AttributeAliases, + namespace_svg, + namespace_html +} from '../../constants.js'; +import { + create_fragment_from_html, + insert, + reconcile_html, remove } from './reconciler.js'; import { render_effect, destroy_signal, @@ -1534,10 +1543,10 @@ function swap_block_dom(block, from, to) { * @param {Comment} anchor_node * @param {() => string} tag_fn * @param {undefined | ((element: Element, anchor: Node) => void)} render_fn - * @param {any} is_svg + * @param {string} namespace * @returns {void} */ -export function element(anchor_node, tag_fn, render_fn, is_svg = false) { +export function element(anchor_node, tag_fn, render_fn, namespace) { const block = create_dynamic_element_block(); hydrate_block_anchor(anchor_node); let has_mounted = false; @@ -1561,11 +1570,13 @@ export function element(anchor_node, tag_fn, render_fn, is_svg = false) { // Managed effect const render_effect_signal = render_effect( () => { + const ns = namespace ?? tag === 'svg' ? namespace_svg : null; + console.log(anchor_node); const next_element = tag ? current_hydration_fragment !== null ? /** @type {HTMLElement | SVGElement} */ (current_hydration_fragment[0]) - : is_svg - ? document.createElementNS('http://www.w3.org/2000/svg', tag) + : ns + ? document.createElementNS(ns, tag) : document.createElement(tag) : null; const prev_element = element; @@ -2036,7 +2047,7 @@ export function cssProps(anchor, is_html, props, component) { tag = document.createElement('div'); tag.style.display = 'contents'; } else { - tag = document.createElementNS('http://www.w3.org/2000/svg', 'g'); + tag = document.createElementNS(namespace_svg, 'g'); } insert(tag, null, anchor); component_anchor = empty(); @@ -2547,7 +2558,7 @@ export function spread_dynamic_element_attributes(node, prev, attrs, css_hash) { /** @type {Element & ElementCSSInlineStyle} */ (node), prev, attrs, - node.namespaceURI !== 'http://www.w3.org/2000/svg', + node.namespaceURI !== namespace_svg, css_hash ); } From 863ee6ec2c195ddee9e35cd5487d1555f49ad0ba Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 25 Nov 2023 16:34:46 -0500 Subject: [PATCH 03/16] remove logging --- packages/svelte/src/internal/client/render.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 7840e2dfcdc3..5e12b41739e6 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -1571,7 +1571,6 @@ export function element(anchor_node, tag_fn, render_fn, namespace) { const render_effect_signal = render_effect( () => { const ns = namespace ?? tag === 'svg' ? namespace_svg : null; - console.log(anchor_node); const next_element = tag ? current_hydration_fragment !== null ? /** @type {HTMLElement | SVGElement} */ (current_hydration_fragment[0]) From 93c168eda13d671aa8addc571b3c63026bea642f Mon Sep 17 00:00:00 2001 From: Karol Czeryna Date: Wed, 27 Dec 2023 14:34:11 +0100 Subject: [PATCH 04/16] fix svg implicit namespace --- .../src/compiler/phases/1-parse/state/element.js | 5 ++++- .../svelte/src/compiler/phases/2-analyze/index.js | 12 ++++++++++++ .../phases/3-transform/client/visitors/template.js | 12 ++++++------ packages/svelte/src/compiler/types/template.d.ts | 4 ++++ packages/svelte/src/internal/client/render.js | 9 +++------ .../svelte-element/_expected/client/index.svelte.js | 2 +- 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js index 6b38da217ef2..f48fbf9bad2d 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -140,7 +140,10 @@ export default function tag(parser) { name, attributes: [], fragment: create_fragment(true), - parent: null + parent: null, + metadata: { + svg: false + } }; parser.allow_whitespace(); diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 35b7bbc170dd..943eb9c4ced2 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1080,6 +1080,18 @@ const common_visitors = { }, SvelteElement(node, { state }) { state.analysis.elements.push(node); + + const isSvgElement = + (node.tag.type === 'Identifier' && node.tag.name && SVGElements.includes(node.tag.name)) || + (node.tag.type === 'Literal' && + node.tag.value && + SVGElements.includes(node.tag.value.toString())); + const isParentSvgElement = + node.parent && + (node.parent.type === 'SvelteElement' || node.parent.type === 'RegularElement') && + node.parent.metadata.svg; + + node.metadata.svg = (isSvgElement || isParentSvgElement) ?? false; } }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 7b1d0465a580..2613631b3aab 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -29,7 +29,8 @@ import { EACH_IS_CONTROLLED, EACH_IS_IMMUTABLE, EACH_ITEM_REACTIVE, - EACH_KEYED + EACH_KEYED, + namespace_svg } from '../../../../../constants.js'; import { regex_is_valid_identifier } from '../../../patterns.js'; import { javascript_visitors_runes } from './javascript-runes.js'; @@ -2070,12 +2071,11 @@ export const template_visitors = { } }; + namespace = node.metadata.svg ? namespace_svg : null; + for (const attribute of node.attributes) { if (attribute.type === 'Attribute') { attributes.push(attribute); - if (attribute.name === 'xmlns' && is_text_attribute(attribute)) { - namespace = attribute.value[0].data; - } } else if (attribute.type === 'SpreadAttribute') { attributes.push(attribute); } else if (attribute.type === 'ClassDirective') { @@ -2131,10 +2131,10 @@ export const template_visitors = { '$.element', context.state.node, get_tag, + b.literal(namespace), inner.length === 0 ? /** @type {any} */ (undefined) - : b.arrow([element_id, b.id('$$anchor')], b.block(inner)), - namespace ? b.literal(namespace) : /** @type {any} */ (undefined) + : b.arrow([element_id, b.id('$$anchor')], b.block(inner)) ) ) ); diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 4646dc6d32b2..7cd5ed6ac7ec 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -317,6 +317,10 @@ export interface SvelteElement extends BaseElement { type: 'SvelteElement'; name: 'svelte:element'; tag: Expression; + metadata: { + /** `true` if this is an svg element */ + svg: boolean; + }; } export interface SvelteFragment extends BaseElement { diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 5e12b41739e6..1d8235e160d8 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -26,10 +26,7 @@ import { namespace_svg, namespace_html } from '../../constants.js'; -import { - create_fragment_from_html, - insert, - reconcile_html, remove } from './reconciler.js'; +import { create_fragment_from_html, insert, reconcile_html, remove } from './reconciler.js'; import { render_effect, destroy_signal, @@ -1542,11 +1539,11 @@ function swap_block_dom(block, from, to) { /** * @param {Comment} anchor_node * @param {() => string} tag_fn + * @param {null | string} namespace * @param {undefined | ((element: Element, anchor: Node) => void)} render_fn - * @param {string} namespace * @returns {void} */ -export function element(anchor_node, tag_fn, render_fn, namespace) { +export function element(anchor_node, tag_fn, namespace, render_fn) { const block = create_dynamic_element_block(); hydrate_block_anchor(anchor_node); let has_mounted = false; diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js index 937e22410f67..faf54996a699 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js @@ -11,7 +11,7 @@ export default function Svelte_element($$anchor, $$props) { var fragment = $.comment($$anchor); var node = $.child_frag(fragment); - $.element(node, tag); + $.element(node, tag, null); $.close_frag($$anchor, fragment); $.pop(); } From 1abeb23d4e3e65286bdadc8a8e1cb81164c044fb Mon Sep 17 00:00:00 2001 From: Karol Czeryna Date: Sun, 31 Dec 2023 14:48:08 +0100 Subject: [PATCH 05/16] add another failing test for #9645 --- .../_config.js | 10 ++++++++++ .../main.svelte | 13 +++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js new file mode 100644 index 000000000000..369e0f67d373 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js @@ -0,0 +1,10 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + const svg = target.querySelector('svg'); + const path = target.querySelector('path'); + assert.equal(svg?.namespaceURI, 'http://www.w3.org/2000/svg'); + assert.equal(path?.namespaceURI, 'http://www.w3.org/2000/svg'); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte new file mode 100644 index 000000000000..713cfd780be8 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte @@ -0,0 +1,13 @@ + + + + {#each iconNode as [tag, attrs]} + + {/each} + From de4ddfc3a969a3ef24541dc1b47863d8221867af Mon Sep 17 00:00:00 2001 From: Karol Czeryna Date: Sun, 31 Dec 2023 14:49:15 +0100 Subject: [PATCH 06/16] handle edge case where svg element is inside of each block --- .../src/compiler/phases/2-analyze/index.js | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 943eb9c4ced2..3cad41f21dcb 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1078,20 +1078,31 @@ const common_visitors = { context.state.analysis.elements.push(node); }, - SvelteElement(node, { state }) { - state.analysis.elements.push(node); + SvelteElement(node, context) { + context.state.analysis.elements.push(node); - const isSvgElement = + const is_svg_element = (node.tag.type === 'Identifier' && node.tag.name && SVGElements.includes(node.tag.name)) || (node.tag.type === 'Literal' && node.tag.value && SVGElements.includes(node.tag.value.toString())); - const isParentSvgElement = - node.parent && - (node.parent.type === 'SvelteElement' || node.parent.type === 'RegularElement') && - node.parent.metadata.svg; - node.metadata.svg = (isSvgElement || isParentSvgElement) ?? false; + const metadata = node.metadata; + metadata.svg = !!is_svg_element; + + if (metadata.svg) { + return; + } + + for (const ancestor of context.path) { + if (ancestor.type === 'RegularElement' || ancestor.type === 'SvelteElement') { + metadata.svg = ancestor.metadata.svg; + + if (metadata.svg) { + break; + } + } + } } }; From 500d57c5125284a68326a0e2f47641655e5390bf Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 25 Jan 2024 20:39:16 -0500 Subject: [PATCH 07/16] fixes --- packages/svelte/src/internal/client/render.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 417746792094..af9f2e4fa1df 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -1594,7 +1594,7 @@ export function element(anchor_node, tag_fn, namespace, render_fn) { /** @type {string} */ let tag; - /** @type {null | HTMLElement | SVGElement} */ + /** @type {null | Element} */ let element = null; const element_effect = render_effect( () => { @@ -1610,13 +1610,13 @@ export function element(anchor_node, tag_fn, namespace, render_fn) { // Managed effect const render_effect_signal = render_effect( () => { - const ns = namespace ?? tag === 'svg' ? namespace_svg : null; + const ns = namespace ?? (tag === 'svg' ? namespace_svg : null); const next_element = tag ? current_hydration_fragment !== null - ? /** @type {HTMLElement | SVGElement} */ (current_hydration_fragment[0]) + ? /** @type {Element} */ (current_hydration_fragment[0]) : ns - ? document.createElementNS(ns, tag) - : document.createElement(tag) + ? document.createElementNS(ns, tag) + : document.createElement(tag) : null; const prev_element = element; if (prev_element !== null) { From ecca555dd4d95c465910905f001c67b60027bd3f Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 14:38:11 +0100 Subject: [PATCH 08/16] everything except dynamic working --- .../src/compiler/phases/2-analyze/index.js | 28 +++++++--------- .../3-transform/client/visitors/template.js | 32 ++++++++++++------- .../3-transform/server/transform-server.js | 15 +++++---- .../src/compiler/phases/3-transform/utils.js | 24 ++++++++++---- packages/svelte/src/internal/index.js | 1 + .../_config.js | 10 ------ .../main.svelte | 13 -------- .../_config.js | 20 ++++++++++++ .../main.svelte | 22 +++++++++++++ 9 files changed, 101 insertions(+), 64 deletions(-) delete mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js delete mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/main.svelte diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 020d69039b88..daf89b6141a0 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -20,7 +20,7 @@ import { warn } from '../../warnings.js'; import check_graph_for_cycles from './utils/check_graph_for_cycles.js'; import { regex_starts_with_newline } from '../patterns.js'; import { create_attribute, is_element_node } from '../nodes.js'; -import { DelegatedEvents } from '../../../constants.js'; +import { DelegatedEvents, namespace_svg } from '../../../constants.js'; import { should_proxy_or_freeze } from '../3-transform/client/utils.js'; /** @@ -1095,24 +1095,20 @@ const common_visitors = { SvelteElement(node, context) { context.state.analysis.elements.push(node); - const is_svg_element = - (node.tag.type === 'Identifier' && node.tag.name && SVGElements.includes(node.tag.name)) || - (node.tag.type === 'Literal' && - node.tag.value && - SVGElements.includes(node.tag.value.toString())); - - const metadata = node.metadata; - metadata.svg = !!is_svg_element; - - if (metadata.svg) { + if ( + context.state.options.namespace !== 'foreign' && + node.tag.type === 'Literal' && + typeof node.tag.value === 'string' && + SVGElements.includes(node.tag.value) + ) { + node.metadata.svg = true; return; } - for (const ancestor of context.path) { - if (ancestor.type === 'RegularElement' || ancestor.type === 'SvelteElement') { - metadata.svg = ancestor.metadata.svg; - - if (metadata.svg) { + for (const attribute of node.attributes) { + if (attribute.type === 'Attribute') { + if (attribute.name === 'xmlns' && is_text_attribute(attribute)) { + node.metadata.svg = attribute.value[0].data === namespace_svg; break; } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 6eb3c43a6e89..1da9dd7f6b52 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -44,11 +44,7 @@ import { sanitize_template_string } from '../../../../utils/sanitize_template_st */ function get_attribute_name(element, attribute, context) { let name = attribute.name; - if ( - element.type === 'RegularElement' && - !element.metadata.svg && - context.state.metadata.namespace !== 'foreign' - ) { + if (!element.metadata.svg && context.state.metadata.namespace !== 'foreign') { name = name.toLowerCase(); if (name in AttributeAliases) { name = AttributeAliases[name]; @@ -2080,8 +2076,11 @@ export const template_visitors = { /** @type {import('estree').ExpressionStatement[]} */ const lets = []; - /** @type {string | null} */ - let namespace = null; + const namespace = determine_element_namespace( + node, + context.state.metadata.namespace, + context.path + ); // Create a temporary context which picks up the init/update statements. // They'll then be added to the function parameter of $.element @@ -2100,8 +2099,6 @@ export const template_visitors = { } }; - namespace = node.metadata.svg ? namespace_svg : null; - for (const attribute of node.attributes) { if (attribute.type === 'Attribute') { attributes.push(attribute); @@ -2133,7 +2130,7 @@ export const template_visitors = { const get_tag = b.thunk(/** @type {import('estree').Expression} */ (context.visit(node.tag))); - if (context.state.options.dev && context.state.metadata.namespace !== 'foreign') { + if (context.state.options.dev && namespace !== 'foreign') { if (node.fragment.nodes.length > 0) { context.state.init.push(b.stmt(b.call('$.validate_void_dynamic_element', get_tag))); } @@ -2153,14 +2150,25 @@ export const template_visitors = { } } inner.push(...inner_context.state.after_update); - inner.push(...create_block(node, 'dynamic_element', node.fragment.nodes, context)); + inner.push( + ...create_block(node, 'dynamic_element', node.fragment.nodes, { + ...context, + state: { + ...context.state, + metadata: { + ...context.state.metadata, + namespace + } + } + }) + ); context.state.after_update.push( b.stmt( b.call( '$.element', context.state.node, get_tag, - b.literal(namespace), + namespace === 'svg' ? b.id('$.namespace_svg') : b.literal(null), inner.length === 0 ? /** @type {any} */ (undefined) : b.arrow([element_id, b.id('$$anchor')], b.block(inner)) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index cc1649c3702f..21c41ffee22d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -482,11 +482,7 @@ function serialize_set_binding(node, context, fallback) { */ function get_attribute_name(element, attribute, context) { let name = attribute.name; - if ( - element.type === 'RegularElement' && - !element.metadata.svg && - context.state.metadata.namespace !== 'foreign' - ) { + if (!element.metadata.svg && context.state.metadata.namespace !== 'foreign') { name = name.toLowerCase(); // don't lookup boolean aliases here, the server runtime function does only // check for the lowercase variants of boolean attributes @@ -742,10 +738,10 @@ function serialize_element_spread_attributes( } const lowercase_attributes = - element.type !== 'RegularElement' || element.metadata.svg || is_custom_element_node(element) + element.metadata.svg || (element.type === 'RegularElement' && is_custom_element_node(element)) ? b.false : b.true; - const is_svg = element.type === 'RegularElement' && element.metadata.svg ? b.true : b.false; + const is_svg = element.metadata.svg ? b.true : b.false; /** @type {import('estree').Expression[]} */ const args = [ b.array(values), @@ -1222,6 +1218,11 @@ const template_visitors = { } }, SvelteElement(node, context) { + const metadata = { + ...context.state.metadata, + namespace: determine_element_namespace(node, context.state.metadata.namespace, context.path) + }; + let tag = /** @type {import('estree').Expression} */ (context.visit(node.tag)); if (tag.type !== 'Identifier') { const tag_id = context.state.scope.generate('$$tag'); diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index a924a59e1a4b..5ad21f559419 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -229,7 +229,7 @@ export function infer_namespace(namespace, parent, nodes, path) { */ function check_nodes_for_namespace(nodes, namespace) { for (const node of nodes) { - if (node.type === 'RegularElement') { + if (node.type === 'RegularElement' || node.type === 'SvelteElement') { if (!node.metadata.svg) { namespace = 'html'; break; @@ -279,19 +279,31 @@ function check_nodes_for_namespace(nodes, namespace) { } /** - * @param {import('#compiler').RegularElement} node + * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} node * @param {import('#compiler').Namespace} namespace * @param {import('#compiler').SvelteNode[]} path * @returns {import('#compiler').Namespace} */ export function determine_element_namespace(node, namespace, path) { if (namespace !== 'foreign') { - let parent = path.at(-1); - if (parent?.type === 'Fragment') { - parent = path.at(-2); + let parent; + for (let i = path.length - 1; i >= 0; i--) { + parent = path[i]; + if (parent.type === 'Fragment') { + parent = path[i - 1]; + i--; + } + if ( + parent?.type !== 'EachBlock' && + parent?.type !== 'IfBlock' && + parent?.type !== 'KeyBlock' && + parent?.type !== 'AwaitBlock' + ) { + break; + } } - if (node.name === 'foreignObject') { + if (parent?.type === 'RegularElement' && parent.name === 'foreignObject') { return 'html'; } else if ( namespace !== 'svg' || diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index 10ab1ef4328b..4084193e7789 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -45,6 +45,7 @@ export * from './client/each.js'; export * from './client/render.js'; export * from './client/validate.js'; export { raf } from './client/timing.js'; +export { namespace_svg } from '../constants.js'; export { proxy, readonly, unstate } from './client/proxy.js'; export { create_custom_element } from './client/custom-element.js'; diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js deleted file mode 100644 index 369e0f67d373..000000000000 --- a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/_config.js +++ /dev/null @@ -1,10 +0,0 @@ -import { test } from '../../test'; - -export default test({ - test({ assert, target }) { - const svg = target.querySelector('svg'); - const path = target.querySelector('path'); - assert.equal(svg?.namespaceURI, 'http://www.w3.org/2000/svg'); - assert.equal(path?.namespaceURI, 'http://www.w3.org/2000/svg'); - } -}); diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte deleted file mode 100644 index 713cfd780be8..000000000000 --- a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace-each/main.svelte +++ /dev/null @@ -1,13 +0,0 @@ - - - - {#each iconNode as [tag, attrs]} - - {/each} - diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/_config.js b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/_config.js new file mode 100644 index 000000000000..999b8fc9b5fd --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/_config.js @@ -0,0 +1,20 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + const [svg1, svg2] = target.querySelectorAll('svg'); + const [path1, path2] = target.querySelectorAll('path'); + const [fO1, fO2] = target.querySelectorAll('foreignObject'); + const [span1, span2] = target.querySelectorAll('span'); + + assert.equal(svg1.namespaceURI, 'http://www.w3.org/2000/svg'); + assert.equal(path1.namespaceURI, 'http://www.w3.org/2000/svg'); + + assert.equal(svg2.namespaceURI, 'http://www.w3.org/2000/svg'); + assert.equal(path2.namespaceURI, 'http://www.w3.org/2000/svg'); + assert.equal(fO1.namespaceURI, 'http://www.w3.org/2000/svg'); + assert.equal(span1.namespaceURI, 'http://www.w3.org/1999/xhtml'); + assert.equal(fO2.namespaceURI, 'http://www.w3.org/2000/svg'); + assert.equal(span2.namespaceURI, 'http://www.w3.org/1999/xhtml'); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/main.svelte b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/main.svelte new file mode 100644 index 000000000000..8d62a862e85c --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-inherit-namespace/main.svelte @@ -0,0 +1,22 @@ + + + + {#each iconNode as [tag, attrs]} + + {/each} + + + + + + ok + + + {#if true} + ok + {/if} + + + From 30a81acd7d389d9535294152e80946da2835bf18 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 14:51:14 +0100 Subject: [PATCH 09/16] skip test for now, do as much as we can for runtime inference --- .../phases/3-transform/server/transform-server.js | 15 +++++++++------ packages/svelte/src/internal/client/render.js | 11 ++++++++++- .../_config.js | 4 ++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 21c41ffee22d..3aba32e22fea 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -1218,11 +1218,6 @@ const template_visitors = { } }, SvelteElement(node, context) { - const metadata = { - ...context.state.metadata, - namespace: determine_element_namespace(node, context.state.metadata.namespace, context.path) - }; - let tag = /** @type {import('estree').Expression} */ (context.visit(node.tag)); if (tag.type !== 'Identifier') { const tag_id = context.state.scope.generate('$$tag'); @@ -1237,11 +1232,16 @@ const template_visitors = { context.state.init.push(b.stmt(b.call('$.validate_dynamic_element_tag', b.thunk(tag)))); } + const metadata = { + ...context.state.metadata, + namespace: determine_element_namespace(node, context.state.metadata.namespace, context.path) + }; /** @type {import('./types').ComponentContext} */ const inner_context = { ...context, state: { ...context.state, + metadata, template: [], init: [] } @@ -1258,7 +1258,10 @@ const template_visitors = { inner_context.state.template.push(t_string('>')); const before = serialize_template(inner_context.state.template); - const main = create_block(node, node.fragment.nodes, context); + const main = create_block(node, node.fragment.nodes, { + ...context, + state: { ...context.state, metadata } + }); const after = serialize_template([ t_expression(inner_id), t_string(' { - const ns = namespace ?? (tag === 'svg' ? namespace_svg : null); + // We try our best infering the namespace in case it's not possible to determine statically, + // but on the first render on the client (without hydration) the parent will be undefined, + // since the anchor is not attached to its parent / the dom yet. + const ns = + namespace ?? + (tag === 'svg' + ? namespace_svg + : anchor_node.parentElement?.tagName === 'FOREIGNOBJECT' + ? null + : anchor_node.parentElement?.namespaceURI ?? null); const next_element = tag ? current_hydration_fragment !== null ? /** @type {Element} */ (current_hydration_fragment[0]) diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js index 1a4df30e9bcf..4ba86ba11473 100644 --- a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js @@ -1,6 +1,10 @@ import { test } from '../../test'; export default test({ + // This is skipped for now, because it's not clear how to make this work on client-side initial run: + // The anchor isn't connected to its parent at the time we can do a runtime check for the namespace, and we + // need the parent for this check. (this didn't work in Svelte 4 either) + solo: true, html: '', test({ assert, target }) { From 6a4b302abda7c8231e16e827d36a1a66d70be0cb Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 14:55:55 +0100 Subject: [PATCH 10/16] I meant skip lol --- .../samples/dynamic-element-svg-implicit-namespace/_config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js index 4ba86ba11473..12a0960d8783 100644 --- a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-svg-implicit-namespace/_config.js @@ -4,7 +4,7 @@ export default test({ // This is skipped for now, because it's not clear how to make this work on client-side initial run: // The anchor isn't connected to its parent at the time we can do a runtime check for the namespace, and we // need the parent for this check. (this didn't work in Svelte 4 either) - solo: true, + skip: true, html: '', test({ assert, target }) { From 348328fe9e6ab236488496d2367b2ec831e8a6c6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 17:59:12 +0100 Subject: [PATCH 11/16] fix --- .../src/compiler/phases/2-analyze/index.js | 22 +++++- .../3-transform/client/visitors/template.js | 11 ++- .../3-transform/server/transform-server.js | 6 +- .../src/compiler/phases/3-transform/utils.js | 70 +++++++------------ .../svelte/src/compiler/types/template.d.ts | 5 +- packages/svelte/src/internal/client/render.js | 2 +- 6 files changed, 59 insertions(+), 57 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index daf89b6141a0..82bfa62d67d6 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1109,10 +1109,30 @@ const common_visitors = { if (attribute.type === 'Attribute') { if (attribute.name === 'xmlns' && is_text_attribute(attribute)) { node.metadata.svg = attribute.value[0].data === namespace_svg; - break; + return; } } } + + for (let i = context.path.length - 1; i >= 0; i--) { + const ancestor = context.path[i]; + if ( + ancestor.type === 'Component' || + ancestor.type === 'SvelteComponent' || + ancestor.type === 'SvelteFragment' || + ancestor.type === 'SnippetBlock' + ) { + // Inside a slot or a snippet -> this resets the namespace, so we can't determine it + return; + } + if (ancestor.type === 'SvelteElement' || ancestor.type === 'RegularElement') { + node.metadata.svg = + ancestor.type === 'RegularElement' && ancestor.name === 'foreignObject' + ? false + : ancestor.metadata.svg; + return; + } + } } }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 1da9dd7f6b52..9fed2b7edbf9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -9,7 +9,7 @@ import { import { binding_properties } from '../../../bindings.js'; import { clean_nodes, - determine_element_namespace, + determine_namespace_for_children, escape_html, infer_namespace } from '../../utils.js'; @@ -1683,6 +1683,7 @@ export const template_visitors = { context.state.template.push(``); }, HtmlTag(node, context) { + console.log('html tag'); context.state.template.push(''); // push into init, so that bindings run afterwards, which might trigger another run and override hydration @@ -1851,7 +1852,7 @@ export const template_visitors = { const metadata = context.state.metadata; const child_metadata = { ...context.state.metadata, - namespace: determine_element_namespace(node, context.state.metadata.namespace, context.path) + namespace: determine_namespace_for_children(node, context.state.metadata.namespace) }; context.state.template.push(`<${node.name}`); @@ -2076,11 +2077,7 @@ export const template_visitors = { /** @type {import('estree').ExpressionStatement[]} */ const lets = []; - const namespace = determine_element_namespace( - node, - context.state.metadata.namespace, - context.path - ); + const namespace = determine_namespace_for_children(node, context.state.metadata.namespace); // Create a temporary context which picks up the init/update statements. // They'll then be added to the function parameter of $.element diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 3aba32e22fea..1d1b2f8e5303 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -15,7 +15,7 @@ import { } from '../../constants.js'; import { clean_nodes, - determine_element_namespace, + determine_namespace_for_children, escape_html, infer_namespace, transform_inspect_rune @@ -1142,7 +1142,7 @@ const template_visitors = { RegularElement(node, context) { const metadata = { ...context.state.metadata, - namespace: determine_element_namespace(node, context.state.metadata.namespace, context.path) + namespace: determine_namespace_for_children(node, context.state.metadata.namespace) }; context.state.template.push(t_string(`<${node.name}`)); @@ -1234,7 +1234,7 @@ const template_visitors = { const metadata = { ...context.state.metadata, - namespace: determine_element_namespace(node, context.state.metadata.namespace, context.path) + namespace: determine_namespace_for_children(node, context.state.metadata.namespace) }; /** @type {import('./types').ComponentContext} */ const inner_context = { diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index 5ad21f559419..ceb8838c90b0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -188,7 +188,7 @@ export function clean_nodes( } /** - * Infers the new namespace for the children of a node. + * Infers the namespace for the children of a node that should be used when creating the `$.template(...)`. * @param {import('#compiler').Namespace} namespace * @param {import('#compiler').SvelteNode} parent * @param {import('#compiler').SvelteNode[]} nodes @@ -201,19 +201,28 @@ export function infer_namespace(namespace, parent, nodes, path) { path.at(-1) : parent; - if ( - namespace !== 'foreign' && + if (namespace !== 'foreign') { + if (parent_node?.type === 'RegularElement' && parent_node.name === 'foreignObject') { + return 'html'; + } + + if (parent_node?.type === 'RegularElement' || parent_node?.type === 'SvelteElement') { + return parent_node.metadata.svg ? 'svg' : 'html'; + } + // Re-evaluate the namespace inside slot nodes that reset the namespace - (parent_node === undefined || + if ( + parent_node === undefined || parent_node.type === 'Root' || parent_node.type === 'Component' || parent_node.type === 'SvelteComponent' || parent_node.type === 'SvelteFragment' || - parent_node.type === 'SnippetBlock') - ) { - const new_namespace = check_nodes_for_namespace(nodes, 'keep'); - if (new_namespace !== 'keep' && new_namespace !== 'maybe_html') { - namespace = new_namespace; + parent_node.type === 'SnippetBlock' + ) { + const new_namespace = check_nodes_for_namespace(nodes, 'keep'); + if (new_namespace !== 'keep' && new_namespace !== 'maybe_html') { + return new_namespace; + } } } @@ -279,48 +288,21 @@ function check_nodes_for_namespace(nodes, namespace) { } /** + * Determines the namespace the children of this node are in. * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} node * @param {import('#compiler').Namespace} namespace - * @param {import('#compiler').SvelteNode[]} path * @returns {import('#compiler').Namespace} */ -export function determine_element_namespace(node, namespace, path) { - if (namespace !== 'foreign') { - let parent; - for (let i = path.length - 1; i >= 0; i--) { - parent = path[i]; - if (parent.type === 'Fragment') { - parent = path[i - 1]; - i--; - } - if ( - parent?.type !== 'EachBlock' && - parent?.type !== 'IfBlock' && - parent?.type !== 'KeyBlock' && - parent?.type !== 'AwaitBlock' - ) { - break; - } - } +export function determine_namespace_for_children(node, namespace) { + if (namespace === 'foreign') { + return namespace; + } - if (parent?.type === 'RegularElement' && parent.name === 'foreignObject') { - return 'html'; - } else if ( - namespace !== 'svg' || - parent?.type === 'Component' || - parent?.type === 'SvelteComponent' || - parent?.type === 'SvelteFragment' || - parent?.type === 'SnippetBlock' - ) { - if (node.metadata.svg) { - return 'svg'; - } else { - return 'html'; - } - } + if (node.name === 'foreignObject') { + return 'html'; } - return namespace; + return node.metadata.svg ? 'svg' : 'html'; } /** diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 91c156fe1934..6b0d7e50c406 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -309,7 +309,10 @@ export interface SvelteElement extends BaseElement { name: 'svelte:element'; tag: Expression; metadata: { - /** `true` if this is an svg element */ + /** + * `true` if this is definitely an svg element. + * `false` could still mean that it is one, but we can't know statically. + */ svg: boolean; }; } diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 03e79ab051ed..25e09e44f9f5 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -1636,7 +1636,7 @@ export function element(anchor_node, tag_fn, namespace, render_fn) { namespace ?? (tag === 'svg' ? namespace_svg - : anchor_node.parentElement?.tagName === 'FOREIGNOBJECT' + : anchor_node.parentElement?.tagName === 'foreignObject' ? null : anchor_node.parentElement?.namespaceURI ?? null); const next_element = tag From 22581cd9806931aedf8e622c00c2b5cb170ac143 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 18:02:35 +0100 Subject: [PATCH 12/16] lint --- packages/svelte/src/compiler/phases/1-parse/state/element.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js index 86c0d7f5a626..da177f596a11 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -143,7 +143,7 @@ export default function tag(parser) { metadata: { svg: false } - }; + }; parser.allow_whitespace(); From 382d49a0a449e436e1ba2343cf64748b04997f40 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 18:05:48 +0100 Subject: [PATCH 13/16] lint --- .../src/compiler/phases/3-transform/client/visitors/template.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 9fed2b7edbf9..cac2b58914ed 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -1683,7 +1683,6 @@ export const template_visitors = { context.state.template.push(``); }, HtmlTag(node, context) { - console.log('html tag'); context.state.template.push(''); // push into init, so that bindings run afterwards, which might trigger another run and override hydration From 4066f0c91fd757573244c70ec139812e66b357c1 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 18:10:02 +0100 Subject: [PATCH 14/16] regenerate, run all steps in parallel --- .github/workflows/ci.yml | 2 ++ packages/svelte/types/index.d.ts | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7c3969cd5432..45e754b35dc3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,8 @@ jobs: - name: type check run: pnpm check - name: lint + if: (${{ success() }} || ${{ failure() }}) # ensures this step runs even if previous steps fail (avoids multiple runs uncovering different issues at different steps) run: pnpm lint - name: build and check generated types + if: (${{ success() }} || ${{ failure() }}) # ensures this step runs even if previous steps fail run: pnpm build && { [ "`git status --porcelain=v1`" == "" ] || (echo "Generated types have changed — please regenerate types locally and commit the changes after you have reviewed them"; git diff; exit 1); } diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index a2daf52c4b76..550fca8a7804 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1342,6 +1342,13 @@ declare module 'svelte/compiler' { type: 'SvelteElement'; name: 'svelte:element'; tag: Expression; + metadata: { + /** + * `true` if this is definitely an svg element. + * `false` could still mean that it is one, but we can't know statically. + */ + svg: boolean; + }; } interface SvelteFragment extends BaseElement { From 6e67bcf34268cbd566cdc843b0862ae5fd876c30 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 20:41:17 +0100 Subject: [PATCH 15/16] tweak --- .../3-transform/client/visitors/template.js | 15 ++++++++------- packages/svelte/src/compiler/types/template.d.ts | 6 +++--- packages/svelte/src/internal/client/render.js | 11 +++++------ packages/svelte/src/internal/index.js | 4 ---- packages/svelte/types/index.d.ts | 6 +++--- 5 files changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index cac2b58914ed..61d426a798eb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -30,8 +30,7 @@ import { EACH_IS_CONTROLLED, EACH_IS_IMMUTABLE, EACH_ITEM_REACTIVE, - EACH_KEYED, - namespace_svg + EACH_KEYED } from '../../../../../constants.js'; import { regex_is_valid_identifier } from '../../../patterns.js'; import { javascript_visitors_runes } from './javascript-runes.js'; @@ -2076,8 +2075,6 @@ export const template_visitors = { /** @type {import('estree').ExpressionStatement[]} */ const lets = []; - const namespace = determine_namespace_for_children(node, context.state.metadata.namespace); - // Create a temporary context which picks up the init/update statements. // They'll then be added to the function parameter of $.element const element_id = b.id(context.state.scope.generate('$$element')); @@ -2126,7 +2123,7 @@ export const template_visitors = { const get_tag = b.thunk(/** @type {import('estree').Expression} */ (context.visit(node.tag))); - if (context.state.options.dev && namespace !== 'foreign') { + if (context.state.options.dev && context.state.metadata.namespace !== 'foreign') { if (node.fragment.nodes.length > 0) { context.state.init.push(b.stmt(b.call('$.validate_void_dynamic_element', get_tag))); } @@ -2153,7 +2150,7 @@ export const template_visitors = { ...context.state, metadata: { ...context.state.metadata, - namespace + namespace: determine_namespace_for_children(node, context.state.metadata.namespace) } } }) @@ -2164,7 +2161,11 @@ export const template_visitors = { '$.element', context.state.node, get_tag, - namespace === 'svg' ? b.id('$.namespace_svg') : b.literal(null), + node.metadata.svg === true + ? b.true + : node.metadata.svg === false + ? b.false + : b.literal(null), inner.length === 0 ? /** @type {any} */ (undefined) : b.arrow([element_id, b.id('$$anchor')], b.block(inner)) diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 6b0d7e50c406..f63fb369a7f3 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -310,10 +310,10 @@ export interface SvelteElement extends BaseElement { tag: Expression; metadata: { /** - * `true` if this is definitely an svg element. - * `false` could still mean that it is one, but we can't know statically. + * `true`/`false` if this is definitely (not) an svg element. + * `null` means we can't know statically. */ - svg: boolean; + svg: boolean | null; }; } diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 25e09e44f9f5..f8cd1a050519 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -1601,11 +1601,11 @@ function swap_block_dom(block, from, to) { /** * @param {Comment} anchor_node * @param {() => string} tag_fn - * @param {null | string} namespace + * @param {boolean | null} is_svg `null` == not statically known * @param {undefined | ((element: Element, anchor: Node) => void)} render_fn * @returns {void} */ -export function element(anchor_node, tag_fn, namespace, render_fn) { +export function element(anchor_node, tag_fn, is_svg, render_fn) { const block = create_dynamic_element_block(); hydrate_block_anchor(anchor_node); let has_mounted = false; @@ -1633,12 +1633,11 @@ export function element(anchor_node, tag_fn, namespace, render_fn) { // but on the first render on the client (without hydration) the parent will be undefined, // since the anchor is not attached to its parent / the dom yet. const ns = - namespace ?? - (tag === 'svg' + is_svg || tag === 'svg' ? namespace_svg - : anchor_node.parentElement?.tagName === 'foreignObject' + : is_svg === false || anchor_node.parentElement?.tagName === 'foreignObject' ? null - : anchor_node.parentElement?.namespaceURI ?? null); + : anchor_node.parentElement?.namespaceURI ?? null; const next_element = tag ? current_hydration_fragment !== null ? /** @type {Element} */ (current_hydration_fragment[0]) diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index 4084193e7789..a3a2a9293351 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -40,16 +40,12 @@ export { unwrap, freeze } from './client/runtime.js'; - export * from './client/each.js'; export * from './client/render.js'; export * from './client/validate.js'; export { raf } from './client/timing.js'; -export { namespace_svg } from '../constants.js'; export { proxy, readonly, unstate } from './client/proxy.js'; - export { create_custom_element } from './client/custom-element.js'; - export { child, child_frag, diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 550fca8a7804..64b82c4ebae6 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1344,10 +1344,10 @@ declare module 'svelte/compiler' { tag: Expression; metadata: { /** - * `true` if this is definitely an svg element. - * `false` could still mean that it is one, but we can't know statically. + * `true`/`false` if this is definitely (not) an svg element. + * `null` means we can't know statically. */ - svg: boolean; + svg: boolean | null; }; } From 77e763c966dfa7449718915964aa7f0cb8998f48 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 29 Jan 2024 20:43:54 +0100 Subject: [PATCH 16/16] fix test --- .../samples/svelte-element/_expected/client/index.svelte.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js index 3ac06131ccc8..b2734b8c4648 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js @@ -11,7 +11,7 @@ export default function Svelte_element($$anchor, $$props) { var fragment = $.comment($$anchor); var node = $.child_frag(fragment); - $.element(node, tag, null); + $.element(node, tag, false); $.close_frag($$anchor, fragment); $.pop(); } \ No newline at end of file