Skip to content

Commit

Permalink
fix: create <svelte:element> instances with the correct namespace (#…
Browse files Browse the repository at this point in the history
…10006)

Infer namespace from parents where possible, and do a runtime-best-effort where it's not statically known
fixes #9645

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
  • Loading branch information
eEQK and dummdidumm committed Jan 29, 2024
1 parent 5ebd9e0 commit 77b4c4b
Show file tree
Hide file tree
Showing 17 changed files with 223 additions and 80 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
3 changes: 2 additions & 1 deletion packages/svelte/src/compiler/phases/1-parse/read/options.js
Original file line number Diff line number Diff line change
@@ -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-]+$/;
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/compiler/phases/1-parse/state/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ export default function tag(parser) {
name,
attributes: [],
fragment: create_fragment(true),
parent: null
parent: null,
metadata: {
svg: false
}
};

parser.allow_whitespace();
Expand Down
45 changes: 42 additions & 3 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -1104,8 +1104,47 @@ 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);

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 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;
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;
}
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -43,11 +43,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];
Expand Down Expand Up @@ -1854,7 +1850,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}`);
Expand Down Expand Up @@ -2079,9 +2075,6 @@ export const template_visitors = {
/** @type {import('estree').ExpressionStatement[]} */
const lets = [];

/** @type {string | null} */
let namespace = null;

// 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'));
Expand All @@ -2102,9 +2095,6 @@ export const template_visitors = {
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') {
Expand Down Expand Up @@ -2153,19 +2143,32 @@ 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: determine_namespace_for_children(node, context.state.metadata.namespace)
}
}
})
);
context.state.after_update.push(
b.stmt(
b.call(
'$.element',
context.state.node,
get_tag,
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)),
namespace === 'http://www.w3.org/2000/svg'
? b.literal(true)
: /** @type {any} */ (undefined)
: b.arrow([element_id, b.id('$$anchor')], b.block(inner))
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -761,10 +757,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),
Expand Down Expand Up @@ -1165,7 +1161,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}`));
Expand Down Expand Up @@ -1255,11 +1251,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_namespace_for_children(node, context.state.metadata.namespace)
};
/** @type {import('./types').ComponentContext} */
const inner_context = {
...context,
state: {
...context.state,
metadata,
template: [],
init: []
}
Expand All @@ -1276,7 +1277,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('</'),
Expand Down
62 changes: 28 additions & 34 deletions packages/svelte/src/compiler/phases/3-transform/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
}
}

Expand All @@ -229,7 +238,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;
Expand Down Expand Up @@ -279,36 +288,21 @@ function check_nodes_for_namespace(nodes, namespace) {
}

/**
* @param {import('#compiler').RegularElement} node
* 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 = path.at(-1);
if (parent?.type === 'Fragment') {
parent = path.at(-2);
}
export function determine_namespace_for_children(node, namespace) {
if (namespace === 'foreign') {
return namespace;
}

if (node.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';
}

/**
Expand Down
7 changes: 7 additions & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,13 @@ export interface SvelteElement extends BaseElement {
type: 'SvelteElement';
name: 'svelte:element';
tag: Expression;
metadata: {
/**
* `true`/`false` if this is definitely (not) an svg element.
* `null` means we can't know statically.
*/
svg: boolean | null;
};
}

export interface SvelteFragment extends BaseElement {
Expand Down
3 changes: 3 additions & 0 deletions packages/svelte/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,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';
Loading

0 comments on commit 77b4c4b

Please sign in to comment.