From 2b36a62c5600738737e0fda4e0fc92e6e1f59c33 Mon Sep 17 00:00:00 2001
From: Henry Jameson <me@hjkos.com>
Date: Mon, 20 Jan 2020 01:44:11 +0200
Subject: [PATCH] fix tests, integrate depenentless sorting into toposort for
 easier testing and better guarantees

---
 src/services/theme_data/theme_data.service.js | 22 +++++++++----------
 .../services/theme_data/theme_data.spec.js    | 10 ++++++++-
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/services/theme_data/theme_data.service.js b/src/services/theme_data/theme_data.service.js
index 36837b44..63bfb5af 100644
--- a/src/services/theme_data/theme_data.service.js
+++ b/src/services/theme_data/theme_data.service.js
@@ -540,7 +540,7 @@ const getDependencies = (key, inheritance) => {
  * @property {Function} getDeps - function that returns dependencies for
  *   given value and inheritance object.
  * @returns {String[]} keys of inheritance object, sorted in topological
- *   order
+ *   order. Additionally, dependency-less nodes will always be first in line
  */
 export const topoSort = (
   inheritance = SLOT_INHERITANCE,
@@ -579,7 +579,14 @@ export const topoSort = (
   while (unprocessed.length > 0) {
     step(unprocessed.pop())
   }
-  return output
+  return output.sort((a, b) => {
+    const depsA = getDeps(a, inheritance).length
+    const depsB = getDeps(b, inheritance).length
+
+    if (depsA === depsB || (depsB !== 0 && depsA !== 0)) return 0
+    if (depsA === 0 && depsB !== 0) return -1
+    if (depsB === 0 && depsA !== 0) return 1
+  })
 }
 
 /**
@@ -657,20 +664,13 @@ export const getLayerSlot = (
 }
 
 /**
- * topologically sorted SLOT_INHERITANCE + additional priority sort
+ * topologically sorted SLOT_INHERITANCE
  */
 export const SLOT_ORDERED = topoSort(
   Object.entries(SLOT_INHERITANCE)
     .sort(([aK, aV], [bK, bV]) => ((aV && aV.priority) || 0) - ((bV && bV.priority) || 0))
     .reduce((acc, [k, v]) => ({ ...acc, [k]: v }), {})
-).sort((a, b) => {
-  const depsA = getDependencies(a, SLOT_INHERITANCE).length
-  const depsB = getDependencies(b, SLOT_INHERITANCE).length
-
-  if (depsA === depsB || (depsB !== 0 && depsA !== 0)) return 0
-  if (depsA === 0 && depsB !== 0) return -1
-  if (depsB === 0 && depsA !== 0) return 1
-})
+)
 
 /**
  * Dictionary where keys are color slots and values are opacity associated
diff --git a/test/unit/specs/services/theme_data/theme_data.spec.js b/test/unit/specs/services/theme_data/theme_data.spec.js
index 507905eb..d8a04ba7 100644
--- a/test/unit/specs/services/theme_data/theme_data.spec.js
+++ b/test/unit/specs/services/theme_data/theme_data.spec.js
@@ -72,8 +72,16 @@ describe('topoSort', () => {
     expect(out.indexOf('layer2A')).to.be.below(out.indexOf('layer3AB'))
     expect(out.indexOf('layer2B')).to.be.below(out.indexOf('layer3AB'))
   })
+
+  it('dependentless nodes should be first', () => {
+    const out = topoSort(fixture2, (node, inheritance) => inheritance[node])
+    // This basically checks all ordering that matters
+    expect(out.indexOf('layerA')).to.eql(0)
+    expect(out.indexOf('layerB')).to.eql(1)
+  })
+
   it('ignores cyclic dependencies', () => {
-    const out = topoSort({ a: 'b', b: 'a', c: 'a' }, (node, inheritance) => inheritance[node])
+    const out = topoSort({ a: 'b', b: 'a', c: 'a' }, (node, inheritance) => [inheritance[node]])
     expect(out.indexOf('a')).to.be.below(out.indexOf('c'))
   })
 })