From fd4aec4d0c4b10b15addac58db18463712ce4489 Mon Sep 17 00:00:00 2001
From: cha0s <git@cha0s.io>
Date: Mon, 8 Jan 2024 22:58:03 -0600
Subject: [PATCH] feat: elided fleck implementation sorting

---
 packages/core/src/digraph.js     |  82 +++++++++++++++++++++++
 packages/core/src/flecks.js      | 108 ++++++++++++++++++++++++-------
 packages/core/test/middleware.js |  77 ++++++++++++++--------
 packages/core/test/one/index.js  |   6 +-
 website/docs/adding-flecks.mdx   |  43 ------------
 5 files changed, 222 insertions(+), 94 deletions(-)
 create mode 100644 packages/core/src/digraph.js

diff --git a/packages/core/src/digraph.js b/packages/core/src/digraph.js
new file mode 100644
index 0000000..25e1796
--- /dev/null
+++ b/packages/core/src/digraph.js
@@ -0,0 +1,82 @@
+class Digraph {
+
+  arcs = new Map();
+
+  addArc(tail, head) {
+    this.arcs.get(tail).add(head);
+  }
+
+  detectCycles() {
+    const cycles = [];
+    const visited = new Set();
+    const walking = new Set();
+    const walk = (vertex) => {
+      if (!visited.has(vertex)) {
+        visited.add(vertex);
+        walking.add(vertex);
+        const it = this.neighbors(vertex);
+        for (let current = it.next(); true !== current.done; current = it.next()) {
+          const {value: neighbor} = current;
+          if (!visited.has(neighbor)) {
+            walk(neighbor);
+          }
+          else if (walking.has(neighbor)) {
+            cycles.push([vertex, neighbor]);
+          }
+        }
+      }
+      walking.delete(vertex);
+    };
+    const {tails} = this;
+    for (let current = tails.next(); true !== current.done; current = tails.next()) {
+      walk(current.value);
+    }
+    return cycles;
+  }
+
+  ensureTail(tail) {
+    if (!this.arcs.has(tail)) {
+      this.arcs.set(tail, new Set());
+    }
+  }
+
+  neighbors(vertex) {
+    return this.arcs.get(vertex).values();
+  }
+
+  sort() {
+    const visited = new Set();
+    const scores = new Map();
+    const walk = (vertex, score) => {
+      visited.add(vertex);
+      const neighbors = this.neighbors(vertex);
+      for (let current = neighbors.next(); true !== current.done; current = neighbors.next()) {
+        const {value: neighbor} = current;
+        if (!visited.has(neighbor)) {
+          // eslint-disable-next-line no-param-reassign
+          score = walk(neighbor, score);
+        }
+      }
+      scores.set(vertex, score);
+      return score - 1;
+    };
+    let score = this.arcs.size - 1;
+    const {tails} = this;
+    for (let current = tails.next(); true !== current.done; current = tails.next()) {
+      const {value: vertex} = current;
+      if (!visited.has(vertex)) {
+        score = walk(vertex, score);
+      }
+    }
+    return Array.from(scores.entries())
+      .sort(([, l], [, r]) => l - r)
+      .map(([vertex]) => vertex);
+  }
+
+  get tails() {
+    return this.arcs.keys();
+  }
+
+}
+
+export default Digraph;
diff --git a/packages/core/src/flecks.js b/packages/core/src/flecks.js
index fa180b5..65cfd47 100644
--- a/packages/core/src/flecks.js
+++ b/packages/core/src/flecks.js
@@ -11,11 +11,15 @@ import set from 'lodash.set';
 import without from 'lodash.without';
 
 import D from './debug';
+import Digraph from './digraph';
 import Middleware from './middleware';
 
 const debug = D('@flecks/core/flecks');
 const debugSilly = debug.extend('silly');
 
+// Symbol for hook ordering.
+const HookOrder = Symbol.for('@flecks/core.hookOrder');
+
 // Symbols for Gathered classes.
 export const ById = Symbol.for('@flecks/core.byId');
 export const ByType = Symbol.for('@flecks/core.byType');
@@ -91,6 +95,40 @@ export default class Flecks {
     debugSilly('config: %O', this.config);
   }
 
+  /**
+   * Configure a hook implementation to run after another implementation.
+   *
+   * @param {string[]} after
+   * @param {function} implementation
+   */
+  static after(after, implementation) {
+    if (!implementation[HookOrder]) {
+      implementation[HookOrder] = {};
+    }
+    if (!implementation[HookOrder].after) {
+      implementation[HookOrder].after = [];
+    }
+    implementation[HookOrder].after.push(...after);
+    return implementation;
+  }
+
+  /**
+   * Configure a hook implementation to run before another implementation.
+   *
+   * @param {string[]} before
+   * @param {function} implementation
+   */
+  static before(before, implementation) {
+    if (!implementation[HookOrder]) {
+      implementation[HookOrder] = {};
+    }
+    if (!implementation[HookOrder].before) {
+      implementation[HookOrder].before = [];
+    }
+    implementation[HookOrder].before.push(...before);
+    return implementation;
+  }
+
   /**
    * Configure defaults for a fleck.
    *
@@ -228,15 +266,49 @@ export default class Flecks {
         for (let j = 0; j < this.platforms.length; ++j) {
           const platform = this.platforms[j];
           const variant = join(fleck, platform);
-          if (this.fleck(variant)) {
+          if (this.fleck(variant) && this.fleckImplementation(variant, hook)) {
             all.push(variant);
           }
         }
       }
-      const rest = without(all, ...before.concat(after));
-      expanded = [...before, ...rest, ...after];
+      // Map the elided fleck implementations to vertices in a dependency graph.
+      const graph = new Digraph();
+      without(all, ...before.concat(after))
+        .forEach((fleck) => {
+          graph.ensureTail(fleck);
+          const implementation = this.fleckImplementation(fleck, hook);
+          if (implementation[HookOrder]) {
+            if (implementation[HookOrder].before) {
+              implementation[HookOrder].before.forEach((before) => {
+                graph.addArc(fleck, before);
+              });
+            }
+            if (implementation[HookOrder].after) {
+              implementation[HookOrder].after.forEach((after) => {
+                graph.ensureTail(after);
+                graph.addArc(after, fleck);
+              });
+            }
+          }
+
+        });
+      // Check for cycles.
+      const cycles = graph.detectCycles();
+      if (cycles.length > 0) {
+        throw new Error(
+          `Illegal ordering specification: hook '${
+            hook
+          }' has positioning cycles: ${
+            cycles.map(([l, r]) => `${l} <-> ${r}`).join(', ')
+          }`,
+        );
+      }
+      // Sort the graph and place it.
+      expanded = [...before, ...graph.sort(), ...after];
     }
-    return expanded;
+    // Filter unimplemented.
+    return expanded
+      .filter((fleck) => this.fleckImplementation(fleck, hook));
   }
 
   /**
@@ -251,14 +323,14 @@ export default class Flecks {
   }
 
   /**
-   * Test whether a fleck implements a hook.
+   * Get a fleck's implementation of a hook.
    *
    * @param {*} fleck
    * @param {string} hook
    * @returns {boolean}
    */
-  fleckImplements(fleck, hook) {
-    return !!this.hooks[hook]?.find(({fleck: candidate}) => fleck === candidate);
+  fleckImplementation(fleck, hook) {
+    return this.hooks[hook]?.find(({fleck: candidate}) => fleck === candidate);
   }
 
   /**
@@ -373,7 +445,6 @@ export default class Flecks {
       return initial;
     }
     return flecks
-      .filter((fleck) => this.fleckImplements(fleck, hook))
       .reduce((r, fleck) => this.invokeFleck(hook, fleck, r, ...args), initial);
   }
 
@@ -391,7 +462,6 @@ export default class Flecks {
       return arg;
     }
     return flecks
-      .filter((fleck) => this.fleckImplements(fleck, hook))
       .reduce(async (r, fleck) => this.invokeFleck(hook, fleck, await r, ...args), arg);
   }
 
@@ -551,9 +621,7 @@ export default class Flecks {
     const results = [];
     while (flecks.length > 0) {
       const fleck = flecks.shift();
-      if (this.fleckImplements(fleck, hook)) {
-        results.push(this.invokeFleck(hook, fleck, ...args));
-      }
+      results.push(this.invokeFleck(hook, fleck, ...args));
     }
     return results;
   }
@@ -574,10 +642,8 @@ export default class Flecks {
     const results = [];
     while (flecks.length > 0) {
       const fleck = flecks.shift();
-      if (this.fleckImplements(fleck, hook)) {
-        // eslint-disable-next-line no-await-in-loop
-        results.push(await this.invokeFleck(hook, fleck, ...args));
-      }
+      // eslint-disable-next-line no-await-in-loop
+      results.push(await this.invokeFleck(hook, fleck, ...args));
     }
     return results;
   }
@@ -616,10 +682,8 @@ export default class Flecks {
     if (0 === flecks.length) {
       return (...args) => args.pop()();
     }
-    const middleware = flecks
-      .filter((fleck) => this.fleckImplements(fleck, hook));
-    debugSilly('middleware: %O', middleware);
-    const instance = new Middleware(middleware.map((fleck) => this.invokeFleck(hook, fleck)));
+    debugSilly('middleware: %O', flecks);
+    const instance = new Middleware(flecks.map((fleck) => this.invokeFleck(hook, fleck)));
     return instance.dispatch.bind(instance);
   }
 
@@ -703,12 +767,12 @@ export default class Flecks {
       } = current;
       let raw;
       // If decorating, gather all again
-      if (this.fleckImplements(fleck, `${hook}.decorate`)) {
+      if (this.fleckImplementation(fleck, `${hook}.decorate`)) {
         raw = this.invokeMerge(hook);
         debugSilly('%s implements %s.decorate', fleck, hook);
       }
       // If only implementing, gather and decorate.
-      else if (this.fleckImplements(fleck, hook)) {
+      else if (this.fleckImplementation(fleck, hook)) {
         raw = this.invokeFleck(hook, fleck);
         debugSilly('%s implements %s', fleck, hook);
       }
diff --git a/packages/core/test/middleware.js b/packages/core/test/middleware.js
index bbeb25e..1767a86 100644
--- a/packages/core/test/middleware.js
+++ b/packages/core/test/middleware.js
@@ -6,10 +6,7 @@ const testOne = require('./one');
 const testTwo = require('./two');
 
 it('can make middleware', (done) => {
-  let flecks;
-  let foo;
-  let mw;
-  flecks = new Flecks({
+  const flecks = new Flecks({
     config: {
       '@flecks/core/test': {
         middleware: [
@@ -23,29 +20,57 @@ it('can make middleware', (done) => {
       '@flecks/core/two': testTwo,
     },
   });
-  foo = {bar: 1};
-  mw = flecks.makeMiddleware('@flecks/core/test.middleware');
+  const foo = {bar: 1};
+  const mw = flecks.makeMiddleware('@flecks/core/test.middleware');
   mw(foo, () => {
     expect(foo.bar).to.equal(4);
-    flecks = new Flecks({
-      config: {
-        '@flecks/core/test': {
-          middleware: [
-            '@flecks/core/two',
-            '@flecks/core/one',
-          ],
-        },
-      },
-      flecks: {
-        '@flecks/core/one': testOne,
-        '@flecks/core/two': testTwo,
-      },
-    });
-    foo = {bar: 1};
-    mw = flecks.makeMiddleware('@flecks/core/test.middleware');
-    mw(foo, () => {
-      expect(foo.bar).to.equal(3);
-      done();
-    });
+    done();
+  });
+});
+
+it('respects explicit middleware configuration', (done) => {
+  const flecks = new Flecks({
+    config: {
+      '@flecks/core/test': {
+        middleware: [
+          '@flecks/core/two',
+          '@flecks/core/one',
+        ],
+      },
+    },
+    flecks: {
+      // Intentionally default to the wrong order...
+      '@flecks/core/one': testOne,
+      '@flecks/core/two': testTwo,
+    },
+  });
+  const foo = {bar: 1};
+  const mw = flecks.makeMiddleware('@flecks/core/test.middleware');
+  mw(foo, () => {
+    expect(foo.bar).to.equal(3);
+    done();
+  });
+});
+
+it('respects middleware elision', (done) => {
+  const flecks = new Flecks({
+    config: {
+      '@flecks/core/test': {
+        middleware: [
+          '...',
+        ],
+      },
+    },
+    flecks: {
+      // Intentionally default to the wrong order...
+      '@flecks/core/one': testOne,
+      '@flecks/core/two': testTwo,
+    },
+  });
+  const foo = {bar: 1};
+  const mw = flecks.makeMiddleware('@flecks/core/test.middleware');
+  mw(foo, () => {
+    expect(foo.bar).to.equal(3);
+    done();
   });
 });
diff --git a/packages/core/test/one/index.js b/packages/core/test/one/index.js
index e4d54e6..3b4d117 100644
--- a/packages/core/test/one/index.js
+++ b/packages/core/test/one/index.js
@@ -17,7 +17,7 @@ export const hooks = {
     Flecks.provide(require.context('./things', false, /^\..*\.js$/))
   ),
   '@flecks/core/one/test-gather.decorate': (
-    Flecks.decorate(require.context('./things/decorators', false, /\..*\.js$/))
+    Flecks.decorate(require.context('./things/decorators', false, /^\..*\.js$/))
   ),
   '@flecks/core/test/invoke': () => 69,
   '@flecks/core/test/invoke-parallel': (O) => {
@@ -27,8 +27,8 @@ export const hooks = {
   '@flecks/core/test/invoke-merge-async': () => new Promise((resolve) => { resolve({foo: 69}); }),
   '@flecks/core/test/invoke-merge-unique': () => ({foo: 69}),
   '@flecks/core/test/invoke-merge-unique-async': () => new Promise((resolve) => { resolve({foo: 69}); }),
-  '@flecks/core/test.middleware': () => (foo, next) => {
+  '@flecks/core/test.middleware': Flecks.after(['@flecks/core/two'], () => (foo, next) => {
     foo.bar += 1;
     next();
-  },
+  }),
 };
diff --git a/website/docs/adding-flecks.mdx b/website/docs/adding-flecks.mdx
index 899cd16..4bb4493 100644
--- a/website/docs/adding-flecks.mdx
+++ b/website/docs/adding-flecks.mdx
@@ -65,49 +65,6 @@ application inside of an instance of [Electron](https://www.electronjs.org/). Yo
 npx flecks add @flecks/electron
 ```
 
-Then you'll update your `build/flecks.yml` like so:
-
-```yml
-'@flecks/core':
-  id: 'hello-world'
-'@flecks/electron': {}
-// highlight-start
-'@flecks/server':
-  up:
-    - '...'
-    - '@flecks/web'
-    - '@flecks/electron'
-// highlight-end
-'@flecks/web': {}
-'@hello-world/say-hello:./packages/say-hello/src': {}
-```
-
-### ~~flecking~~ pecking order
-
-We added some configuration to `@flecks/server`. The `up` key configures the order in which flecks
-are initialized when the server comes up. We make sure `@flecks/web` serves a webpage before
-`@flecks/electron` tries to visit it.
-
-:::tip[...and Bob's your uncle]
-
-`'...'` just means "everything else": if any other flecks implement that hook then they will run
-here in an **undefined** order. It is valid to provide entries both before and after `'...'`, but
-`'...'` must only appear one time per list.
-
-The default configuration of
-`@flecks/server.up` is simply:
-
-```yml
-'@flecks/server':
-  up:
-    - '...'
-```
-
-However in this case the order of hook execution is undefined. That's why we configure it
-explicitly.
-
-:::
-
 Finally `npm start` and you will see something like this:
 
 ![An image of our simple hello world application running inside an Electron window](./flecks-electron.png)