perf(documentarray): refactor to use ES6 classes instead of mixins, 3…

…0% speedup Re: #7895
Automattic · Jul 30, 2019 · 8784ef0 · 8784ef0
1 parent 9db4857
commit 8784ef0
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 141 deletions.
diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js
@@ -434,13 +434,12 @@ DocumentArray.prototype.clone = function() {
  */
 
 function _clearListeners(arr) {
-  if (arr == null || arr[arrayParentSymbol] == null) {
+  if (arr == null || arr[arrayParentSymbol] == null || !arr.isMongooseDocumentArray) {
     return;
   }
 
-  for (const key in arr._handlers) {
-    arr[arrayParentSymbol].removeListener(key, arr._handlers[key]);
-  }
+  arr[arrayParentSymbol].removeListener('isNew', arr.notifyIsNew);
+  arr[arrayParentSymbol].removeListener('save', arr.notifySave);
 }
 
 /*!

diff --git a/lib/types/documentarray.js b/lib/types/documentarray.js
@@ -21,106 +21,18 @@ const documentArrayParent = require('../helpers/symbols').documentArrayParent;
 
 const _basePush = Array.prototype.push;
 
-/**
- * DocumentArray constructor
- *
- * @param {Array} values
- * @param {String} path the path to this array
- * @param {Document} doc parent document
- * @api private
- * @return {MongooseDocumentArray}
- * @inherits MongooseArray
- * @see http://bit.ly/f6CnZU
- */
-
-function MongooseDocumentArray(values, path, doc) {
-  // TODO: replace this with `new CoreMongooseArray().concat()` when we remove
-  // support for node 4.x and 5.x, see https://i.imgur.com/UAAHk4S.png
-  const arr = new CoreMongooseArray();
-
-  const props = {
-    isMongooseDocumentArray: true,
-    validators: [],
-    _handlers: void 0
-  };
-
-  arr[arrayAtomicsSymbol] = {};
-  arr[arraySchemaSymbol] = void 0;
-  if (Array.isArray(values)) {
-    if (values instanceof CoreMongooseArray &&
-        values[arrayPathSymbol] === path &&
-        values[arrayParentSymbol] === doc) {
-      arr[arrayAtomicsSymbol] = Object.assign({}, values[arrayAtomicsSymbol]);
-    }
-    values.forEach(v => {
-      _basePush.call(arr, v);
-    });
-  }
-  arr[arrayPathSymbol] = path;
-
-  const keysMDA = Object.keys(MongooseDocumentArray.mixin);
-  let numKeys = keysMDA.length;
-  for (let i = 0; i < numKeys; ++i) {
-    arr[keysMDA[i]] = MongooseDocumentArray.mixin[keysMDA[i]];
-  }
-  if (util.inspect.custom) {
-    props[util.inspect.custom] = arr.inspect;
-  }
-
-  const keysP = Object.keys(props);
-  numKeys = keysP.length;
-  for (let k = 0; k < numKeys; ++k) {
-    arr[keysP[k]] = props[keysP[k]];
+class CoreDocumentArray extends CoreMongooseArray {
+  get isMongooseDocumentArray() {
+    return true;
   }
 
-  // Because doc comes from the context of another function, doc === global
-  // can happen if there was a null somewhere up the chain (see #3020 && #3034)
-  // RB Jun 17, 2015 updated to check for presence of expected paths instead
-  // to make more proof against unusual node environments
-  if (doc && doc instanceof Document) {
-    arr[arrayParentSymbol] = doc;
-    arr[arraySchemaSymbol] = doc.schema.path(path);
-
-    // `schema.path()` doesn't drill into nested arrays properly yet, see
-    // gh-6398, gh-6602. This is a workaround because nested arrays are
-    // always plain non-document arrays, so once you get to a document array
-    // nesting is done. Matryoshka code.
-    while (arr != null &&
-        arr[arraySchemaSymbol] != null &&
-        arr[arraySchemaSymbol].$isMongooseArray &&
-        !arr[arraySchemaSymbol].$isMongooseDocumentArray) {
-      arr[arraySchemaSymbol] = arr[arraySchemaSymbol].casterConstructor;
-    }
-
-    // Tricky but this may be a document array embedded in a normal array,
-    // in which case `path` would point to the embedded array. See #6405, #6398
-    if (arr[arraySchemaSymbol] && !arr[arraySchemaSymbol].$isMongooseDocumentArray) {
-      arr[arraySchemaSymbol] = arr[arraySchemaSymbol].casterConstructor;
-    }
-
-    arr._handlers = {
-      isNew: arr.notify('isNew'),
-      save: arr.notify('save')
-    };
-
-    doc.on('save', arr._handlers.save);
-    doc.on('isNew', arr._handlers.isNew);
-  }
-
-  return arr;
-}
-
-/*!
- * Inherits from MongooseArray
- */
-
-MongooseDocumentArray.mixin = {
   /*!
    * ignore
    */
-  toBSON: function() {
+
+  toBSON() {
     return this.toObject(internalToObjectOptions);
-  },
+  }
 
   /**
    * Overrides MongooseArray#cast
@@ -130,7 +42,7 @@ MongooseDocumentArray.mixin = {
    * @receiver MongooseDocumentArray
    */
 
-  _cast: function(value, index) {
+  _cast(value, index) {
     let Constructor = this[arraySchemaSymbol].casterConstructor;
     const isInstance = Constructor.$isMongooseDocumentArray ?
       value && value.isMongooseDocumentArray :
@@ -179,7 +91,7 @@ MongooseDocumentArray.mixin = {
       return Constructor.cast(value, this, undefined, undefined, index);
     }
     return new Constructor(value, this, undefined, undefined, index);
-  },
+  }
 
   /**
    * Searches array items for the first document with a matching _id.
@@ -196,7 +108,7 @@ MongooseDocumentArray.mixin = {
    * @receiver MongooseDocumentArray
    */
 
-  id: function(id) {
+  id(id) {
     let casted;
     let sid;
     let _id;
@@ -230,7 +142,7 @@ MongooseDocumentArray.mixin = {
     }
 
     return null;
-  },
+  }
 
   /**
    * Returns a native js Array of plain js objects
@@ -246,15 +158,15 @@ MongooseDocumentArray.mixin = {
    * @receiver MongooseDocumentArray
    */
 
-  toObject: function(options) {
+  toObject(options) {
     return this.map(function(doc) {
       try {
         return doc.toObject(options);
       } catch (e) {
         return doc || null;
       }
     });
-  },
+  }
 
   /**
    * Helper for console.log
@@ -264,9 +176,9 @@ MongooseDocumentArray.mixin = {
    * @receiver MongooseDocumentArray
    */
 
-  inspect: function() {
+  inspect() {
     return this.toObject();
-  },
+  }
 
   /**
    * Creates a subdocument casted to this schema.
@@ -279,7 +191,7 @@ MongooseDocumentArray.mixin = {
    * @receiver MongooseDocumentArray
    */
 
-  create: function(obj) {
+  create(obj) {
     let Constructor = this[arraySchemaSymbol].casterConstructor;
     if (obj &&
         Constructor.discriminators &&
@@ -298,47 +210,117 @@ MongooseDocumentArray.mixin = {
     }
 
     return new Constructor(obj, this);
-  },
+  }
 
-  /**
-   * Creates a fn that notifies all child docs of `event`.
-   *
-   * @param {String} event
-   * @return {Function}
-   * @method notify
-   * @api private
-   * @receiver MongooseDocumentArray
+  /*!
+   * ignore
    */
 
-  notify: function notify(event) {
-    const _this = this;
-    return function notify(val, _arr) {
-      _arr = _arr || _this;
-      let i = _arr.length;
-      while (i--) {
-        if (_arr[i] == null) {
-          continue;
-        }
-        switch (event) {
-          // only swap for save event for now, we may change this to all event types later
-          case 'save':
-            val = _this[i];
-            break;
-          default:
-            // NO-OP
-            break;
-        }
+  notifyIsNew(val, _arr) {
+    _arr = _arr || this;
+    let i = _arr.length;
+    while (i--) {
+      if (_arr[i] == null) {
+        continue;
+      }
 
-        if (_arr[i].isMongooseArray) {
-          notify(val, _arr[i]);
-        } else if (_arr[i]) {
-          _arr[i].emit(event, val);
-        }
+      if (_arr[i].isMongooseArray) {
+        notify(val, _arr[i]);
+      } else if (_arr[i]) {
+        _arr[i].emit('isNew', val);
+      }
+    }
+  }
+
+  /*!
+   * ignore
+   */
+
+  notifySave(val, _arr) {
+    _arr = _arr || this;
+    let i = _arr.length;
+    while (i--) {
+      if (_arr[i] == null) {
+        continue;
       }
-    };
+      val = this[i];
+
+      if (_arr[i].isMongooseArray) {
+        notify(val, _arr[i]);
+      } else if (_arr[i]) {
+        _arr[i].emit('save', val);
+      }
+    }
   }
+}
+
+if (util.inspect.custom) {
+  CoreDocumentArray.prototype[util.inspect.custom] =
+    CoreDocumentArray.prototype.inspect;
+}
+
+/**
+ * DocumentArray constructor
+ *
+ * @param {Array} values
+ * @param {String} path the path to this array
+ * @param {Document} doc parent document
+ * @api private
+ * @return {MongooseDocumentArray}
+ * @inherits MongooseArray
+ * @see http://bit.ly/f6CnZU
+ */
+
+function MongooseDocumentArray(values, path, doc) {
+  // TODO: replace this with `new CoreMongooseArray().concat()` when we remove
+  // support for node 4.x and 5.x, see https://i.imgur.com/UAAHk4S.png
+  const arr = new CoreDocumentArray();
 
-};
+  arr[arrayAtomicsSymbol] = {};
+  arr[arraySchemaSymbol] = void 0;
+  if (Array.isArray(values)) {
+    if (values instanceof CoreDocumentArray &&
+        values[arrayPathSymbol] === path &&
+        values[arrayParentSymbol] === doc) {
+      arr[arrayAtomicsSymbol] = Object.assign({}, values[arrayAtomicsSymbol]);
+    }
+    values.forEach(v => {
+      _basePush.call(arr, v);
+    });
+  }
+  arr[arrayPathSymbol] = path;
+
+  // Because doc comes from the context of another function, doc === global
+  // can happen if there was a null somewhere up the chain (see #3020 && #3034)
+  // RB Jun 17, 2015 updated to check for presence of expected paths instead
+  // to make more proof against unusual node environments
+  if (doc && doc instanceof Document) {
+    arr[arrayParentSymbol] = doc;
+    arr[arraySchemaSymbol] = doc.schema.path(path);
+
+    // `schema.path()` doesn't drill into nested arrays properly yet, see
+    // gh-6398, gh-6602. This is a workaround because nested arrays are
+    // always plain non-document arrays, so once you get to a document array
+    // nesting is done. Matryoshka code.
+    while (arr != null &&
+        arr[arraySchemaSymbol] != null &&
+        arr[arraySchemaSymbol].$isMongooseArray &&
+        !arr[arraySchemaSymbol].$isMongooseDocumentArray) {
+      arr[arraySchemaSymbol] = arr[arraySchemaSymbol].casterConstructor;
+    }
+
+    // Tricky but this may be a document array embedded in a normal array,
+    // in which case `path` would point to the embedded array. See #6405, #6398
+    if (arr[arraySchemaSymbol] && !arr[arraySchemaSymbol].$isMongooseDocumentArray) {
+      arr[arraySchemaSymbol] = arr[arraySchemaSymbol].casterConstructor;
+    }
+
+    doc.on('save', arr.notifySave);
+    doc.on('isNew', arr.notifyIsNew);
+  }
+
+  return arr;
+}
 
 /*!
  * Module exports.

diff --git a/test/schema.documentarray.test.js b/test/schema.documentarray.test.js
@@ -90,6 +90,7 @@ describe('schema.documentarray', function() {
     assert.equal(doc.nested[0][0].title, 'cool');
 
     doc.set({nested: [[{ title: 'new' }]]});
+    console.log(doc.nested)
     assert.equal(doc.nested[0].length, 1);
     assert.equal(doc.nested[0][0].title, 'new');