Repository: maxGraph/maxGraph
Pull request #445 (2024-06-06 07:40:30)

fix!: make `cellArrayUtils.getOpposite` return an array of Cells
Previously, it returned a function which created an extra indirection and make it harder to migrate from mxGraph as it didn't match the original signature.
Also simplify the implementation after having added tests to validate the expected behavior.

BREAKING CHANGES:
- The signature of `cellArrayUtils.getOpposites` has changed. It now returns an array and take an edges Cell array parameter.



## Notes

`mxGraph` original code in `mxGraphModel`: https://github.com/jgraph/mxgraph/blob/v4.2.2/javascript/src/js/model/mxGraphModel.js#L1457

Covers #355





Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f56e61df4..93fa76dc5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,9 @@
 GlobalConfig.logger = new MaxLogAsLogger();
 ```
 - `MaxWindow.activeWindow` is no longer available; it was intended for internal use only, so there's no reason to make it public.
+- The signature of `cellArrayUtils.getOpposites` has changed. It now returns an array and take an edges Cell array parameter.
+Previously it was returning a function and this was an extra indirection. This is now simpler to use and match the signature of the mxGraph function.
+
 
 ## 0.10.3
 
diff --git a/packages/core/src/util/cellArrayUtils.ts b/packages/core/src/util/cellArrayUtils.ts
index a9d97d824..132b8ee4d 100644
--- a/packages/core/src/util/cellArrayUtils.ts
+++ b/packages/core/src/util/cellArrayUtils.ts
@@ -19,42 +19,41 @@ export const filterCells = (filter: Function) => (cells: Cell[]) => {
 };
 
 /**
- * Returns all opposite vertices wrt terminal for the given edges, only
- * returning sources and/or targets as specified. The result is returned
- * as an array of {@link Cell}.
+ * Returns all opposite vertices terminal for the given edges, only returning sources and/or targets as specified.
+ * The result is returned as an array of {@link Cell}.
  *
+ * @param edges Array of {Cell} that contain the edges to be examined.
  * @param {Cell} terminal  that specifies the known end of the edges.
- * @param sources  Boolean that specifies if source terminals should be contained
- * in the result. Default is true.
- * @param targets  Boolean that specifies if target terminals should be contained
- * in the result. Default is true.
+ * @param includeSources  Boolean that specifies if source terminals should be included in the result. Default is `true`.
+ * @param includeTargets  Boolean that specifies if target terminals should be included in the result. Default is `true`.
  */
-export const getOpposites =
-  (terminal: Cell, sources = true, targets = true) =>
-  (edges: Cell[]) => {
-    const terminals = [] as Cell[];
-
-    for (let i = 0; i < edges.length; i += 1) {
-      const source = edges[i].getTerminal(true);
-      const target = edges[i].getTerminal(false);
-
-      // Checks if the terminal is the source of
-      // the edge and if the target should be
-      // stored in the result
-      if (source === terminal && target != null && target !== terminal && targets) {
-        terminals.push(target);
-      }
-
-      // Checks if the terminal is the taget of
-      // the edge and if the source should be
-      // stored in the result
-      else if (target === terminal && source != null && source !== terminal && sources) {
-        terminals.push(source);
-      }
+export const getOpposites = (
+  edges: Cell[],
+  terminal: Cell,
+  includeSources = true,
+  includeTargets = true
+): Cell[] => {
+  return edges.reduce((terminals, edge) => {
+    const source = edge.getTerminal(true);
+    const target = edge.getTerminal(false);
+
+    // Checks if the terminal is the source of the edge and if the target should be stored in the result
+    if (source === terminal && target != null && target !== terminal && includeTargets) {
+      terminals.push(target);
+    }
+    // Checks if the terminal is the target of the edge and if the source should be stored in the result
+    else if (
+      target === terminal &&
+      source != null &&
+      source !== terminal &&
+      includeSources
+    ) {
+      terminals.push(source);
     }
 
     return terminals;
-  };
+  }, [] as Cell[]);
+};
 
 /**
  * Returns the topmost cells of the hierarchy in an array that contains no
@@ -163,8 +162,7 @@ const cloneCellImpl = (cell: Cell, mapping: any = {}, includeChildren = false):
 };
 
 /**
- * Inner helper method for restoring the connections in
- * a network of cloned cells.
+ * Inner helper method for restoring the connections in a network of cloned cells.
  *
  * @private
  */
diff --git a/packages/core/src/view/mixins/TerminalMixin.ts b/packages/core/src/view/mixins/TerminalMixin.ts
index 00a3dbee6..6fedda576 100644
--- a/packages/core/src/view/mixins/TerminalMixin.ts
+++ b/packages/core/src/view/mixins/TerminalMixin.ts
@@ -21,12 +21,32 @@ import { mixInto } from '../../util/Utils';
 
 declare module '../Graph' {
   interface Graph {
+    /**
+     * Returns true if the given terminal point is movable. This is independent
+     * from {@link isCellConnectable} and {@link isCellDisconnectable} and controls if terminal
+     * points can be moved in the graph if the edge is not connected.
+     * Note that it is required for this to return true to connect unconnected edges.
+     *
+     * This implementation returns `true`.
+     *
+     * @param cell {@link Cell} whose terminal point should be moved.
+     * @param source Boolean indicating if the source or target terminal should be moved.
+     */
     isTerminalPointMovable: (cell: Cell, source: boolean) => boolean;
+
+    /**
+     * Returns all distinct visible opposite cells for the specified terminal on the given edges.
+     *
+     * @param edges Array of {@link Cell} that contains the edges whose opposite terminals should be returned.
+     * @param terminal Terminal that specifies the end whose opposite should be returned. Default is `null`.
+     * @param includeSources Optional boolean that specifies if source terminals should be included in the result. Default is `true`.
+     * @param includeTargets Optional boolean that specifies if target terminals should be included in the result. Default is `true`.
+     */
     getOpposites: (
       edges: Cell[],
       terminal: Cell | null,
-      sources?: boolean,
-      targets?: boolean
+      includeSources?: boolean,
+      includeTargets?: boolean
     ) => Cell[];
   }
 }
@@ -37,43 +57,17 @@ type PartialType = PartialGraph & PartialTerminal;
 
 // @ts-expect-error The properties of PartialGraph are defined elsewhere.
 const TerminalMixin: PartialType = {
-  /*****************************************************************************
-   * Group: Graph behaviour
-   *****************************************************************************/
-
-  /**
-   * Returns true if the given terminal point is movable. This is independent
-   * from {@link isCellConnectable} and {@link isCellDisconnectable} and controls if terminal
-   * points can be moved in the graph if the edge is not connected. Note that it
-   * is required for this to return true to connect unconnected edges. This
-   * implementation returns true.
-   *
-   * @param cell {@link mxCell} whose terminal point should be moved.
-   * @param source Boolean indicating if the source or target terminal should be moved.
-   */
   isTerminalPointMovable(cell, source) {
     return true;
   },
 
-  /*****************************************************************************
-   * Group: Cell retrieval
-   *****************************************************************************/
-
-  /**
-   * Returns all distinct visible opposite cells for the specified terminal
-   * on the given edges.
-   *
-   * @param edges Array of {@link Cell} that contains the edges whose opposite
-   * terminals should be returned.
-   * @param terminal Terminal that specifies the end whose opposite should be
-   * returned.
-   * @param sources Optional boolean that specifies if source terminals should be
-   * included in the result. Default is `true`.
-   * @param targets Optional boolean that specifies if targer terminals should be
-   * included in the result. Default is `true`.
-   */
-  getOpposites(edges, terminal = null, sources = true, targets = true) {
-    const terminals = [];
+  getOpposites(
+    edges,
+    terminal = null,
+    includeSources = true,
+    includeTargets = true
+  ): Cell[] {
+    const terminals: Cell[] = [];
 
     // Fast lookup to avoid duplicates in terminals array
     const dict = new Dictionary<Cell, boolean>();
@@ -90,7 +84,7 @@ const TerminalMixin: PartialType = {
 
       // Checks if the terminal is the source of the edge and if the
       // target should be stored in the result
-      if (source === terminal && target && target !== terminal && targets) {
+      if (source === terminal && target && target !== terminal && includeTargets) {
         if (!dict.get(target)) {
           dict.put(target, true);
           terminals.push(target);
@@ -99,7 +93,7 @@ const TerminalMixin: PartialType = {
 
       // Checks if the terminal is the taget of the edge and if the
       // source should be stored in the result
-      else if (target === terminal && source && source !== terminal && sources) {
+      else if (target === terminal && source && source !== terminal && includeSources) {
         if (!dict.get(source)) {
           dict.put(source, true);
           terminals.push(source);
diff --git a/packages/website/docs/usage/migrate-from-mxgraph.md b/packages/website/docs/usage/migrate-from-mxgraph.md
index 3ee764916..8a8b39bb8 100644
--- a/packages/website/docs/usage/migrate-from-mxgraph.md
+++ b/packages/website/docs/usage/migrate-from-mxgraph.md
@@ -344,11 +344,8 @@ Moved methods
 
 
 ### Cell manipulation
-Functions that existed in mxGraph and mxGraphModel have been removed. They provided a way to extend/override the default behavior of mxGraphModel or mxCell.
-Only the functions for mxCell/Cell remain. See https://github.com/maxGraph/maxGraph/pull/24
 
-
-Some functions previously available in `mxGraph` and `mxGraphModel` have been removed. These functions allowed for customizing the behavior of `mxGraphModel` and `mxCell`. However, now only the functions specific to `mxCell`/`Cell` remain.  
+Some methods previously available in `mxGraph` and `mxGraphModel` have been removed. These methods allowed for customizing the behavior of `mxGraphModel` and `mxCell`. However, now only the methods specific to `Cell` remain.  
 
 :::note
 
@@ -356,6 +353,7 @@ You can find more information about these changes in the following GitHub pull r
 
 :::
 
+
 #### `mxCell`
 
 The `mxCell` class has been renamed to `Cell` for simplicity.
@@ -372,15 +370,24 @@ Some methods were removed:
 
 #### `mxGraphDataModel`
 
-Several functions from the `mxGraphDataModel` class have been moved to the `Cell` class. These functions no longer need the `cell` parameter:
+Several methods from the `mxGraphDataModel` class have been moved to the `Cell` class. These methods no longer need the `cell` parameter:
 
 - `filterDescendants()`
 - `getGeometry()`
 - `isEdge()`
 - `getParent()`
 
+Some methods in `mxGraphModel` that were general manipulation of cells and independent of the model have been moved to the `cellArrayUtils` namespace and are now available as individual functions.
+- `cloneImpl()` (should not be used, private implementation)
+- `cloneCells()`
+- `getOpposite()`
+- `getParents()`
+- `getTopmostCells()`
+- `restoreClone()` (should not be used, private implementation)
+
 Others were removed:
-- `mxGraphModel.isVisible(cell)` see https://github.com/maxGraph/maxGraph/discussions/179#discussioncomment-5389942 for rationale
+- `isVisible(cell)` see https://github.com/maxGraph/maxGraph/discussions/179#discussioncomment-5389942 for rationale
+- `cloneCell()`
 
 ### Misc
 


Test patch:
diff --git a/packages/core/__tests__/util/cellArrayUtils.test.ts b/packages/core/__tests__/util/cellArrayUtils.test.ts
new file mode 100644
index 000000000..2ab602f97
--- /dev/null
+++ b/packages/core/__tests__/util/cellArrayUtils.test.ts
@@ -0,0 +1,75 @@
+/*
+Copyright 2024-present The maxGraph project Contributors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { describe, expect, test } from '@jest/globals';
+import { getOpposites } from '../../src/util/cellArrayUtils';
+import Cell from '../../src/view/cell/Cell';
+
+describe('getOpposites', () => {
+  const edges: Cell[] = [];
+  const terminal = new Cell();
+
+  // source == terminal
+  const edge1 = new Cell();
+  edge1.setTerminal(terminal, true);
+  const oppositeOfMatchingSource = new Cell();
+  edge1.setTerminal(oppositeOfMatchingSource, false);
+  edges.push(edge1);
+  // target == terminal
+  const edge2 = new Cell();
+  const oppositeOfMatchingTarget = new Cell();
+  edge2.setTerminal(oppositeOfMatchingTarget, true);
+  edge2.setTerminal(terminal, false);
+  edges.push(edge2);
+  // edge without source and target
+  const edge3 = new Cell();
+  edges.push(edge3);
+
+  // edge with source matching terminal, but no target
+  const edge4 = new Cell();
+  edge4.setTerminal(terminal, true);
+  edges.push(edge4);
+
+  // edge with target matching terminal, but no target
+  const edge5 = new Cell();
+  edge5.setTerminal(terminal, false);
+  edges.push(edge5);
+
+  // edge with no match terminal
+  const edge6 = new Cell();
+  edge6.setTerminal(new Cell(), true);
+  edge6.setTerminal(new Cell(), false);
+  edges.push(edge6);
+
+  test('include sources and targets', () => {
+    expect(getOpposites(edges, terminal)).toStrictEqual([
+      oppositeOfMatchingSource,
+      oppositeOfMatchingTarget,
+    ]);
+  });
+
+  test('include sources', () => {
+    expect(getOpposites(edges, terminal, true, false)).toStrictEqual([
+      oppositeOfMatchingTarget,
+    ]);
+  });
+
+  test('include targets', () => {
+    expect(getOpposites(edges, terminal, false, true)).toStrictEqual([
+      oppositeOfMatchingSource,
+    ]);
+  });
+});