fix: create <svelte:element> instances with the correct namespace #10006
Conversation
|
|
@eEQK is attempting to deploy a commit to the Svelte Team on Vercel. A member of the Team first needs to authorize it. |
| context.state.analysis.elements.push(node); | ||
|
|
||
| const is_svg_element = | ||
| (node.tag.type === 'Identifier' && node.tag.name && SVGElements.includes(node.tag.name)) || |
There was a problem hiding this comment.
This looks wrong - the name of the identifier shouldn't be used to infer whether or not this is an SVG element
There was a problem hiding this comment.
You're right, I thought it was the evaluated name, but it's just the variable name
however, how would you determine the tag value in this case? don't we have to do it at runtime?
<script>
export let a = 'svg';
export let b = 'path';
</script>
<svelte:element this={a}>
<svelte:element this={b}></svelte:element>
</svelte:element>
There was a problem hiding this comment.
We can't determine it statically in this case, and determining it at runtime would mean loading a long list of tags into the runtime, which I'm not sure if it's worth it. I'm ok with this not working for now, AFAIK this also doesn't work in Svelte 4.
There was a problem hiding this comment.
Yeah, it does not work in Svelte 4. Should I also add other tests that will be failing? using slots/snippets is not working either (both in svelte 4 and 5)
as for doing this at runtime - don't we only have to check if one of the parents is an svg without custom namespace?
There was a problem hiding this comment.
It needs to be determined at runtime, yeah, but I'm pretty sure we only need to check two cases:
- value is
"svg"— it's an SVG element - value is
"foreignObject"— it's an HTML element - otherwise inherit namespace from parent element
There was a problem hiding this comment.
I had a go at this and it's unfortantely not possible to determine in some cases:
If this is client-side only (i.e. no hydration) and the first run, then the anchor from which we'll get the parent element will not be attached to its parent yet, so the parent element is null and we can't know.
packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
Outdated
Show resolved
Hide resolved
packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
Show resolved
Hide resolved
| context.state.analysis.elements.push(node); | ||
|
|
||
| const is_svg_element = | ||
| (node.tag.type === 'Identifier' && node.tag.name && SVGElements.includes(node.tag.name)) || |
There was a problem hiding this comment.
You're right, I thought it was the evaluated name, but it's just the variable name
however, how would you determine the tag value in this case? don't we have to do it at runtime?
<script>
export let a = 'svg';
export let b = 'path';
</script>
<svelte:element this={a}>
<svelte:element this={b}></svelte:element>
</svelte:element>
|
just wondering if theres any progress on this :( , my lucide -svelte icons looks blank for couple months now xd |
In #10006 we added more elaborate mechanisms to determine which namespace a given element is in. For `<svelte:element>` we added a "can't know at compile time" case and introduced a limited heuristic into the runtime. This doesn't work for a few reasons: - we're checking the parent's namespace to determine the current namespace, but the element itself could be the one that _changes_ the namespace - as mentioned in the previous comment already, on the first render we can't do any parent analysis - it does not take into account the static component namespace The last point is the crucial one: In Svelte 4, we're falling back to the component namespace if we can't know statically - e.g. if someone added `<svelte:options namespace="svg">` then `<svelte:element>` should fall back to that namespace instead. We were not doing that up until now, which introduced a regression. Fixing this also means getting rid of the (flawed) "can't know statically" heuristic. Fixes #10858, though for a complete solution we likely need some way to tell `<svelte:element>` the namespace at runtime through a special attribute. Maybe we can use `xmlns` for that like we do in the static case
* fix: fall back to component namespace when not statically determinable In #10006 we added more elaborate mechanisms to determine which namespace a given element is in. For `<svelte:element>` we added a "can't know at compile time" case and introduced a limited heuristic into the runtime. This doesn't work for a few reasons: - we're checking the parent's namespace to determine the current namespace, but the element itself could be the one that _changes_ the namespace - as mentioned in the previous comment already, on the first render we can't do any parent analysis - it does not take into account the static component namespace The last point is the crucial one: In Svelte 4, we're falling back to the component namespace if we can't know statically - e.g. if someone added `<svelte:options namespace="svg">` then `<svelte:element>` should fall back to that namespace instead. We were not doing that up until now, which introduced a regression. Fixing this also means getting rid of the (flawed) "can't know statically" heuristic. Fixes #10858, though for a complete solution we likely need some way to tell `<svelte:element>` the namespace at runtime through a special attribute. Maybe we can use `xmlns` for that like we do in the static case * support dynamic svelte:element namespace through xmlns attribute * fix
A continuation of #9647 that closes #9645
my commit adds checking whether an element or its parent is an SVG element, and if so - sets the namespace to
namespace_svgBefore submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.Tests and linting
pnpm testand lint the project withpnpm lint