Closed SkaterDad closed 11 months ago
I've started running this through the debugger and there is definitely a problem in the parse()
function.
I've been able to reduce the minimum markup to make this fail:
const input = `<svg>
<use></use>
</svg>`
I'm still investigating but it seems to have something to do with using a tag that can be self closing but has a closing tag provided.
further reduced the minimum case; this fails too:
const input = `<svg><use></use> </svg>`
I think this is not a problem with parse()
, it seems to be in the handling of the generated tokens, because the generated tokens seems right (I was mistaken in my last comment because I misunderstood the token types.)
@SkaterDad I've found the exact bug!
When we iterate over tokens and an encounter a tag that is in a list of ones we know can be self closing (rect
, use
, path
, etc.) we immediately assume that it is closed, and that an explicit close tag will never appear in the token list.
Then later, if we encounter that unexpected closing tag (</use>
, </rect>
, etc.) we close again. So basically these elements get put one level too high in the stack. here it is illustrated.
input string:
<svg><use></use>☺</svg>
expected tree structure:
svg
use
text (☺ character)
actual tree structure:
svg
use
text (☺ character)
That's why these html strings are throwing the multiple root elements must be wrapped in an enclosing tag
error; due to this bug we end up with a tree that has more than one root node, and hyperx
is designed to allow only one root node.
I think I can develop a fix for this, where we explicitly check to see if a self-closing tag does indeed close itself, rather than assume that based on the name of the tag. Whatever strategy we pursue to fix this would require a major version bump because it would definitely change the behavior of the parser. @goto-bus-stop what are your thoughts on this? Is this something you'd be keen to accept a PR for?
@goto-bus-stop got a fix for this! I'd love some feedback.
I'd really like to get this fix merged, if this is something we're open to.
hyperx underpins some really high value frameworks, so I'm keen to see some of these foundational problems get resolved.
I'm also happy to help with a bit of maintainership if that would be useful.
Issue
hyperx
is not allowing many SVG elements to have animation-related tags embedded in them.Both of these examples reproduce the issue:
In the 1st example, Hyperx is treating the and tags as sibling elements.
In the 2nd example, Hyperx is treating the 2nd and tags as siblings.
It seems to be a fairly common technique for animating SVGs, based on Codepen examples and articles like this: https://css-tricks.com/guide-svg-animations-smil/
Is this something that could be allowed?