Alpine: x-text doesn't work with SVG elements

Created on 28 Aug 2020  ·  10Comments  ·  Source: alpinejs/alpine

I have an SVG image that is embedded in an HTML page inside an Alpine component. As part of the SVG there's a text element that I would like to bind to some Alpine data. Unfortunately, x-text does not seem to work for SVG elements.

Example

I believe this is because x-texttries to set .innerText on the element, but SVG elements, or at least <text> don't have an .innerText attribute.

Something like this could work:

-     el.innerText = output
+     if (el.hasAttribute('innerText') {
+       el.innerText = output;
+     } else if (el.hasAttribute('textContent') {
+       el.textContent = output;
+     }

It's not the most elegant solution, but unless somebody has a better approach, I can put together a PR for this.

All 10 comments

That seems like a reasonable PR.

Make sure to include a test.

I don't know how exactly JSDOM works but I think it doesn't implement innerText correctly so you might have issues there

@philippbosch If you don't fancy doing a PR for this, I'll take a look on Sunday.

Oh, I‘ll happily put together a PR. But thanks!

Would we have issues if we always set textContent?

Would we have issues if we always set textContent?

I don't think so tbh, it should be fine. There are some differences in the behaviour of the two properties but I can't see any major problems.

That was my first thought as well. What made me shy away from that approach was that the other x-text test were failing. But that might be a limitiation in jsdom which maybe doesn't update innerText when textContent is changed.

But yeah, I can definitely look into using textContent always. I found this article which covers the differences between the two. I guess I'll start with writing some more tests first.

No worries, it was just to satisfy my curiosity. :)

No, you're totally right, @SimoTod!

Turns out that jsdom does not implement innerText at all, so the existing tests are not very helpful. We are simply testing if we can set a variable on an object … 

I'm gonna change the implementation of x-text and also the tests to use textContent instead. I'm fairly sure it will be fine, like @ryangjchandler said, because the differences between innerText and textContent are mostly about _reading_ and not about _writing_ the value.

So, this PR is getting a lot more complex when we swap innerText for textContent. But I still think it‘s the right thing to do.

jsdom has a pretty robust implementation of textContent but none at all of innerText. That means all tests that currently test something on innerText are not very realistic. Also, textContent – in jsdom and browsers – is always a string. So tests like this one need to be changed to expect "0" instead of 0: https://github.com/alpinejs/alpine/blob/2db9cfbc589a1fc4537bf85dca5beb7730d189c9/test/if.spec.js#L92

I‘m going through all the tests and will update the PR tomorrow or on Monday.

Scary changes ☹️

Was this page helpful?
0 / 5 - 0 ratings

Related issues

andruu picture andruu  ·  3Comments

mikemartin picture mikemartin  ·  3Comments

zaydek picture zaydek  ·  3Comments

dkuku picture dkuku  ·  5Comments

allmarkedup picture allmarkedup  ·  4Comments