chore(widget): make controller more functional (#182)

* promote detail-navigation-header out of assets directory
* processStep and processThing no longer side effect into the nodesById and edges they are passed
* replace forEach with for ... of
* improve test descriptions

packages/widget/src/assets/index.ts

@@ -1,4 +1,3 @@
 // All files which will be accessible when the widget is installed via npm are declared here
 export * from './close-details-button';
 export * from './logo';
-export * from './detail-navigation-header';

packages/widget/src/assets/detail-navigation-header.ts → packages/widget/src/detail-navigation-header.ts

@@ -1,5 +1,5 @@
 import { nothing, svg, SVGTemplateResult } from 'lit';
-import { DisplayObject, TYPE_DISPLAY_OPTIONS } from '../util';
+import { DisplayObject, TYPE_DISPLAY_OPTIONS } from './util';
 
 const backButton = (
   allNodes: DisplayObject[],

packages/widget/src/docmap-controller.ts

@@ -59,58 +59,82 @@ function getOrderedSteps(docmap: DocmapT): StepT[] {
   return orderedSteps;
 }
 
-export function stepsToGraph(steps: StepT[]): DisplayObjectGraph {
-  const nodesById: { [id: string]: DisplayObject } = {};
-  const edges: DisplayObjectEdge[] = [];
+interface NodesById {
+  [id: string]: DisplayObject;
+}
 
-  let idCounter: number = 1;
-  const idGenerator = (): string => {
-    const newId = `n${idCounter}`;
-    idCounter++;
-    return newId;
-  };
+export function stepsToGraph(steps: StepT[]): DisplayObjectGraph {
+  let nodesById: NodesById = {};
+  let edges: DisplayObjectEdge[] = [];
 
-  steps.forEach((step) => processStep(step, nodesById, edges, idGenerator));
+  for (const step of steps) {
+    const { newNodesById, newEdges } = processStep(step, nodesById);
+    nodesById = newNodesById;
+    edges = [...edges, ...newEdges];
+  }
 
   const nodes: DisplayObject[] = Object.values(nodesById);
   return { nodes, edges };
 }
 
 function processStep(
   step: StepT,
-  nodesById: { [id: string]: DisplayObject },
-  edges: DisplayObjectEdge[],
-  generateId: () => string,
-) {
-  const inputIds: string[] =
-    step.inputs?.map((input) => processThing(input, nodesById, [], generateId)) || [];
-
-  step.actions.forEach((action) => {
-    action.outputs.forEach((output) => {
-      const outputId = processThing(output, nodesById, action.participants, generateId);
-
-      inputIds.forEach((inputId: string) => {
-        edges.push({ sourceId: inputId, targetId: outputId });
-      });
-    });
-  });
+  nodesById: NodesById,
+): { newNodesById: NodesById; newEdges: DisplayObjectEdge[] } {
+  let newNodesById: NodesById = { ...nodesById };
+  let newEdges: DisplayObjectEdge[] = [];
+
+  const inputIds =
+    step.inputs?.map((input) => {
+      const result = processThing(input, newNodesById);
+      newNodesById = result.newNodesById;
+      return result.id;
+    }) || [];
+
+  for (const action of step.actions) {
+    for (const output of action.outputs) {
+      const result = processThing(output, newNodesById, action.participants);
+      newNodesById = result.newNodesById;
+
+      const edgesForThisOutput: DisplayObjectEdge[] = inputIds.map(
+        (inputId): DisplayObjectEdge => ({
+          sourceId: inputId,
+          targetId: result.id,
+        }),
+      );
+      newEdges = [...newEdges, ...edgesForThisOutput];
+    }
+  }
+
+  // Return the newly created objects instead of the mutated ones
+  return { newNodesById, newEdges };
 }
 
 function processThing(
   thing: ThingT,
-  nodesById: { [id: string]: DisplayObject },
+  inputNodes: NodesById,
   participants: RoleInTimeT[] = [],
-  generateId: () => string,
-): string {
-  const id: string = thing.doi || thing.id || generateId();
-  if (!(id in nodesById)) {
-    nodesById[id] = thingToDisplayObject(thing, id, participants);
+): { id: string; newNodesById: NodesById } {
+  const newNodesById: NodesById = { ...inputNodes };
+  const id = thing.doi || thing.id || generateId(newNodesById);
+
+  if (!(id in newNodesById)) {
+    newNodesById[id] = thingToDisplayObject(thing, id, participants);
   }
-  return id;
+
+  return { id, newNodesById };
 }
 
-interface NameHaver {
-  name: string;
+function generateId(nodesById: NodesById): string {
+  let idCounter: number = 1;
+  let newId: string = `n${idCounter}`;
+
+  while (newId in nodesById) {
+    idCounter++;
+    newId = `n${idCounter}`;
+  }
+
+  return newId;
 }
 
 function thingToDisplayObject(
@@ -150,6 +174,10 @@ function extractContentUrls(content: ManifestationT[] | undefined) {
     .filter((url: string | undefined): url is string => url !== undefined);
 }
 
+interface NameHaver {
+  name: string;
+}
+
 function extractActorNames(participants: RoleInTimeT[]) {
   return participants
     .map((participant) => participant.actor)
@@ -184,6 +212,7 @@ function formatDate(date: Date) {
   return yyyy + '-' + mm + '-' + dd;
 }
 
+// These sort functions are only used by tests currently, but seem universally useful
 export function sortDisplayObjects(objects: DisplayObject[]): DisplayObject[] {
   return [...objects].sort((a: DisplayObject, b: DisplayObject) =>
     a.nodeId.localeCompare(b.nodeId),

packages/widget/src/docmaps-widget.ts

@@ -1,7 +1,7 @@
 import { html, HTMLTemplateResult, LitElement, nothing } from 'lit';
 import { customElement, property, state } from 'lit/decorators.js';
 import { customCss } from './styles';
-import { closeDetailsButton, logo, renderDetailNavigationHeader } from './assets';
+import { closeDetailsButton, logo } from './assets';
 import { Task } from '@lit/task';
 import { DocmapFetchingParams, getDocmap } from './docmap-controller';
 import {
@@ -12,6 +12,7 @@ import {
   TYPE_DISPLAY_OPTIONS,
 } from './util';
 import { clearGraph, displayGraph } from './graph-view';
+import { renderDetailNavigationHeader } from './detail-navigation-header';
 
 @customElement('docmaps-widget')
 export class DocmapsWidget extends LitElement {

packages/widget/test/unit/docmap-controller.test.ts

@@ -4,9 +4,9 @@ import {
   sortDisplayObjectEdges,
   sortDisplayObjects,
   stepsToGraph,
-} from '../../src/docmap-controller';
-import docmapWithOneStep from '../fixtures/sciety-docmap-1';
-import anotherDocmapWithOneStep from '../fixtures/sciety-docmap-2';
+} from '../../src';
+import smallDocmapWithOneStep from '../fixtures/sciety-docmap-1';
+import largeDocmapWithOneStep from '../fixtures/sciety-docmap-2';
 import docmapWithMultipleSteps from '../fixtures/elife-docmap-1';
 
 interface Item {
@@ -15,7 +15,7 @@ interface Item {
 }
 
 test('getSteps when there is only one step', (t) => {
-  const steps = getSteps(docmapWithOneStep);
+  const steps = getSteps(smallDocmapWithOneStep);
   t.is(steps.length, 1);
   const step = steps[0];
   t.is(step.inputs[0].doi, '10.21203/rs.3.rs-1043992/v1');
@@ -70,17 +70,17 @@ test('getSteps on non-docmaps', (t) => {
 });
 
 test('getSteps on docmaps without a first step', (t) => {
-  const result = getSteps({ ...docmapWithOneStep, 'first-step': undefined });
+  const result = getSteps({ ...smallDocmapWithOneStep, 'first-step': undefined });
   t.deepEqual(result, []);
 });
 
 test('getSteps on docmaps with no steps', (t) => {
-  const result = getSteps({ ...docmapWithOneStep, steps: undefined });
+  const result = getSteps({ ...smallDocmapWithOneStep, steps: undefined });
   t.deepEqual(result, []);
 });
 
 test('stepsToGraph for a docmap with one step', (t) => {
-  const steps = getSteps(docmapWithOneStep);
+  const steps = getSteps(smallDocmapWithOneStep);
   const { nodes, edges } = stepsToGraph(steps);
 
   t.is(nodes.length, 2);
@@ -105,8 +105,8 @@ test('stepsToGraph for a docmap with one step', (t) => {
   ]);
 });
 
-test('stepsToGraph for another docmap with one step', (t) => {
-  const steps = getSteps(anotherDocmapWithOneStep);
+test('stepsToGraph for a larger docmap with one step', (t) => {
+  const steps = getSteps(largeDocmapWithOneStep);
   const { nodes, edges } = stepsToGraph(steps);
 
   t.is(nodes.length, 4);