symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
851 stars 313 forks source link

[LiveComponent] ElementChanges fails with 'DOMException: DOMTokenList.remove: The empty string is not a valid token' if HTML class attribute contains newlines #1358

Closed manuelkiessling closed 10 months ago

manuelkiessling commented 10 months ago

I'm using UX Live Components 2.13.2 in a Symfony 5.4.33 project on top of PHP 8.2.14.

Admittingly, I'm kind of holding it wrong, so no bad feelings if this is not going to be addressed — but here we go:

Within the Twig template of a UX Live Component, if I distribute the list of CSS classes in the class attribute of an HTML element over multiple lines, like this:

<button
    class="
        tw-p-4
        tw-m-8
    "
>Foo</button>

instead of putting everything on the same line as the opening and closing quotations marks (as in class="tw-p-4 tw-m-8"), then every LiveAction call will result in the client-side JavaScript error Uncaught (in promise) DOMException: DOMTokenList.remove: The token can not contain whitespace..

According to the Firefox Developer Tools Debugger, the error is thrown by line element.classList.remove(className); of this code:

key: "applyToElement",
value: function applyToElement(element) {
  this.addedClasses.forEach(function (className) {
    element.classList.add(className);
  });
  this.removedClasses.forEach(function (className) {
    element.classList.remove(className);
  });

which is part of the _createClass(ElementChanges, ... call.

It seems as if the issues is limited to those HTML elements that are "part of the LiveAction" — I'm running into the problem with

smnandre commented 10 months ago

It seems to be indeed a syntax violation : https://developer.mozilla.org/en-US/docs/Web/API/DOMTokenList/add#exceptions

if you hard-replace in the LiveComponent JS code .add(className) and .remove(className)

with

.add(className.trim()) and .remove(className.trim())

Does it work as expected ?

smnandre commented 10 months ago

I wanted to try this, but i'm not able to reproduce your bug... could you share a bit more of your template code ? Or ideally create a little reproducer ?

manuelkiessling commented 10 months ago

@smnandre I'll try to find the time, I will also test the behaviour once a release which includes https://github.com/symfony/ux/pull/1380 is out.

smnandre commented 10 months ago

As #1380 is merged, you can already test it using 2.x-dev as constraint in your composer.json

manuelkiessling commented 10 months ago

@smnandre

I'm afraid I'm still getting the same error.

I'm now getting Uncaught (in promise) DOMException: DOMTokenList.remove: The empty string is not a valid token..

The only change I'm seeing is in the offending code that the Firefox Debugger highlights (last line):

applyToElement(element) {
        element.classList.add(...this.addedClasses);
        element.classList.remove(...this.removedClasses);

However, this.removedClasses is a Set, and I'm currently too dumb to get its entries trimmed.

smnandre commented 10 months ago

I'm sorry it did not work for you. Can you create the smallest reproducer possible and push it in a repo, i'll look right after.

As I said i did not reproduce, and even if i'm pretty sure to see where is the problem i'll need a (non)working state to ensure i fix the bug.