[Architecture/fix]: Better documentation & error handling of issues causing inconsistent rendering

.changeset/blue-bottles-look.md

@@ -0,0 +1,5 @@
+---
+'mermaid': patch
+---
+
+fix: resolved inconsistent rendering with architecture diagrams

cypress/integration/rendering/architecture.spec.ts

@@ -223,6 +223,79 @@ describe.skip('architecture diagram', () => {
       `
     );
   });
+
+  it('should render a consistent architecture diagram', () => {
+    // #6024
+    imgSnapshotTest(
+      `architecture-beta
+              group api(cloud)[API]
+
+              service db(database)[Database] in api
+              service disk1(disk)[Storage] in api
+              service disk2(disk)[Storage] in api
+              service server(server)[Server] in api
+
+              db:L <--> R:server
+              disk1:T <--> L:server
+              disk2:T <--> R:db
+      `
+    );
+  });
+
+  it('should render a consistent architecture diagram', () => {
+    // #6166
+    imgSnapshotTest(
+      `architecture-beta
+          service client[Client]
+          junction j1
+          junction j2
+
+          service api(logos:aws-api-gateway)[Amazon API Gateway]
+          service lambda1(logos:aws-lambda)[AWS Lambda 1]
+
+          service dynamodb(logos:aws-dynamodb)[Amazon DynamoDB]
+
+          service s3(logos:aws-s3)[Amazon S3]
+          service lambda2(logos:aws-lambda)[AWS Lambda 2]
+
+          client:R -[1]-> L:api
+          api:R -[2]-> L:lambda1
+          lambda1:R -[3]-> T:dynamodb
+
+          client:B -[j1]- T:j1
+          j1:B -[j1 to j2]- T:j2
+          j2:R -[j2]- L:s3
+
+          s3:R -[5]-> L:lambda2
+          lambda2:R -[6]-> B:dynamodb
+      `
+    );
+  });
+
+  it('should render an error', () => {
+    // #6166
+    imgSnapshotTest(
+      `architecture-beta
+              architecture-beta
+              service client[Client]
+              service api(logos:aws-api-gateway)[Amazon API Gateway]
+              service lambda1(logos:aws-lambda)[AWS Lambda]
+
+              service dynamodb(logos:aws-dynamodb)[Amazon DynamoDB]
+
+              service s3(logos:aws-s3)[Amazon S3]
+              service lambda2(logos:aws-lambda)[AWS Lambda]
+
+              client:R -[1]-> L:api
+              api:R -[2]-> L:lambda1
+              lambda1:R -[3]-> T:dynamodb
+
+              client:B -[4]-> L:s3
+              s3:R -[5]-> L:lambda2
+              lambda2:R -[6]-> B:dynamodb
+      `
+    );
+  });
 });
 
 // Skipped as the layout is not deterministic, and causes issues in E2E tests.

docs/syntax/architecture.md

@@ -191,6 +191,141 @@ architecture-beta
     bottom_gateway:T -- B:junctionRight
 ```
 
+## Limitations
+
+Due to underlying limitations in the rendering engine, there are a couple things to keep in mind when designing your Architecture diagrams.
+
+### Grid-based layout
+
+Internally, Architecture diagrams are represented using a grid layout where each slot in the grid is a service. When designing your diagrams, take care to ensure there isn't a jump/empty slot between two services.
+
+For example:
+
+```
+architecture-beta
+    service client[Client]                                 %%[0, 0] <--
+    service api(logos:aws-api-gateway)[Amazon API Gateway] %%[1, 0]
+    service lambda1(logos:aws-lambda)[AWS Lambda]          %%[2, 0]
+
+    service dynamodb(logos:aws-dynamodb)[Amazon DynamoDB]  %%[3,-1]
+
+    service s3(logos:aws-s3)[Amazon S3]                    %%[1,-2] <--
+    service lambda2(logos:aws-lambda)[AWS Lambda]          %%[2,-2]
+
+    client:R -[1]-> L:api
+    api:R -[2]-> L:lambda1
+    lambda1:R -[3]-> T:dynamodb
+
+
+    client:B -[4]-> L:s3                                   %% invalid edge (y difference from 0 to -2 is > 1)
+    s3:R -[5]-> L:lambda2
+    lambda2:R -[6]-> B:dynamodb
+```
+
+Would attempt to render as:
+
+![](./img/Architecture-grid.png)
+
+There are two potential ways to resolve this:
+
+1. **Without junctions**
+
+```mermaid-example
+architecture-beta
+    service client[Client]                                 %%[0, 0]
+    service api(logos:aws-api-gateway)[Amazon API Gateway] %%[1, 0]
+    service lambda1(logos:aws-lambda)[AWS Lambda]          %%[2, 0]
+
+    service dynamodb(logos:aws-dynamodb)[Amazon DynamoDB]  %%[3,-1]
+
+    service s3(logos:aws-s3)[Amazon S3]                    %%[1,-1]
+    service lambda2(logos:aws-lambda)[AWS Lambda]          %%[2,-1]
+
+    client:R -[1]-> L:api
+    api:R -[2]-> L:lambda1
+    lambda1:R -[3]-> T:dynamodb
+
+
+    client:B -[4]-> L:s3
+    s3:R -[5]-> L:lambda2
+    lambda2:R -[6]-> L:dynamodb                            %% Modified
+```
+
+```mermaid
+architecture-beta
+    service client[Client]                                 %%[0, 0]
+    service api(logos:aws-api-gateway)[Amazon API Gateway] %%[1, 0]
+    service lambda1(logos:aws-lambda)[AWS Lambda]          %%[2, 0]
+
+    service dynamodb(logos:aws-dynamodb)[Amazon DynamoDB]  %%[3,-1]
+
+    service s3(logos:aws-s3)[Amazon S3]                    %%[1,-1]
+    service lambda2(logos:aws-lambda)[AWS Lambda]          %%[2,-1]
+
+    client:R -[1]-> L:api
+    api:R -[2]-> L:lambda1
+    lambda1:R -[3]-> T:dynamodb
+
+
+    client:B -[4]-> L:s3
+    s3:R -[5]-> L:lambda2
+    lambda2:R -[6]-> L:dynamodb                            %% Modified
+```
+
+2. **With junctions**
+
+```mermaid-example
+architecture-beta
+    service client[Client]                                   %%[0, 0]
+    junction j1                                              %%[0,-1]
+    junction j2                                              %%[0,-2]
+
+    service api(logos:aws-api-gateway)[Amazon API Gateway]   %%[1, 0]
+    service lambda1(logos:aws-lambda)[AWS Lambda 1]          %%[2, 0]
+
+    service dynamodb(logos:aws-dynamodb)[Amazon DynamoDB]    %%[3,-1]
+
+    service s3(logos:aws-s3)[Amazon S3]                      %%[1,-2]
+    service lambda2(logos:aws-lambda)[AWS Lambda 2]          %%[2,-2]
+
+    client:R -[1]-> L:api
+    api:R -[2]-> L:lambda1
+    lambda1:R -[3]-> T:dynamodb
+
+    client:B -[j1]- T:j1
+    j1:B -[j1 to j2]- T:j2
+    j2:R -[j2]- L:s3
+
+    s3:R -[5]-> L:lambda2
+    lambda2:R -[6]-> B:dynamodb
+```
+
+```mermaid
+architecture-beta
+    service client[Client]                                   %%[0, 0]
+    junction j1                                              %%[0,-1]
+    junction j2                                              %%[0,-2]
+
+    service api(logos:aws-api-gateway)[Amazon API Gateway]   %%[1, 0]
+    service lambda1(logos:aws-lambda)[AWS Lambda 1]          %%[2, 0]
+
+    service dynamodb(logos:aws-dynamodb)[Amazon DynamoDB]    %%[3,-1]
+
+    service s3(logos:aws-s3)[Amazon S3]                      %%[1,-2]
+    service lambda2(logos:aws-lambda)[AWS Lambda 2]          %%[2,-2]
+
+    client:R -[1]-> L:api
+    api:R -[2]-> L:lambda1
+    lambda1:R -[3]-> T:dynamodb
+
+    client:B -[j1]- T:j1
+    j1:B -[j1 to j2]- T:j2
+    j2:R -[j2]- L:s3
+
+    s3:R -[5]-> L:lambda2
+    lambda2:R -[6]-> B:dynamodb
+```
+
 ## Icons
 
 By default, architecture diagram supports the following icons: `cloud`, `database`, `disk`, `internet`, `server`.

packages/mermaid/src/diagrams/architecture/architectureDb.ts

@@ -285,10 +285,18 @@ const getDataStructures = () => {
           const [posX, posY] = spatialMap[id];
           Object.entries(adj).forEach(([dir, rhsId]) => {
             if (!visited[rhsId]) {
-              spatialMap[rhsId] = shiftPositionByArchitectureDirectionPair(
+              const shift = shiftPositionByArchitectureDirectionPair(
                 [posX, posY],
                 dir as ArchitectureDirectionPair
               );
+
+              if (spatialMap[rhsId] && spatialMap[rhsId].toString() !== shift.toString()) {
+                throw new Error(
+                  `Edges cannot jump more then 1 grid space. Please see the "Considerations" section of the diagram documentation for details.`
+                );
+              }
+
+              spatialMap[rhsId] = shift;
               queue.push(rhsId);
             }
           });
@@ -308,9 +316,42 @@ const getDataStructures = () => {
       groupAlignments,
     };
   }
+  validateDataStructures(state.records.dataStructures);
   return state.records.dataStructures;
 };
 
+/**
+ * Validates the data structures for irregularities that could effect rendering accuracy.
+ * @param ds - data structures create by `getDataStructures()`
+ */
+const validateDataStructures = (ds: typeof state.records.dataStructures) => {
+  if (ds) {
+    const { adjList, spatialMaps } = ds;
+
+    // Throw error if two services are more then 2 units away from one another
+    // See: #6166
+    Object.entries(adjList).forEach(([sourceNode, edges]) => {
+      Object.values(edges).forEach((targetNode) => {
+        spatialMaps.forEach((spatialMap) => {
+          const [sX, sY] = spatialMap[sourceNode];
+          const [tX, tY] = spatialMap[targetNode];
+
+          const xDiff = Math.abs(sX - tX);
+          const yDiff = Math.abs(sY - tY);
+
+          const errMsg = `Edges cannot jump more then 1 grid space ([${sourceNode}]--[${targetNode}]). Please see the "Considerations" section of the diagram documentation for details.`;
+          if (xDiff > 1) {
+            throw new Error(errMsg);
+          }
+          if (yDiff > 1) {
+            throw new Error(errMsg);
+          }
+        });
+      });
+    });
+  }
+};
+
 const setElementForId = (id: string, element: D3Element) => {
   state.records.elements[id] = element;
 };

packages/mermaid/src/diagrams/architecture/architectureRenderer.ts

@@ -12,6 +12,7 @@ import { setupGraphViewbox } from '../../setupGraphViewbox.js';
 import { getConfigField } from './architectureDb.js';
 import { architectureIcons } from './architectureIcons.js';
 import type {
+  ArchitectureAdjacencyList,
   ArchitectureAlignment,
   ArchitectureDataStructures,
   ArchitectureGroupAlignments,
@@ -257,7 +258,8 @@ function getAlignments(
 }
 
 function getRelativeConstraints(
-  spatialMaps: ArchitectureSpatialMap[]
+  spatialMaps: ArchitectureSpatialMap[],
+  adjList: ArchitectureAdjacencyList
 ): fcose.FcoseRelativePlacementConstraint[] {
   const relativeConstraints: fcose.FcoseRelativePlacementConstraint[] = [];
   const posToStr = (pos: number[]) => `${pos[0]},${pos[1]}`;
@@ -268,14 +270,33 @@ function getRelativeConstraints(
       Object.entries(spatialMap).map(([id, pos]) => [posToStr(pos), id])
     );
 
+    // Restructure the adjList to just track which services connect (both ways)
+    const newAdjList = Object.entries(adjList).reduce(
+      (prev, [lhsId, pair]) => {
+        prev[lhsId] ??= {};
+        Object.values(pair).forEach((rhsId) => {
+          prev[lhsId][rhsId] = true;
+          prev[rhsId] ??= {};
+          prev[rhsId][lhsId] = true;
+        });
+
+        return prev;
+      },
+      {} as Record<string, Record<string, boolean>>
+    );
+
     // perform BFS
     const queue = [posToStr([0, 0])];
     const visited: Record<string, number> = {};
-    const directions: Record<ArchitectureDirection, number[]> = {
+    const directions: Record<ArchitectureDirection | 'LB' | 'LT' | 'RB' | 'RT', number[]> = {
       L: [-1, 0],
       R: [1, 0],
       T: [0, 1],
       B: [0, -1],
+      LB: [-1, -1],
+      LT: [-1, 1],
+      RB: [1, -1],
+      RT: [1, 1],
     };
     while (queue.length > 0) {
       const curr = queue.shift();
@@ -288,7 +309,7 @@ function getRelativeConstraints(
             const newPos = posToStr([currPos[0] + shift[0], currPos[1] + shift[1]]);
             const newId = invSpatialMap[newPos];
             // If there is an adjacent service to the current one and it has not yet been visited
-            if (newId && !visited[newPos]) {
+            if (dir.length === 1 && newId && !visited[newPos]) {
               queue.push(newPos);
               // @ts-ignore cannot determine if left/right or top/bottom are paired together
               relativeConstraints.push({
@@ -298,6 +319,26 @@ function getRelativeConstraints(
                 ]]: currId,
                 gap: 1.5 * getConfigField('iconSize'),
               });
+              // For XY, only add a constraint if there is a physical connection between the nodes
+            } else if (dir.length === 2 && newId && !visited[newPos] && newAdjList[currId][newId]) {
+              queue.push(newPos);
+              relativeConstraints.push(
+                // @ts-ignore cannot determine if left/right or top/bottom are paired together
+                {
+                  [ArchitectureDirectionName[dir[0] as ArchitectureDirection]]: newId,
+                  [ArchitectureDirectionName[
+                    getOppositeArchitectureDirection(dir[0] as ArchitectureDirection)
+                  ]]: currId,
+                  gap: 1.5 * getConfigField('iconSize'),
+                },
+                {
+                  [ArchitectureDirectionName[dir[1] as ArchitectureDirection]]: newId,
+                  [ArchitectureDirectionName[
+                    getOppositeArchitectureDirection(dir[1] as ArchitectureDirection)
+                  ]]: currId,
+                  gap: 1.5 * getConfigField('iconSize'),
+                }
+              );
             }
           });
         }
@@ -313,7 +354,7 @@ function layoutArchitecture(
   groups: ArchitectureGroup[],
   edges: ArchitectureEdge[],
   db: ArchitectureDB,
-  { spatialMaps, groupAlignments }: ArchitectureDataStructures
+  { spatialMaps, groupAlignments, adjList }: ArchitectureDataStructures
 ): Promise<cytoscape.Core> {
   return new Promise((resolve) => {
     const renderEl = select('body').append('div').attr('id', 'cy').attr('style', 'display:none');
@@ -391,7 +432,7 @@ function layoutArchitecture(
     const alignmentConstraint = getAlignments(db, spatialMaps, groupAlignments);
 
     // Create the relative constraints for fcose by using an inverse of the spatial map and performing BFS on it
-    const relativePlacementConstraint = getRelativeConstraints(spatialMaps);
+    const relativePlacementConstraint = getRelativeConstraints(spatialMaps, adjList);
 
     const layout = cy.layout({
       name: 'fcose',