Repository: symfony/ux
Pull request #1380 (2024-01-05 14:13:42)

[LiveComponent] Fix trim classes in live_controller.js
 Fix trim classes in live_controller.js .. . and should solve #1358 

Add various CS / minor fixes while i looked at the code... you may refuse as it's way over the original scope :)




Patch:
diff --git a/src/LiveComponent/assets/dist/HookManager.d.ts b/src/LiveComponent/assets/dist/HookManager.d.ts
index 5980af07..0db9a9e3 100644
--- a/src/LiveComponent/assets/dist/HookManager.d.ts
+++ b/src/LiveComponent/assets/dist/HookManager.d.ts
@@ -1,6 +1,5 @@
 export default class {
     private hooks;
-    constructor();
     register(hookName: string, callback: () => void): void;
     unregister(hookName: string, callback: () => void): void;
     triggerHook(hookName: string, ...args: any[]): void;
diff --git a/src/LiveComponent/assets/dist/live_controller.js b/src/LiveComponent/assets/dist/live_controller.js
index 70c7e543..5997d564 100644
--- a/src/LiveComponent/assets/dist/live_controller.js
+++ b/src/LiveComponent/assets/dist/live_controller.js
@@ -139,10 +139,13 @@ function parseDirectives(content) {
 function combineSpacedArray(parts) {
     const finalParts = [];
     parts.forEach((part) => {
-        finalParts.push(...part.split(' '));
+        finalParts.push(...trimAll(part).split(' '));
     });
     return finalParts;
 }
+function trimAll(str) {
+    return str.replace(/[\s]+/g, ' ').trim();
+}
 function normalizeModelName(model) {
     return (model
         .replace(/\[]$/, '')
@@ -381,8 +384,7 @@ class ValueStore {
     }
     set(name, value) {
         const normalizedName = normalizeModelName(name);
-        const currentValue = this.get(normalizedName);
-        if (currentValue === value) {
+        if (this.get(normalizedName) === value) {
             return false;
         }
         this.dirtyProps[normalizedName] = value;
@@ -1392,9 +1394,7 @@ class HookManager {
     }
     triggerHook(hookName, ...args) {
         const hooks = this.hooks.get(hookName) || [];
-        hooks.forEach((callback) => {
-            callback(...args);
-        });
+        hooks.forEach((callback) => callback(...args));
     }
 }
 
@@ -1449,18 +1449,10 @@ class ChangingItemsTracker {
         }
     }
     getChangedItems() {
-        const changedItems = [];
-        this.changedItems.forEach((value, key) => {
-            changedItems.push({ name: key, value: value.new });
-        });
-        return changedItems;
+        return Array.from(this.changedItems, ([name, { new: value }]) => ({ name, value }));
     }
     getRemovedItems() {
-        const removedItems = [];
-        this.removedItems.forEach((value, key) => {
-            removedItems.push(key);
-        });
-        return removedItems;
+        return Array.from(this.removedItems.keys());
     }
     isEmpty() {
         return this.changedItems.size === 0 && this.removedItems.size === 0;
@@ -1469,27 +1461,19 @@ class ChangingItemsTracker {
 
 class ElementChanges {
     constructor() {
-        this.addedClasses = [];
-        this.removedClasses = [];
+        this.addedClasses = new Set();
+        this.removedClasses = new Set();
         this.styleChanges = new ChangingItemsTracker();
         this.attributeChanges = new ChangingItemsTracker();
     }
     addClass(className) {
-        if (this.removedClasses.includes(className)) {
-            this.removedClasses = this.removedClasses.filter((name) => name !== className);
-            return;
-        }
-        if (!this.addedClasses.includes(className)) {
-            this.addedClasses.push(className);
+        if (!this.removedClasses.delete(className)) {
+            this.addedClasses.add(className);
         }
     }
     removeClass(className) {
-        if (this.addedClasses.includes(className)) {
-            this.addedClasses = this.addedClasses.filter((name) => name !== className);
-            return;
-        }
-        if (!this.removedClasses.includes(className)) {
-            this.removedClasses.push(className);
+        if (!this.addedClasses.delete(className)) {
+            this.removedClasses.add(className);
         }
     }
     addStyle(styleName, newValue, originalValue) {
@@ -1505,10 +1489,10 @@ class ElementChanges {
         this.attributeChanges.removeItem(attributeName, originalValue);
     }
     getAddedClasses() {
-        return this.addedClasses;
+        return [...this.addedClasses];
     }
     getRemovedClasses() {
-        return this.removedClasses;
+        return [...this.removedClasses];
     }
     getChangedStyles() {
         return this.styleChanges.getChangedItems();
@@ -1523,12 +1507,8 @@ class ElementChanges {
         return this.attributeChanges.getRemovedItems();
     }
     applyToElement(element) {
-        this.addedClasses.forEach((className) => {
-            element.classList.add(className);
-        });
-        this.removedClasses.forEach((className) => {
-            element.classList.remove(className);
-        });
+        element.classList.add(...this.addedClasses);
+        element.classList.remove(...this.removedClasses);
         this.styleChanges.getChangedItems().forEach((change) => {
             element.style.setProperty(change.name, change.value);
             return;
@@ -1544,8 +1524,8 @@ class ElementChanges {
         });
     }
     isEmpty() {
-        return (this.addedClasses.length === 0 &&
-            this.removedClasses.length === 0 &&
+        return (this.addedClasses.size === 0 &&
+            this.removedClasses.size === 0 &&
             this.styleChanges.isEmpty() &&
             this.attributeChanges.isEmpty());
     }
@@ -2381,9 +2361,7 @@ class LoadingPlugin {
         let loadingDirective;
         switch (finalAction) {
             case 'show':
-                loadingDirective = () => {
-                    this.showElement(element);
-                };
+                loadingDirective = () => this.showElement(element);
                 break;
             case 'hide':
                 loadingDirective = () => this.hideElement(element);
@@ -2443,7 +2421,7 @@ class LoadingPlugin {
     removeClass(element, classes) {
         element.classList.remove(...combineSpacedArray(classes));
         if (element.classList.length === 0) {
-            this.removeAttributes(element, ['class']);
+            element.removeAttribute('class');
         }
     }
     addAttributes(element, attributes) {
diff --git a/src/LiveComponent/assets/dist/string_utils.d.ts b/src/LiveComponent/assets/dist/string_utils.d.ts
index cf965ff2..87d65fe5 100644
--- a/src/LiveComponent/assets/dist/string_utils.d.ts
+++ b/src/LiveComponent/assets/dist/string_utils.d.ts
@@ -1,2 +1,3 @@
 export declare function combineSpacedArray(parts: Array<string>): string[];
+export declare function trimAll(str: string): string;
 export declare function normalizeModelName(model: string): string;
diff --git a/src/LiveComponent/assets/src/Component/ValueStore.ts b/src/LiveComponent/assets/src/Component/ValueStore.ts
index ae073743..3f5afdd8 100644
--- a/src/LiveComponent/assets/src/Component/ValueStore.ts
+++ b/src/LiveComponent/assets/src/Component/ValueStore.ts
@@ -69,10 +69,7 @@ export default class {
      */
     set(name: string, value: any): boolean {
         const normalizedName = normalizeModelName(name);
-
-        const currentValue = this.get(normalizedName);
-
-        if (currentValue === value) {
+        if (this.get(normalizedName) === value) {
             return false;
         }
 
diff --git a/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts b/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts
index 0c26dae2..b9ce269a 100644
--- a/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts
+++ b/src/LiveComponent/assets/src/Component/plugins/LoadingPlugin.ts
@@ -3,7 +3,7 @@ import {
     DirectiveModifier,
     parseDirectives
 } from '../../Directive/directives_parser';
-import { combineSpacedArray}  from '../../string_utils';
+import { combineSpacedArray } from '../../string_utils';
 import BackendRequest from '../../Backend/BackendRequest';
 import Component from '../../Component';
 import { PluginInterface } from './PluginInterface';
@@ -64,11 +64,9 @@ export default class implements PluginInterface {
 
         const validModifiers: Map<string, (modifier: DirectiveModifier) => void> = new Map();
         validModifiers.set('delay', (modifier: DirectiveModifier) => {
-            // if loading has *stopped*, the delay modifier has no effect
             if (!isLoading) {
                 return;
             }
-
             delay = modifier.value ? parseInt(modifier.value) : 200;
         });
         validModifiers.set('action', (modifier: DirectiveModifier) => {
@@ -110,9 +108,7 @@ export default class implements PluginInterface {
 
         switch (finalAction) {
             case 'show':
-                loadingDirective = () => {
-                    this.showElement(element)
-                };
+                loadingDirective = () => this.showElement(element);
                 break;
 
             case 'hide':
@@ -154,12 +150,10 @@ export default class implements PluginInterface {
 
     getLoadingDirectives(element: HTMLElement|SVGElement) {
         const loadingDirectives: ElementLoadingDirectives[] = [];
-
         let matchingElements = element.querySelectorAll('[data-loading]');
 
         // querySelectorAll doesn't include the element itself
         if (element.hasAttribute('data-loading')) {
-            // add element at the beginning of matchingElements
             matchingElements = [element, ...matchingElements];
         }
 
@@ -194,10 +188,9 @@ export default class implements PluginInterface {
 
     private removeClass(element: HTMLElement|SVGElement, classes: string[]) {
         element.classList.remove(...combineSpacedArray(classes));
-
-        // remove empty class="" to avoid morphdom "diff" problem
         if (element.classList.length === 0) {
-            this.removeAttributes(element, ['class']);
+            // remove empty class="" to avoid morphdom "diff" problem
+            element.removeAttribute('class');
         }
     }
 
@@ -214,7 +207,7 @@ export default class implements PluginInterface {
     }
 }
 
-const parseLoadingAction = function(action: string, isLoading: boolean) {
+const parseLoadingAction = function (action: string, isLoading: boolean) {
     switch (action) {
         case 'show':
             return isLoading ? 'show' : 'hide';
diff --git a/src/LiveComponent/assets/src/HookManager.ts b/src/LiveComponent/assets/src/HookManager.ts
index 835bfb2f..9e0dacd8 100644
--- a/src/LiveComponent/assets/src/HookManager.ts
+++ b/src/LiveComponent/assets/src/HookManager.ts
@@ -1,9 +1,5 @@
 export default class {
-    private hooks: Map<string, Array<(...args: any[]) => void>>;
-
-    constructor() {
-        this.hooks = new Map();
-    }
+    private hooks: Map<string, Array<(...args: any[]) => void>> = new Map();
 
     register(hookName: string, callback: () => void): void {
         const hooks = this.hooks.get(hookName) || [];
@@ -13,7 +9,6 @@ export default class {
 
     unregister(hookName: string, callback: () => void): void {
         const hooks = this.hooks.get(hookName) || [];
-
         const index = hooks.indexOf(callback);
         if (index === -1) {
             return;
@@ -25,8 +20,6 @@ export default class {
 
     triggerHook(hookName: string, ...args: any[]): void {
         const hooks = this.hooks.get(hookName) || [];
-        hooks.forEach((callback) => {
-            callback(...args);
-        });
+        hooks.forEach((callback) => callback(...args));
     }
 }
diff --git a/src/LiveComponent/assets/src/Rendering/ChangingItemsTracker.ts b/src/LiveComponent/assets/src/Rendering/ChangingItemsTracker.ts
index 64663ca6..b6b22fee 100644
--- a/src/LiveComponent/assets/src/Rendering/ChangingItemsTracker.ts
+++ b/src/LiveComponent/assets/src/Rendering/ChangingItemsTracker.ts
@@ -62,23 +62,11 @@ export default class {
     }
 
     getChangedItems(): { name: string, value: string }[] {
-        const changedItems: { name: string, value: string }[] = [];
-
-        this.changedItems.forEach((value, key) => {
-            changedItems.push({ name: key, value: value.new });
-        });
-
-        return changedItems;
+        return Array.from(this.changedItems, ([name, { new: value }]) => ({ name, value }));
     }
 
     getRemovedItems(): string[] {
-        const removedItems: string[] = [];
-
-        this.removedItems.forEach((value, key) => {
-            removedItems.push(key);
-        });
-
-        return removedItems;
+        return Array.from(this.removedItems.keys());
     }
 
     isEmpty(): boolean {
diff --git a/src/LiveComponent/assets/src/Rendering/ElementChanges.ts b/src/LiveComponent/assets/src/Rendering/ElementChanges.ts
index 503f8440..66742225 100644
--- a/src/LiveComponent/assets/src/Rendering/ElementChanges.ts
+++ b/src/LiveComponent/assets/src/Rendering/ElementChanges.ts
@@ -4,35 +4,21 @@ import ChangingItemsTracker from './ChangingItemsTracker';
  * Tracks attribute changes for a specific element.
  */
 export default class ElementChanges {
-    private addedClasses: string[] = [];
-    private removedClasses: string[] = [];
+    private addedClasses: Set<string> = new Set();
+    private removedClasses: Set<string> = new Set();
 
     private styleChanges: ChangingItemsTracker = new ChangingItemsTracker();
     private attributeChanges: ChangingItemsTracker = new ChangingItemsTracker();
 
     addClass(className: string) {
-        if (this.removedClasses.includes(className)) {
-            // this was previously removed, so we're just undoing that
-            this.removedClasses = this.removedClasses.filter((name) => name !== className);
-
-            return;
-        }
-
-        if (!this.addedClasses.includes(className)) {
-            this.addedClasses.push(className);
+        if (!this.removedClasses.delete(className)) {
+            this.addedClasses.add(className);
         }
     }
 
     removeClass(className: string) {
-        if (this.addedClasses.includes(className)) {
-            // this was previously added, so we're just undoing that
-            this.addedClasses = this.addedClasses.filter((name) => name !== className);
-
-            return;
-        }
-
-        if (!this.removedClasses.includes(className)) {
-            this.removedClasses.push(className);
+        if (!this.addedClasses.delete(className)) {
+            this.removedClasses.add(className);
         }
     }
 
@@ -53,11 +39,11 @@ export default class ElementChanges {
     }
 
     getAddedClasses(): string[] {
-        return this.addedClasses;
+        return [...this.addedClasses];
     }
 
     getRemovedClasses(): string[] {
-        return this.removedClasses;
+        return [...this.removedClasses];
     }
 
     getChangedStyles(): { name: string, value: string }[] {
@@ -77,13 +63,8 @@ export default class ElementChanges {
     }
 
     applyToElement(element: HTMLElement): void {
-        this.addedClasses.forEach((className) => {
-            element.classList.add(className);
-        });
-
-        this.removedClasses.forEach((className) => {
-            element.classList.remove(className);
-        });
+        element.classList.add(...this.addedClasses);
+        element.classList.remove(...this.removedClasses);
 
         this.styleChanges.getChangedItems().forEach((change) => {
             element.style.setProperty(change.name, change.value);
@@ -105,8 +86,8 @@ export default class ElementChanges {
 
     isEmpty(): boolean {
         return (
-            this.addedClasses.length === 0 &&
-            this.removedClasses.length === 0 &&
+            this.addedClasses.size === 0 &&
+            this.removedClasses.size === 0 &&
             this.styleChanges.isEmpty() &&
             this.attributeChanges.isEmpty()
         );
diff --git a/src/LiveComponent/assets/src/string_utils.ts b/src/LiveComponent/assets/src/string_utils.ts
index e555b586..d30dedf0 100644
--- a/src/LiveComponent/assets/src/string_utils.ts
+++ b/src/LiveComponent/assets/src/string_utils.ts
@@ -5,21 +5,26 @@
  *      [
  *          'foo',
  *          'bar baz',
+ *          ' foobar  foobaz ',
  *      ]
  *
  * Output:
- *      ['foo', 'bar', 'baz']
+ *      ['foo', 'bar', 'baz', 'foobar', 'foobaz']
  *
  */
 export function combineSpacedArray(parts: Array<string>) {
     const finalParts: Array<string> = [];
     parts.forEach((part) => {
-        finalParts.push(...part.split(' '));
+        finalParts.push(...trimAll(part).split(' '));
     });
 
     return finalParts;
 }
 
+export function trimAll(str: string) {
+    return str.replace(/[\s]+/g, ' ').trim();
+}
+
 /**
  * Normalizes model names with [] into the "." syntax.
  *


Test patch:
diff --git a/src/LiveComponent/assets/test/string_utils.test.ts b/src/LiveComponent/assets/test/string_utils.test.ts
index a081e166..4d03c644 100644
--- a/src/LiveComponent/assets/test/string_utils.test.ts
+++ b/src/LiveComponent/assets/test/string_utils.test.ts
@@ -10,6 +10,11 @@ describe('combinedSpacedArray', () => {
         const items = combineSpacedArray(['hidden other', 'bar']);
         expect(items).toEqual(['hidden', 'other', 'bar']);
     });
+
+    it('parse array with multispaced item', () => {
+        const items = combineSpacedArray([' hidden  other', 'bar']);
+        expect(items).toEqual(['hidden', 'other', 'bar']);
+    });
 });
 
 describe('normalizeModelName', () => {