fix(form): form element infinite loop on setAttributes & mutation obs…

…ervable Signed-off-by: Alexandre Esteves <[email protected]>
ovh · Nov 28, 2024 · 73158a5 · 73158a5
1 parent 2bb3de9
commit 73158a5
Show file tree

Hide file tree

Showing 15 changed files with 270 additions and 35 deletions.
diff --git a/packages/ods/react/tests/_app/src/components/ods-textarea.tsx b/packages/ods/react/tests/_app/src/components/ods-textarea.tsx
@@ -1,19 +1,39 @@
-import React from 'react-dom/client';
+import { useFormik } from 'formik';
 import { OdsTextarea } from 'ods-components-react';
+import { ChangeEvent, type ReactElement } from 'react';
+import React from 'react-dom/client';
 
-const Textarea = () => {
-  function onOdsChange() {
+// eslint-disable-next-line func-style
+const Textarea = (): ReactElement => {
+  const formik = useFormik({
+    initialValues: {
+      textarea: '',
+    },
+    onSubmit: (values) => {
+      console.log('Formik values', values);
+    },
+    validateOnMount: true,
+  });
+
+  function onOdsChange(e: ChangeEvent): void {
     console.log('React textarea odsChange');
   }
 
   return (
     <>
       <OdsTextarea name="ods-textarea"
-                   onOdsChange={ onOdsChange } />
+        onOdsChange={ onOdsChange } />
 
       <OdsTextarea isDisabled
-                   name="ods-textarea-disabled"
-                   onOdsChange={ onOdsChange } />
+        name="ods-textarea-disabled"
+        onOdsChange={ onOdsChange } />
+
+      <OdsTextarea name="ods-textarea-formik"
+        onOdsChange={ (e) => {
+          onOdsChange(e);
+          return formik.handleChange(e);
+        } }
+        value={ formik.values.textarea } />
     </>
   );
 };

diff --git a/packages/ods/react/tests/e2e/ods-textarea.e2e.ts b/packages/ods/react/tests/e2e/ods-textarea.e2e.ts
@@ -48,4 +48,19 @@ describe('ods-textarea react', () => {
 
     expect(consoleLog).toBe('');
   });
+
+  it('should not do an infinite loop', async () => {
+    const elem = await page.$('ods-textarea[name="ods-textarea-formik"] >>> textarea');
+    let consoleLog = '';
+    page.on('console', (consoleObj) => {
+      console.log('consoleObj', consoleObj.text())
+      consoleLog += consoleObj.text();
+    });
+
+    await elem?.type('a');
+    // Small delay to ensure page console event has been resolved
+    await new Promise((resolve) => setTimeout(resolve, 100));
+
+    expect(consoleLog).toBe('React textarea odsChangeReact textarea odsChange');
+  });
 });
diff --git a/packages/ods/src/components/datepicker/tests/behaviour/ods-datepicker.e2e.ts b/packages/ods/src/components/datepicker/tests/behaviour/ods-datepicker.e2e.ts
@@ -217,6 +217,27 @@ describe('ods-datepicker behaviour', () => {
           value: '2024-05-11T00:00:00.000Z',
         });
       });
+
+      it('should not do an infinite loop', async() => {
+        const dummyValue = '11/05/2024';
+        await setup('<ods-datepicker></ods-datepicker>');
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsDatepicker = document.querySelector<OdsDatepicker & HTMLElement>('ods-datepicker');
+          odsDatepicker?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsDatepicker!.value = (event.detail?.value && new Date(event.detail.value.toISOString())) ?? null;
+          }) as unknown as EventListener);
+        });
+
+        await page.evaluate((value) => {
+          document.querySelector<OdsDatepicker & HTMLElement>('ods-datepicker')!.value = new Date(value);
+        }, dummyValue);
+        await page.waitForChanges();
+
+        // expect(await el.getProperty('value')).toBe(new Date(dummyValue).toISOString());
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
     });
   });
 });
diff --git a/packages/ods/src/components/input/src/components/ods-input/ods-input.tsx b/packages/ods/src/components/input/src/components/ods-input/ods-input.tsx
@@ -149,7 +149,7 @@ export class OdsInput {
 
     this.observer = new MutationObserver((mutations: MutationRecord[]) => {
       for (const mutation of mutations) {
-        if (mutation.attributeName === 'value') {
+        if (mutation.attributeName === 'value' && this.value !== mutation.oldValue) {
           this.onValueChange(mutation.oldValue);
         }
 

diff --git a/packages/ods/src/components/input/tests/behaviour/ods-input.e2e.ts b/packages/ods/src/components/input/tests/behaviour/ods-input.e2e.ts
@@ -236,6 +236,44 @@ describe('ods-input behaviour', () => {
         expect(await el.getProperty('value')).toBe(null);
         expect(odsChangeSpy).not.toHaveReceivedEvent();
       });
+
+      it('should not do an infinite loop with string', async() => {
+        const dummyValue = 'dummy value';
+        await setup('<ods-input></ods-input>');
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsInput = document.querySelector('ods-input');
+          odsInput?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsInput.setAttribute('value', event.detail.value?.toString() ?? '');
+          }) as unknown as EventListener);
+        });
+
+        await el.setAttribute('value', dummyValue);
+        await page.waitForChanges();
+
+        expect(await el.getProperty('value')).toBe(dummyValue);
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
+
+      it('should not do an infinite loop with string', async() => {
+        const dummyValue = 2;
+        await setup('<ods-input></ods-input>');
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsInput = document.querySelector('ods-input');
+          odsInput?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsInput.setAttribute('value', event.detail.value?.toString() ?? '');
+          }) as unknown as EventListener);
+        });
+
+        await el.setAttribute('value', dummyValue);
+        await page.waitForChanges();
+
+        expect(await el.getProperty('value')).toBe(dummyValue.toString());
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
     });
   });
 

diff --git a/packages/ods/src/components/password/tests/behaviour/ods-password.e2e.ts b/packages/ods/src/components/password/tests/behaviour/ods-password.e2e.ts
@@ -187,6 +187,25 @@ describe('ods-password behaviour', () => {
         expect(await el.getProperty('value')).toBe(null);
         expect(odsChangeSpy).not.toHaveReceivedEvent();
       });
+
+      it('should not do an infinite loop', async() => {
+        const dummyValue = 'dummy value';
+        await setup('<ods-password></ods-password>');
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsPassword = document.querySelector('ods-password');
+          odsPassword?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsPassword.setAttribute('value', event.detail.value?.toString() ?? '');
+          }) as unknown as EventListener);
+        });
+
+        await el.setAttribute('value', dummyValue);
+        await page.waitForChanges();
+
+        expect(await el.getProperty('value')).toBe(dummyValue);
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
     });
   });
 

diff --git a/packages/ods/src/components/phone-number/tests/behaviour/ods-phone-number.e2e.ts b/packages/ods/src/components/phone-number/tests/behaviour/ods-phone-number.e2e.ts
@@ -274,6 +274,25 @@ describe('ods-phone-number behaviour', () => {
 
         expect(odsChangeSpy).toHaveReceivedEventTimes(0);
       });
+
+      it('should not do an infinite loop', async() => {
+        const dummyValue = '0123456789';
+        await setup('<ods-phone-number iso-code="fr"></ods-phone-number>');
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsPhoneNumber = document.querySelector('ods-phone-number');
+          odsPhoneNumber?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsPhoneNumber.setAttribute('value', event.detail.value?.toString() ?? '');
+          }) as unknown as EventListener);
+        });
+
+        await el.setAttribute('value', dummyValue);
+        await page.waitForChanges();
+
+        expect(await el.getProperty('value')).toBe('+33' + dummyValue.substring(1));
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
     });
   });
 });
diff --git a/packages/ods/src/components/quantity/tests/behaviour/ods-quantity.e2e.ts b/packages/ods/src/components/quantity/tests/behaviour/ods-quantity.e2e.ts
@@ -367,6 +367,25 @@ describe('ods-quantity behaviour', () => {
 
         expect(odsChangeSpy).toHaveReceivedEventTimes(0);
       });
+
+      it('should not do an infinite loop', async() => {
+        const dummyValue = 2;
+        await setup('<ods-quantity></ods-quantity>');
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsQuantity = document.querySelector('ods-quantity');
+          odsQuantity?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsQuantity.setAttribute('value', event.detail.value?.toString() ?? '');
+          }) as unknown as EventListener);
+        });
+
+        await el.setAttribute('value', dummyValue);
+        await page.waitForChanges();
+
+        expect(await el.getProperty('value')).toBe(dummyValue);
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
     });
   });
 });
diff --git a/packages/ods/src/components/select/src/components/ods-select/ods-select.tsx b/packages/ods/src/components/select/src/components/ods-select/ods-select.tsx
@@ -172,7 +172,7 @@ export class OdsSelect {
   componentWillLoad(): void {
     this.observer = new MutationObserver((mutations) => {
       for (const mutation of mutations) {
-        if (mutation.attributeName === 'value') {
+        if (mutation.attributeName === 'value' && this.value !== mutation.oldValue) {
           this.onValueChange(this.value, mutation.oldValue);
         }
       }

diff --git a/packages/ods/src/components/select/tests/behaviour/ods-select.e2e.ts b/packages/ods/src/components/select/tests/behaviour/ods-select.e2e.ts
@@ -414,6 +414,25 @@ describe('ods-select behaviour', () => {
           value: dummyValue,
         });
       });
+
+      it('should not do an infinite loop', async() => {
+        const dummyValue = 'dummy value';
+        await setup(`<ods-select><option value="${dummyValue}">Value 1</option></ods-select>`);
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsSelect = document.querySelector('ods-select');
+          odsSelect?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsSelect.setAttribute('value', event.detail.value);
+          }) as unknown as EventListener);
+        });
+
+        await el.setAttribute('value', dummyValue);
+        await page.waitForChanges();
+
+        expect(await el.getProperty('value')).toBe(dummyValue);
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
     });
 
     describe('readonly', () => {

diff --git a/packages/ods/src/components/textarea/src/components/ods-textarea/ods-textarea.tsx b/packages/ods/src/components/textarea/src/components/ods-textarea/ods-textarea.tsx
@@ -102,7 +102,7 @@ export class OdsTextarea {
   componentWillLoad(): void {
     this.observer = new MutationObserver((mutations: MutationRecord[]) => {
       for (const mutation of mutations) {
-        if (mutation.attributeName === 'value') {
+        if (mutation.attributeName === 'value' && this.value !== mutation.oldValue) {
           this.onValueChange(mutation.oldValue);
         }
 

diff --git a/packages/ods/src/components/textarea/tests/behaviour/ods-textarea.e2e.ts b/packages/ods/src/components/textarea/tests/behaviour/ods-textarea.e2e.ts
@@ -203,6 +203,25 @@ describe('ods-textarea behaviour', () => {
         expect(await el.getProperty('value')).toBe(null);
         expect(odsValueChangeSpy).not.toHaveReceivedEvent();
       });
+
+      it('should not do an infinite loop', async() => {
+        const dummyValue = 'dummy value';
+        await setup('<ods-textarea></ods-textarea>');
+        const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+        await page.evaluate(() => {
+          const odsTextarea = document.querySelector('ods-textarea');
+          odsTextarea?.addEventListener('odsChange', ((event: CustomEvent): void => {
+            odsTextarea.setAttribute('value', event.detail.value ?? '');
+          }) as unknown as EventListener);
+        });
+
+        await el.setAttribute('value', dummyValue);
+        await page.waitForChanges();
+
+        expect(await el.getProperty('value')).toBe(dummyValue);
+        expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+      });
     });
   });
 });
diff --git a/packages/ods/src/components/timepicker/tests/behavior/ods-timepicker.e2e.ts b/packages/ods/src/components/timepicker/tests/behavior/ods-timepicker.e2e.ts
@@ -214,6 +214,25 @@ describe('ods-timepicker behavior', () => {
         value: newValue,
       });
     });
+
+    it('should not do an infinite loop', async() => {
+      const dummyValue = '11:11';
+      await setup('<ods-timepicker></ods-timepicker>');
+      const odsValueChangeSpy = await page.spyOnEvent('odsChange');
+
+      await page.evaluate(() => {
+        const odsTimepicker = document.querySelector('ods-timepicker');
+        odsTimepicker?.addEventListener('odsChange', ((event: CustomEvent): void => {
+          odsTimepicker.setAttribute('value', event.detail.value ?? '');
+        }) as unknown as EventListener);
+      });
+
+      await el.setAttribute('value', dummyValue);
+      await page.waitForChanges();
+
+      expect(await el.getProperty('value')).toBe(dummyValue);
+      expect(odsValueChangeSpy).toHaveReceivedEventTimes(1);
+    });
   });
 
   describe('form', () => {

diff --git a/packages/ods/src/components/toggle/src/components/ods-toggle/ods-toggle.tsx b/packages/ods/src/components/toggle/src/components/ods-toggle/ods-toggle.tsx
@@ -96,7 +96,8 @@ export class OdsToggle {
   componentWillLoad(): void {
     this.observer = new MutationObserver((mutations: MutationRecord[]) => {
       for (const mutation of mutations) {
-        if (mutation.attributeName === 'value') {
+        const hasChange = this.value && mutation.oldValue === null || !this.value && mutation.oldValue !== null;
+        if (mutation.attributeName === 'value' && hasChange) {
           this.onValueChange();
         }