chore: refactor input-like components to avoid calling setValue and validation if the value hasn't changed

Author: AlekseyManetov

src/components/entries/FEEL/Feel.js

@@ -656,18 +656,18 @@ export default function FeelEntry(props) {
 
   const onInput = useStaticCallback((newValue) => {
     const value = getValue(element);
-    let newValidationError = null;
 
-    if (isFunction(validate)) {
-      newValidationError = validate(newValue) || null;
-    }
-
-    // don't create multiple commandStack entries for the same value
+    // don't create multiple commandStack entries and do validation for the same value
     if (newValue !== value) {
+      let newValidationError = null;
+
+      if (isFunction(validate)) {
+        newValidationError = validate(newValue) || null;
+      }
+
       setValue(newValue, newValidationError);
+      setValidationError(newValidationError);
     }
-
-    setValidationError(newValidationError);
   });
 
   const onError = useCallback(err => {

src/components/entries/NumberField.js

@@ -139,14 +139,16 @@ export default function NumberFieldEntry(props) {
   }, [ value, validate ]);
 
   const onInput = (newValue) => {
-    let newValidationError = null;
+    if (newValue !== value) {
+      let newValidationError = null;
 
-    if (isFunction(validate)) {
-      newValidationError = validate(newValue) || null;
-    }
+      if (isFunction(validate)) {
+        newValidationError = validate(newValue) || null;
+      }
 
-    setValue(newValue, newValidationError);
-    setLocalError(newValidationError);
+      setValue(newValue, newValidationError);
+      setLocalError(newValidationError);
+    }
   };
 
   const error = globalError || localError;

src/components/entries/Select.js

@@ -162,14 +162,16 @@ export default function SelectEntry(props) {
 
 
   const onChange = (newValue) => {
-    let newValidationError = null;
+    if (newValue !== value) {
+      let newValidationError = null;
 
-    if (isFunction(validate)) {
-      newValidationError = validate(newValue) || null;
-    }
+      if (isFunction(validate)) {
+        newValidationError = validate(newValue) || null;
+      }
 
-    setValue(newValue, newValidationError);
-    setLocalError(newValidationError);
+      setValue(newValue, newValidationError);
+      setLocalError(newValidationError);
+    }
   };
 
   const error = globalError || localError;

src/components/entries/TextArea.js

@@ -223,17 +223,17 @@ export default function TextAreaEntry(props) {
 
   const onInput = useStaticCallback((newValue) => {
     const value = getValue(element);
-    let newValidationError = null;
-
-    if (isFunction(validate)) {
-      newValidationError = validate(newValue) || null;
-    }
 
     if (newValue !== value) {
+      let newValidationError = null;
+
+      if (isFunction(validate)) {
+        newValidationError = validate(newValue) || null;
+      }
+
       setValue(newValue, newValidationError);
+      setLocalError(newValidationError);
     }
-
-    setLocalError(newValidationError);
   });
 
 

src/components/entries/TextField.js

@@ -189,17 +189,17 @@ export default function TextfieldEntry(props) {
 
   const onInput = useStaticCallback((newValue) => {
     const value = getValue(element);
-    let newValidationError = null;
-
-    if (isFunction(validate)) {
-      newValidationError = validate(newValue) || null;
-    }
 
     if (newValue !== value) {
+      let newValidationError = null;
+
+      if (isFunction(validate)) {
+        newValidationError = validate(newValue) || null;
+      }
+
       setValue(newValue, newValidationError);
+      setLocalError(newValidationError);
     }
-
-    setLocalError(newValidationError);
   });
 
 

test/spec/components/JsonEditor.spec.js

@@ -306,6 +306,34 @@ describe('<JsonEditor>', function() {
     });
 
 
+    it('should not call setValue or validate if value is same', async function() {
+
+      // given
+      const setValueSpy = sinonSpy();
+      const validateSpy = sinonSpy();
+
+      const result = createJsonEditor({
+        container,
+        getValue: () => '{"existing": true}',
+        setValue: setValueSpy,
+        validate: validateSpy
+      });
+
+      await waitFor(() => {
+        expect(getEditorView(result.container)).to.exist;
+      });
+
+      // when
+      validateSpy.resetHistory();
+      setEditorValue(result.container, '{"existing": true}');
+
+      // then
+      await new Promise(resolve => setTimeout(resolve, 50));
+      expect(setValueSpy).to.not.have.been.called;
+      expect(validateSpy).to.not.have.been.called;
+    });
+
+
     it('should pass validation error to setValue', async function() {
 
       // given

test/spec/components/NumberField.spec.js

@@ -91,7 +91,7 @@ describe('<NumberField>', function() {
     // given
     const updateSpy = sinonSpy();
 
-    const result = createNumberField({ container, setValue: updateSpy });
+    const result = createNumberField({ container, getValue: () => 42, setValue: updateSpy });
 
     const input = domQuery('.bio-properties-panel-input', result.container);
 
@@ -122,6 +122,31 @@ describe('<NumberField>', function() {
   });
 
 
+  it('should not call setValue or validate if the value is same', function() {
+
+    // given
+    const setValueSpy = sinonSpy();
+    const validateSpy = sinonSpy();
+
+    const result = createNumberField({
+      container,
+      getValue: () => 20.5,
+      setValue: setValueSpy,
+      validate: validateSpy
+    });
+
+    const input = domQuery('.bio-properties-panel-input', result.container);
+
+    // when
+    validateSpy.resetHistory();
+    changeInput(input, 20.5);
+
+    // then
+    expect(setValueSpy).to.not.have.been.called;
+    expect(validateSpy).to.not.have.been.called;
+  });
+
+
   it('should use unique input element on element change', function() {
 
     // given

test/spec/components/Select.spec.js

@@ -147,6 +147,34 @@ describe('<Select>', function() {
   });
 
 
+  it('should not call setValue or validate if the value is same', function() {
+
+    // given
+    const setValueSpy = sinonSpy();
+    const validateSpy = sinonSpy();
+
+    const getOptions = () => createOptions();
+
+    const result = createSelect({
+      container,
+      getValue: () => 'A',
+      setValue: setValueSpy,
+      validate: validateSpy,
+      getOptions
+    });
+
+    const select = domQuery('.bio-properties-panel-input', result.container);
+
+    // when
+    validateSpy.resetHistory();
+    changeInput(select, 'A');
+
+    // then
+    expect(setValueSpy).to.not.have.been.called;
+    expect(validateSpy).to.not.have.been.called;
+  });
+
+
   it('should use unique input element on element change', function() {
 
     // given

test/spec/components/TextArea.spec.js

@@ -336,26 +336,30 @@ describe('<TextArea>', function() {
   });
 
 
-  it('should not call setValue if the value is same', async function() {
+  it('should not call setValue or validate if the value is same', async function() {
 
     // given
     const setValueSpy = sinonSpy();
+    const validateSpy = sinonSpy();
 
     const result = createTextArea({
       container,
       getValue: () => 'a value',
-      setValue: setValueSpy
+      setValue: setValueSpy,
+      validate: validateSpy
     });
 
     const input = domQuery('.bio-properties-panel-input', result.container);
 
     // when
+    validateSpy.resetHistory();
     input.focus();
     changeInput(input, 'a value');
     input.blur();
 
     // then
     expect(setValueSpy).to.not.have.been.called;
+    expect(validateSpy).to.not.have.been.called;
 
   });
 

test/spec/components/TextField.spec.js

@@ -294,26 +294,30 @@ describe('<TextField>', function() {
   });
 
 
-  it('should not call setValue if the value is same', async function() {
+  it('should not call setValue or validate if the value is same', async function() {
 
     // given
     const setValueSpy = sinonSpy();
+    const validateSpy = sinonSpy();
 
     const result = createTextField({
       container,
       getValue: () => 'a value',
-      setValue: setValueSpy
+      setValue: setValueSpy,
+      validate: validateSpy
     });
 
     const input = domQuery('.bio-properties-panel-input', result.container);
 
     // when
+    validateSpy.resetHistory();
     input.focus();
     changeInput(input, 'a value');
     input.blur();
 
     // then
     expect(setValueSpy).to.not.have.been.called;
+    expect(validateSpy).to.not.have.been.called;
 
   });