Repository: baianat/vee-validate
Pull request #1241 (2018-03-29 17:29:06)

[WIP] Performance improvements
This PR aims to improve many aspects of the performance issues of this plugin:

### HTML Radio and Checkboxes

- [x] Force Non-text inputs to use the `change` event regardless of configuration (unless none is specified).
- [x] Don't add extra validation listeners when the input is used in a v-for loop #1191  

### General Improvements

- [x] Apply error bag changes in batches #1191 

### Possible improvements

- [ ] Use a single validator instance for everything (memory improvement), scoping fields will be a challenge.


codecov[bot]: # [Codecov](https://codecov.io/gh/baianat/vee-validate/pull/1241?src=pr&el=h1) Report
> Merging [#1241](https://codecov.io/gh/baianat/vee-validate/pull/1241?src=pr&el=desc) into [master](https://codecov.io/gh/baianat/vee-validate/commit/0f8dc7540d156e2f504ecd7ba00321033b4cd21d?src=pr&el=desc) will **decrease** coverage by `0.2%`.
> The diff coverage is `89.65%`.

[![Impacted file tree graph](https://codecov.io/gh/baianat/vee-validate/pull/1241/graphs/tree.svg?width=650&src=pr&token=XFzlvHKFP5&height=150)](https://codecov.io/gh/baianat/vee-validate/pull/1241?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1241      +/-   ##
==========================================
- Coverage   99.79%   99.58%   -0.21%     
==========================================
  Files          50       50              
  Lines        1432     1443      +11     
  Branches      459      463       +4     
==========================================
+ Hits         1429     1437       +8     
- Misses          3        6       +3
```


| [Impacted Files](https://codecov.io/gh/baianat/vee-validate/pull/1241?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/core/field.js](https://codecov.io/gh/baianat/vee-validate/pull/1241/diff?src=pr&el=tree#diff-c3JjL2NvcmUvZmllbGQuanM=) | `100% <100%> (ø)` | :arrow_up: |
| [src/directive.js](https://codecov.io/gh/baianat/vee-validate/pull/1241/diff?src=pr&el=tree#diff-c3JjL2RpcmVjdGl2ZS5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/core/utils/index.js](https://codecov.io/gh/baianat/vee-validate/pull/1241/diff?src=pr&el=tree#diff-c3JjL2NvcmUvdXRpbHMvaW5kZXguanM=) | `98.4% <76.92%> (-1.6%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/baianat/vee-validate/pull/1241?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/baianat/vee-validate/pull/1241?src=pr&el=footer). Last update [0f8dc75...04364ff](https://codecov.io/gh/baianat/vee-validate/pull/1241?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/flow/validator.js b/flow/validator.js
index c4642098..031348ab 100644
--- a/flow/validator.js
+++ b/flow/validator.js
@@ -9,6 +9,7 @@ declare type ResultObject = {
 declare type Rule = (value: any, params: any[]) => boolean | ResultObject | Promise<boolean | ResultObject>;
 
 declare type ValidationResult = {
+  id: string,
   valid: boolean,
   error: ?MapObject,
   errors: ?Array<FieldError>
diff --git a/src/core/errorBag.js b/src/core/errorBag.js
index d3ac7be5..0f21f4bd 100644
--- a/src/core/errorBag.js
+++ b/src/core/errorBag.js
@@ -21,7 +21,7 @@ export default class ErrorBag {
   /**
    * Adds an error to the internal array.
    */
-  add (error: FieldError) {
+  add (error: FieldError | FieldError[]) {
     // handle old signature.
     if (arguments.length > 1) {
       error = {
@@ -33,8 +33,26 @@ export default class ErrorBag {
       };
     }
 
+    this.items.push(
+      ...this._normalizeError(error)
+    );
+  }
+
+  /**
+   * Normalizes passed errors to an error array.
+   */
+  _normalizeError (error: FieldError | FieldError[]): FieldError[] {
+    if (Array.isArray(error)) {
+      return error.map(e => {
+        e.scope = !isNullOrUndefined(e.scope) ? e.scope : null;
+
+        return e;
+      });
+    }
+
     error.scope = !isNullOrUndefined(error.scope) ? error.scope : null;
-    this.items.push(error);
+
+    return [error];
   }
 
   /**
@@ -209,9 +227,15 @@ export default class ErrorBag {
   }
 
   /**
-   * Removes errors by matching against the id.
+   * Removes errors by matching against the id or ids.
    */
-  removeById (id: string) {
+  removeById (id: string | string[]) {
+    if (Array.isArray(id)) {
+      // filter out the non-matching fields.
+      this.items = this.items.filter(i => id.indexOf(i.id) === -1);
+      return;
+    }
+
     for (let i = 0; i < this.items.length; ++i) {
       if (this.items[i].id === id) {
         this.items.splice(i, 1);
@@ -223,13 +247,9 @@ export default class ErrorBag {
   /**
    * Removes all error messages associated with a specific field.
    */
-  remove (field: string, scope: ?string, id?: string) {
+  remove (field: string, scope: ?string) {
     field = !isNullOrUndefined(field) ? String(field) : field;
     const removeCondition = (e: FieldError) => {
-      if (e.id && id) {
-        return e.id === id;
-      }
-
       if (!isNullOrUndefined(scope)) {
         return e.field === field && e.scope === scope;
       }
diff --git a/src/core/field.js b/src/core/field.js
index 2eed18b7..53e81ade 100644
--- a/src/core/field.js
+++ b/src/core/field.js
@@ -7,7 +7,7 @@ import {
   getDataAttribute,
   setDataAttribute,
   toggleClass,
-  getInputEventName,
+  isTextInput,
   debounce,
   isCallable,
   warn,
@@ -16,7 +16,8 @@ import {
   makeEventsArray,
   makeDelayObject,
   merge,
-  isObject
+  isObject,
+  addEventListener
 } from './utils';
 import Generator from './generator';
 import Validator from './validator';
@@ -418,6 +419,8 @@ export default class Field {
     // remove previous listeners.
     this.unwatch(/class/);
 
+    if (!this.el) return;
+
     const onBlur = () => {
       this.flags.touched = true;
       this.flags.untouched = false;
@@ -430,7 +433,7 @@ export default class Field {
       this.unwatch(/^class_blur$/);
     };
 
-    const inputEvent = getInputEventName(this.el);
+    const inputEvent = isTextInput(this.el) ? 'input' : 'change';
     const onInput = () => {
       this.flags.dirty = true;
       this.flags.pristine = false;
@@ -463,10 +466,10 @@ export default class Field {
 
     if (!this.el) return;
 
-    this.el.addEventListener(inputEvent, onInput);
+    addEventListener(this.el, inputEvent, onInput);
     // Checkboxes and radio buttons on Mac don't emit blur naturally, so we listen on click instead.
     const blurEvent = ['radio', 'checkbox'].indexOf(this.el.type) === -1 ? 'blur' : 'click';
-    this.el.addEventListener(blurEvent, onBlur);
+    addEventListener(this.el, blurEvent, onBlur);
     this.watchers.push({
       tag: 'class_input',
       unwatch: () => {
@@ -487,7 +490,7 @@ export default class Field {
    */
   addValueListeners () {
     this.unwatch(/^input_.+/);
-    if (!this.listen) return;
+    if (!this.listen || !this.el) return;
 
     const fn = this.targetOf ? () => {
       this.validator.validate(`#${this.targetOf}`);
@@ -500,11 +503,11 @@ export default class Field {
       this.validator.validate(`#${this.id}`, args[0]);
     };
 
-    const inputEvent = this.model && this.model.lazy ? 'change' : getInputEventName(this.el);
-    // replace input event with suitable one.
-    let events = this.events.map(e => {
-      return e === 'input' ? inputEvent : e;
-    });
+    let inputEvent = isTextInput(this.el) ? 'input' : 'change';
+    inputEvent = this.model && this.model.lazy ? 'change' : inputEvent;
+    // force change event for non-text type fields, otherwise use the configured.
+    // if no event is configured then respect the user choice.
+    let events = !this.events.length || isTextInput(this.el) ? this.events : ['change'];
 
     // if there is a watchable model and an on input validation is requested.
     if (this.model && this.model.expression && events.indexOf(inputEvent) !== -1) {
@@ -517,6 +520,7 @@ export default class Field {
         tag: 'input_model',
         unwatch
       });
+
       // filter out input event as it is already handled by the watcher API.
       events = events.filter(e => e !== inputEvent);
     }
@@ -549,10 +553,24 @@ export default class Field {
   _addHTMLEventListener (evt, validate) {
     if (!this.el || this.component) return;
 
+    // listen for the current element.
+    addEventListener(this.el, evt, validate);
+    this.watchers.push({
+      tag: 'input_native',
+      unwatch: () => {
+        this.el.removeEventListener(evt, validate);
+      }
+    });
+
     if (~['radio', 'checkbox'].indexOf(this.el.type)) {
       const els = document.querySelectorAll(`input[name="${this.el.name}"]`);
       toArray(els).forEach(el => {
-        el.addEventListener(evt, validate);
+        // skip if it is added by v-validate and is not the current element.
+        if (getDataAttribute(el, 'id') && el !== this.el) {
+          return;
+        }
+
+        addEventListener(el, evt, validate);
         this.watchers.push({
           tag: 'input_native',
           unwatch: () => {
@@ -560,17 +578,7 @@ export default class Field {
           }
         });
       });
-
-      return;
     }
-
-    this.el.addEventListener(evt, validate);
-    this.watchers.push({
-      tag: 'input_native',
-      unwatch: () => {
-        this.el.removeEventListener(evt, validate);
-      }
-    });
   }
 
   /**
diff --git a/src/core/utils/index.js b/src/core/utils/index.js
index 11de6c27..b0ad8db1 100644
--- a/src/core/utils/index.js
+++ b/src/core/utils/index.js
@@ -1,5 +1,31 @@
 // @flow
 
+let supportsPassive = true;
+
+export const detectPassiveSupport = () => {
+  try {
+    const opts = Object.defineProperty({}, 'passive', {
+      get () {
+        supportsPassive = true;
+      }
+    });
+    window.addEventListener('testPassive', null, opts);
+    window.removeEventListener('testPassive', null, opts);
+  } catch (e) {
+    supportsPassive = false;
+  };
+
+  return supportsPassive;
+};
+
+export const addEventListener = (el, eventName, cb) => {
+  el.addEventListener(eventName, cb, supportsPassive ? { passive: true } : false);
+};
+
+export const isTextInput = (el) => {
+  return ['text', 'number', 'password', 'search', 'email', 'tel', 'url', 'textarea'].indexOf(el.type) !== -1;
+};
+
 /**
  * Gets the data attribute. the name must be kebab-case.
  */
@@ -359,17 +385,6 @@ export const find = (arrayLike: { length: number } | any[], predicate: (any) =>
   return undefined;
 };
 
-/**
- * Returns a suitable event name for the input element.
- */
-export const getInputEventName = (el: HTMLInputElement) => {
-  if (el && (el.tagName === 'SELECT' || ~['radio', 'checkbox', 'file'].indexOf(el.type))) {
-    return 'change';
-  }
-
-  return 'input';
-};
-
 export const isBuiltInComponent = (vnode: Object): boolean => {
   if (!vnode) {
     return false;
diff --git a/src/core/validator.js b/src/core/validator.js
index a1c9f314..e95e5dae 100644
--- a/src/core/validator.js
+++ b/src/core/validator.js
@@ -321,21 +321,14 @@ export default class Validator {
       value = field.value;
     }
 
-    const result = await this._validate(field, value, silent);
-    if (silent) return result.valid;
-
-    this.errors.remove(field.name, field.scope, field.id);
     if (field.isDisabled) {
       return true;
-    } else if (result.errors) {
-      result.errors.forEach(e => this.errors.add(e));
     }
 
-    field.setFlags({
-      pending: false,
-      valid: result.valid,
-      validated: true
-    });
+    const result = await this._validate(field, value);
+    if (!silent) {
+      this._handleValidationResults([result]);
+    }
 
     return result.valid;
   }
@@ -382,16 +375,15 @@ export default class Validator {
       matcher = { scope };
     }
 
-    const promises = this.fields.filter(matcher).map(field => this.validate(
-      `#${field.id}`,
-      providedValues ? values[field.name] : field.value,
-      null,
-      silent
-    ));
+    const results = await Promise.all(
+      this.fields.filter(matcher).map(field => this._validate(field, providedValues ? values[field.name] : field.value))
+    );
 
-    const results = await Promise.all(promises);
+    if (!silent) {
+      this._handleValidationResults(results);
+    }
 
-    return results.every(t => t);
+    return results.every(t => t.valid);
   }
 
   /**
@@ -400,16 +392,15 @@ export default class Validator {
   async validateScopes (silent?: boolean = false): Promise<boolean> {
     if (this.paused) return true;
 
-    const promises = this.fields.map(field => this.validate(
-      `#${field.id}`,
-      field.value,
-      null,
-      silent
-    ));
+    const results = await Promise.all(
+      this.fields.map(field => this._validate(field, field.value))
+    );
 
-    const results = await Promise.all(promises);
+    if (!silent) {
+      this._handleValidationResults(results);
+    }
 
-    return results.every(t => t);
+    return results.every(t => t.valid);
   }
 
   /**
@@ -538,7 +529,7 @@ export default class Validator {
 
         return {
           valid: allValid,
-          error: allValid ? undefined : this._createFieldError(field, rule, data, targetName)
+          errors: allValid ? [] : [this._createFieldError(field, rule, data, targetName)]
         };
       });
     }
@@ -549,7 +540,7 @@ export default class Validator {
 
     return {
       valid: result.valid,
-      error: result.valid ? undefined : this._createFieldError(field, rule, result.data, targetName)
+      errors: result.valid ? [] : [this._createFieldError(field, rule, result.data, targetName)]
     };
   }
 
@@ -640,12 +631,34 @@ export default class Validator {
     );
   }
 
+  _handleValidationResults (results) {
+    const matchers = results.map(result => ({ id: result.id }));
+    this.errors.removeById(matchers.map(m => m.id));
+    const allErrors = results.reduce((prev, curr) => {
+      prev.push(...curr.errors);
+
+      return prev;
+    }, []);
+
+    this.errors.add(allErrors);
+
+    // handle flags.
+    this.fields.filter(matchers).forEach(field => {
+      const result = find(results, r => r.id === field.id);
+      field.setFlags({
+        pending: false,
+        valid: result.valid,
+        validated: true
+      });
+    });
+  }
+
   /**
    * Starts the validation process.
    */
-  async _validate (field: Field, value: any, silent?: boolean = false): Promise<ValidationResult> {
+  async _validate (field: Field, value: any): Promise<ValidationResult> {
     if (!field.isRequired && (isNullOrUndefined(value) || value === '')) {
-      return { valid: true };
+      return { valid: true, id: field.id, errors: [] };
     }
 
     const promises = [];
@@ -657,7 +670,7 @@ export default class Validator {
       if (isCallable(result.then)) {
         promises.push(result);
       } else if (this.fastExit && !result.valid) {
-        errors.push(result.error);
+        errors.push(...result.errors);
         isExitEarly = true;
       } else {
         // promisify the result.
@@ -668,17 +681,17 @@ export default class Validator {
     });
 
     if (isExitEarly) {
-      return { valid: false, errors };
+      return { valid: false, errors, id: field.id };
     }
 
     return (await Promise.all(promises)).reduce((prev, v) => {
       if (!v.valid) {
-        prev.errors.push(v.error);
+        prev.errors.push(...v.errors);
       }
 
       prev.valid = prev.valid && v.valid;
 
       return prev;
-    }, { valid: true, errors });
+    }, { valid: true, errors, id: field.id });
   }
 }
diff --git a/src/directive.js b/src/directive.js
index a3f4d651..53660154 100644
--- a/src/directive.js
+++ b/src/directive.js
@@ -1,6 +1,6 @@
 import Generator from './core/generator';
 import Field from './core/field';
-import { getDataAttribute, isEqual, warn } from './core/utils';
+import { getDataAttribute, isEqual, warn, toArray } from './core/utils';
 
 // @flow
 
@@ -18,7 +18,7 @@ const findField = (el: HTMLElement, context: ValidatingVM): ?Field => {
 export default {
   bind (el: HTMLElement, binding, vnode) {
     const validator = vnode.context.$validator;
-    if (! validator) {
+    if (!validator) {
       warn(`No validator instance is present on vm, did you forget to inject '$validator'?`);
       return;
     }
diff --git a/src/install.js b/src/install.js
index a204b1c5..99f69cee 100644
--- a/src/install.js
+++ b/src/install.js
@@ -2,7 +2,7 @@ import mixin from './mixin';
 import directive from './directive';
 import Config from './config';
 import Validator from './core/validator';
-import { warn, isCallable } from './core/utils';
+import { warn, isCallable, detectPassiveSupport } from './core/utils';
 
 let Vue;
 
@@ -14,6 +14,7 @@ function install (_Vue, options = {}) {
     return;
   }
 
+  detectPassiveSupport();
   Vue = _Vue;
   Config.register('vm', new Vue());
   Config.merge(options);


Test patch:
diff --git a/tests/unit/errorBag.js b/tests/unit/errorBag.js
index 58dca9ba..6c148714 100644
--- a/tests/unit/errorBag.js
+++ b/tests/unit/errorBag.js
@@ -22,6 +22,17 @@ test('adds errors to the collection', () => {
   expect(errors.first()).toBe(null);
 });
 
+test('accepts an array of errors', () => {
+  const errors = new ErrorBag();
+
+  errors.add([
+    { field: 'name', msg: 'The scoped name is invalid', rule: 'rule1' },
+    { field: 'name', msg: 'The scoped name is invalid', rule: 'rule1' },
+  ]);
+
+  expect(errors.count()).toBe(2);
+});
+
 test('updates error objects by matching against field id', () => {
   const errors = new ErrorBag();
   errors.add({
@@ -390,4 +401,5 @@ test('error bag instance is iterable', () => {
   }
 
   expect(idx).toBe(2);
-});
\ No newline at end of file
+});
+
diff --git a/tests/unit/utils.js b/tests/unit/utils.js
index f6068a31..9c5e09c7 100644
--- a/tests/unit/utils.js
+++ b/tests/unit/utils.js
@@ -279,20 +279,6 @@ describe('normalizes rules', () => {
   });
 });
 
-test('gets appropiate input event name for inputs', () => {
-  const text = { type: 'text' };
-  const select = { tagName: 'SELECT' };
-  const checkbox = { type: 'checkbox' };
-  const radio = { type: 'radio' };
-  const file = { type: 'file' };
-
-  expect(utils.getInputEventName(text)).toBe('input');
-  expect(utils.getInputEventName(select)).toBe('change');
-  expect(utils.getInputEventName(checkbox)).toBe('change');
-  expect(utils.getInputEventName(radio)).toBe('change');
-  expect(utils.getInputEventName(file)).toBe('change');
-});
-
 test('creates branded errors', () => {
   expect(() => {
     throw utils.createError('My Error')
@@ -461,3 +447,26 @@ describe('deepParseInt', () => {
     expect(utils.deepParseInt({ blur: "10", input: "400", focus: 300, change: "hello" })).toEqual({ blur: 10, input: 400, focus: 300, change: NaN });
   });
 });
+
+test('detects passive events support', () => {
+  expect(utils.detectPassiveSupport()).toBe(true);
+
+  const addEvt = window.addEventListener;
+  window.addEventListener = (evt, cb, opts) => {
+    if (opts.passive) {
+      return 'yay!';
+    }
+  }
+
+  expect(utils.detectPassiveSupport()).toBe(true);
+
+  window.addEventListener = (evt, cb, opts) => {
+    if (typeof opts === 'object') {
+      throw new Error('Should not pass an object');
+    }
+  }
+
+  expect(utils.detectPassiveSupport()).toBe(false);
+
+  window.addEventListener = addEvt;
+});
\ No newline at end of file