Refactor nesting of children (more robust fix for #32)

changelog.md

@@ -296,3 +296,7 @@ Breaking changes:
 
 * Tests `Support.clearDatabase()` clears all models in Sequelize v3.x
 * Travis CI runs on all branches (to support greenkeeper.io)
+
+## Next
+
+* Refactor nesting of children (more robust fix for #32)

lib/hooksUniversal.js

@@ -73,13 +73,19 @@ module.exports = {
 			var include = options.includeMap[this.hierarchy.through.name];
 
 			if (include && include.where && include.where[this.hierarchy.throughForeignKey]) {
-				parent = {dataValues: {}};
+				parent = {};
 				parent[this.hierarchy.primaryKey] = include.where[this.hierarchy.throughForeignKey];
 			}
 		}
 
 		// convert hierarchies into trees
 		convertHierarchies(result, options, this, parent);
+
+		// where called from getDescendents, retrieve result from parent.children
+		if (parent) {
+			result.length = 0;
+			result.push.apply(result, parent[this.hierarchy.childrenAs]);
+		}
 	}
 };
 
@@ -138,59 +144,60 @@ function convertHierarchy(results, model, parent) {
 		descendentsAccessor = hierarchy.descendentsAs,
 		throughAccessor = hierarchy.through.name;
 
-	// get parent id and move results from parent.descendents to parent.children
-	var parentId = null;
+	// get parent id and create output array
+	var parentId, output;
 	if (parent) {
 		parentId = parent[primaryKey];
 
-		parent[childrenAccessor] = results;
+		// remove parent.descendents and create empty parent.children array
+		output = [];
+		parent[childrenAccessor] = output;
 		delete parent[descendentsAccessor];
 
 		if (parent instanceof Sequelize.Instance) {
-			parent.dataValues[childrenAccessor] = results;
+			parent.dataValues[childrenAccessor] = output;
 			delete parent.dataValues[descendentsAccessor];
 		}
+	} else {
+		parentId = null;
+
+		// duplicate results array and empty output array
+		output = results;
+		results = [].concat(results);
+		output.length = 0;
 	}
 
 	// run through all results, turning into tree
 
-	// find rows which are not top level and move into references
+	// create references object keyed by id
 	var references = {};
+	results.forEach(function(item) {
+		references[item[primaryKey]] = item;
+	});
 
-	for (var i = 0; i < results.length; i++) {
-		var item = results[i];
-
+	// run through results, transferring to output array or nesting within parent
+	results.forEach(function(item) {
 		// remove reference to through table
 		delete item[throughAccessor];
 		if (item instanceof Sequelize.Instance) delete item.dataValues[throughAccessor];
 
-		// add into references
-		// NB '_' is used because if use numbers as keys for an object, properties get sorted
-		// in order of the keys rather than maintaining order in which items are inserted.
-		// https://github.com/overlookmotel/sequelize-hierarchy/issues/32
-		references['_' + item[primaryKey]] = item;
-
-		// if not a root-level item, remove from results
-		if (item[foreignKey] !== parentId) {
-			results.splice(i, 1);
-			i--;
-		}
-	}
-
-	// run through references, nesting within parents
-	_.forIn(references, function(item) {
-		// nest inside parent
+		// if top-level item, add to output array
 		var thisParentId = item[foreignKey];
-		if (thisParentId !== parentId) {
-			var parent = references['_' + thisParentId];
-			if (!parent) throw new Sequelize.HierarchyError('Parent ID ' + thisParentId + ' not found in result set');
+		if (thisParentId === parentId) {
+			output.push(item);
+			return;
+		}
 
-			if (!parent[childrenAccessor]) {
-				parent[childrenAccessor] = [];
-				if (parent.dataValues) parent.dataValues[childrenAccessor] = parent[childrenAccessor];
-			}
+		// not top-level item - nest inside parent
+		var parent = references[thisParentId];
+		if (!parent) throw new Sequelize.HierarchyError('Parent ID ' + thisParentId + ' not found in result set');
 
-			parent[childrenAccessor].push(item);
+		var parentChildren = parent[childrenAccessor];
+		if (!parentChildren) {
+			parentChildren = parent[childrenAccessor] = [];
+			if (parent instanceof Sequelize.Instance) parent.dataValues[childrenAccessor] = parentChildren;
 		}
+
+		parentChildren.push(item);
 	});
 }