chore: enhance execution on empty api

ing-bank · Aug 2, 2023 · 99ca940 · 99ca940
1 parent 92b429e
commit 99ca940
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 41 deletions.
diff --git a/.changeset/sharp-plants-shave.md b/.changeset/sharp-plants-shave.md
@@ -2,4 +2,4 @@
 '@lion/ui': patch
 ---
 
-feat: allow Required validator on Fieldset and Form; `static executesOnEmpty` flag on Validators
+feat: allow Required validator on Fieldset and Form;
diff --git a/packages/ui/components/form-core/src/validate/ValidateMixin.js b/packages/ui/components/form-core/src/validate/ValidateMixin.js
@@ -432,22 +432,30 @@ export const ValidateMixinImplementation = superclass =>
        */
 
       const isEmpty = this.__isEmpty(value);
-
       this.__syncValidationResult = [];
+
+      /**
+       * So we can have the following scenarios:
+       * - we're empty
+       *   - we have a single value (we are an 'endpoint') => we want to halt execution, because validation only makes sense when we have a value
+       *     - 1. we have Required => we fill .__syncValidationResult and finish validation, because we have a value to validate
+       *     - 2. we don't have Required => we stop validation
+       *   - we have a group of values (object/array) => we want to continue execution, because the constellation of the object (even though the individual endpoints are empty) might be interesting for validation.
+       *     - 3. we have Required => we fill .__syncValidationResult and continue validation (so we can add more to .__syncValidationResult)
+       *     - 4. we don't have Required => we continue validation
+       *  - we're not empty
+       *    - we have a single value or group of values
+       *     - 5. we may have Required, but if we have it will not be 'active' => we continue execution, because we have a value to validate
+       */
+
       if (isEmpty) {
+        const hasSingleValue = !(/** @type {*  & ValidateHost} */ (this)._isFormOrFieldset);
         const requiredValidator = this._allValidators.find(v => v instanceof Required);
         if (requiredValidator) {
           this.__syncValidationResult = [{ validator: requiredValidator, outcome: true }];
         }
-        // TODO: get rid of _isFormOrFieldset in a breaking future update.
-        // For now, We should keep forms and fieldsets backwards compatible...
-        const validatorsThatShouldRunOnEmpty = this._allValidators.filter(
-          v => v.constructor.executesOnEmpty,
-        );
-        const shouldHaltValidationOnEmpty =
-          !validatorsThatShouldRunOnEmpty.length && !this._isFormOrFieldset;
 
-        if (shouldHaltValidationOnEmpty) {
+        if (hasSingleValue) {
           this.__finishValidationPass({
             syncValidationResult: this.__syncValidationResult,
           });
@@ -463,7 +471,6 @@ export const ValidateMixinImplementation = superclass =>
         if (v instanceof MetaValidator) {
           metaValidators.push(v);
         } else if (v instanceof Required) {
-          // syncValidators.unshift(v);
           // Required validator was already handled
         } else if (/** @type {typeof Validator} */ (v.constructor).async) {
           asyncValidators.push(v);

diff --git a/packages/ui/components/form-core/test-suites/ValidateMixin.suite.js b/packages/ui/components/form-core/test-suites/ValidateMixin.suite.js
@@ -855,34 +855,6 @@ export function runValidateMixinSuite(customConfig) {
         expect(alwaysInvalidSpy.callCount).to.equal(1); // __isRequired returned true (valid)
       });
 
-      it('does not prevent other Validators from being called when input is empty, but at least one Validator has "executesOnEmpty"', async () => {
-        class AlwaysInvalidExecutingOnEmpty extends Validator {
-          static validatorName = 'AlwaysInvalidExecutingOnEmpty';
-
-          static executesOnEmpty = true;
-
-          execute() {
-            return true;
-          }
-        }
-        const alwaysInvalidExecutingOnEmpty = new AlwaysInvalidExecutingOnEmpty();
-        const aalwaysInvalidExecutingOnEmptySpy = sinon.spy(
-          alwaysInvalidExecutingOnEmpty,
-          'execute',
-        );
-        const el = /** @type {ValidateElement} */ (
-          await fixture(html`
-          <${tag}
-            .validators=${[new Required(), alwaysInvalidExecutingOnEmpty]}
-            .modelValue=${''}
-          >${lightDom}</${tag}>
-        `)
-        );
-        expect(aalwaysInvalidExecutingOnEmptySpy.callCount).to.equal(1);
-        el.modelValue = 'foo';
-        expect(aalwaysInvalidExecutingOnEmptySpy.callCount).to.equal(2);
-      });
-
       it('adds [aria-required="true"] to "._inputNode"', async () => {
         const el = /** @type {ValidateElement} */ (
           await fixture(html`

diff --git a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js
@@ -589,9 +589,9 @@ export function runFormGroupMixinSuite(cfg = {}) {
         // initially the group is invalid
         expect(el.validationStates.error.Required).to.be.true;
         el.formElements.fieldA.modelValue = 'foo';
-        // If at least one child is filled, the group is valid
+        // if at least one child is filled, the group is valid
         expect(el.validationStates.error.Required).to.be.undefined;
-        // // initially the group is invalid
+        // make Required trigger error state again
         el.formElements.fieldA.modelValue = '';
         expect(el.validationStates.error.Required).to.be.true;
       });