From 7fa8b35bdc14b295a08dfd300f0d357d2390bc1e Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Fri, 18 Oct 2024 16:57:46 +0900 Subject: [PATCH 1/4] refactor: convert if-statements to switch..case --- packages/mermaid/src/diagrams/flowchart/flowDb.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.ts index 8d8245e677..23a066523e 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.ts @@ -832,14 +832,15 @@ const getTypeFromVertex = (vertex: FlowVertex) => { } return 'icon'; } - if (vertex.type === 'square') { - return 'squareRect'; + switch (vertex.type) { + case 'square': + case undefined: + return 'squareRect'; + case 'round': + return 'roundedRect'; + default: + return vertex.type; } - if (vertex.type === 'round') { - return 'roundedRect'; - } - - return vertex.type ?? 'squareRect'; }; const findNode = (nodes: Node[], id: string) => nodes.find((node) => node.id === id); From 17e2f9e0d4935cee1d032b0bc4fb6e9de37fe912 Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Fri, 18 Oct 2024 16:59:21 +0900 Subject: [PATCH 2/4] refactor: fix `addVertex` `type` parameter I went through `packages/mermaid/src/diagrams/flowchart/parser/flow.jison` and found every possible value this `type` parameter could be. --- .../mermaid/src/diagrams/flowchart/flowDb.ts | 13 ++++++++--- .../mermaid/src/diagrams/flowchart/types.ts | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.ts index 23a066523e..a2956b5d84 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.ts @@ -14,7 +14,15 @@ import { setDiagramTitle, getDiagramTitle, } from '../common/commonDb.js'; -import type { FlowVertex, FlowClass, FlowSubGraph, FlowText, FlowEdge, FlowLink } from './types.js'; +import type { + FlowVertex, + FlowClass, + FlowSubGraph, + FlowText, + FlowEdge, + FlowLink, + FlowVertexTypeParam, +} from './types.js'; import type { NodeMetaData } from '../../types.js'; const MERMAID_DOM_ID_PREFIX = 'flowchart-'; @@ -53,12 +61,11 @@ export const lookUpDomId = function (id: string) { /** * Function called by parser when a node definition has been found - * */ export const addVertex = function ( id: string, textObj: FlowText, - type: 'group', + type: FlowVertexTypeParam, style: string[], classes: string[], dir: string, diff --git a/packages/mermaid/src/diagrams/flowchart/types.ts b/packages/mermaid/src/diagrams/flowchart/types.ts index 770ee24b46..2767f09b40 100644 --- a/packages/mermaid/src/diagrams/flowchart/types.ts +++ b/packages/mermaid/src/diagrams/flowchart/types.ts @@ -1,3 +1,26 @@ +/** + * Valid `type` args to `yy.addVertex` taken from + * `packages/mermaid/src/diagrams/flowchart/parser/flow.jison` + */ +export type FlowVertexTypeParam = + | undefined + | 'square' + | 'doublecircle' + | 'circle' + | 'ellipse' + | 'stadium' + | 'subroutine' + | 'rect' + | 'cylinder' + | 'round' + | 'diamond' + | 'hexagon' + | 'odd' + | 'trapezoid' + | 'inv_trapezoid' + | 'lean_right' + | 'lean_left'; + export interface FlowVertex { classes: string[]; dir?: string; From 5fabd414fbee01e43bf6c900907ffc1511ca7440 Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Fri, 18 Oct 2024 19:36:08 +0900 Subject: [PATCH 3/4] fix: error `mermaid.parse` on invalid shapes Currently, invalid shapes cause an error when rendering, but not when parsing. This confuses the Mermaid Live Editor, e.g. by not showing the error message. Instead, I've added an `isValidShape()` that validates if the shape is valid at parse time. This only checks shapes using the new extended shapes syntax. Currenlty, using `A(-this is an ellipse node-)` is broken (see #5976) and also causes an invalid shape error at render time, but I've ignored it in this PR, so it will continue pass at parse-time (we have unit tests checking ellipse parsing). See: https://github.com/mermaid-js/mermaid/issues/5976 --- .changeset/thick-elephants-search.md | 5 +++++ .../mermaid/src/diagrams/flowchart/flowDb.ts | 19 ++++++++++++------- .../flowchart/parser/flow-node-data.spec.js | 15 +++++++++++++++ .../mermaid/src/diagrams/flowchart/types.ts | 4 +++- .../rendering-elements/nodes.ts | 2 +- .../rendering-elements/shapes.ts | 4 ++++ packages/mermaid/src/rendering-util/types.ts | 3 ++- 7 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 .changeset/thick-elephants-search.md diff --git a/.changeset/thick-elephants-search.md b/.changeset/thick-elephants-search.md new file mode 100644 index 0000000000..5e29c42d66 --- /dev/null +++ b/.changeset/thick-elephants-search.md @@ -0,0 +1,5 @@ +--- +'mermaid': patch +--- + +fix: error `mermaid.parse` on an invalid shape, so that it matches the errors thrown by `mermaid.render` diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.ts index a2956b5d84..9e3f64a6ce 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.ts @@ -2,6 +2,7 @@ import { select } from 'd3'; import utils, { getEdgeId } from '../../utils.js'; import { getConfig, defaultConfig } from '../../diagram-api/diagramAPI.js'; import common from '../common/common.js'; +import { isValidShape, type ShapeID } from '../../rendering-util/rendering-elements/shapes.js'; import type { Node, Edge } from '../../rendering-util/types.js'; import { log } from '../../logger.js'; import * as yaml from 'js-yaml'; @@ -140,14 +141,15 @@ export const addVertex = function ( } // console.log('yamlData', yamlData); const doc = yaml.load(yamlData, { schema: yaml.JSON_SCHEMA }) as NodeMetaData; - if (doc.shape && (doc.shape !== doc.shape.toLowerCase() || doc.shape.includes('_'))) { - throw new Error(`No such shape: ${doc.shape}. Shape names should be lowercase.`); - } - - // console.log('yamlData doc', doc); - if (doc?.shape) { + if (doc.shape) { + if (doc.shape !== doc.shape.toLowerCase() || doc.shape.includes('_')) { + throw new Error(`No such shape: ${doc.shape}. Shape names should be lowercase.`); + } else if (!isValidShape(doc.shape)) { + throw new Error(`No such shape: ${doc.shape}.`); + } vertex.type = doc?.shape; } + if (doc?.label) { vertex.text = doc?.label; } @@ -823,7 +825,7 @@ export const lex = { firstGraph, }; -const getTypeFromVertex = (vertex: FlowVertex) => { +const getTypeFromVertex = (vertex: FlowVertex): ShapeID => { if (vertex.img) { return 'imageSquare'; } @@ -845,6 +847,9 @@ const getTypeFromVertex = (vertex: FlowVertex) => { return 'squareRect'; case 'round': return 'roundedRect'; + case 'ellipse': + // @ts-expect-error -- Ellipses are broken, see https://github.com/mermaid-js/mermaid/issues/5976 + return 'ellipse'; default: return vertex.type; } diff --git a/packages/mermaid/src/diagrams/flowchart/parser/flow-node-data.spec.js b/packages/mermaid/src/diagrams/flowchart/parser/flow-node-data.spec.js index 42e3bbbb4e..1669cfada0 100644 --- a/packages/mermaid/src/diagrams/flowchart/parser/flow-node-data.spec.js +++ b/packages/mermaid/src/diagrams/flowchart/parser/flow-node-data.spec.js @@ -197,6 +197,21 @@ describe('when parsing directions', function () { expect(data4Layout.nodes[0].shape).toEqual('squareRect'); expect(data4Layout.nodes[0].label).toEqual('This is }'); }); + it('should error on non-existent shape', function () { + expect(() => { + flow.parser.parse(`flowchart TB + A@{ shape: this-shape-does-not-exist } + `); + }).toThrow('No such shape: this-shape-does-not-exist.'); + }); + it('should error on internal-only shape', function () { + expect(() => { + // this shape does exist, but it's only supposed to be for internal/backwards compatibility use + flow.parser.parse(`flowchart TB + A@{ shape: rect_left_inv_arrow } + `); + }).toThrow('No such shape: rect_left_inv_arrow. Shape names should be lowercase.'); + }); it('Diamond shapes should work as usual', function () { const res = flow.parser.parse(`flowchart TB A{This is a label} diff --git a/packages/mermaid/src/diagrams/flowchart/types.ts b/packages/mermaid/src/diagrams/flowchart/types.ts index 2767f09b40..b2c5cf6202 100644 --- a/packages/mermaid/src/diagrams/flowchart/types.ts +++ b/packages/mermaid/src/diagrams/flowchart/types.ts @@ -1,3 +1,5 @@ +import type { ShapeID } from '../../rendering-util/rendering-elements/shapes.js'; + /** * Valid `type` args to `yy.addVertex` taken from * `packages/mermaid/src/diagrams/flowchart/parser/flow.jison` @@ -33,7 +35,7 @@ export interface FlowVertex { props?: any; styles: string[]; text?: string; - type?: string; + type?: ShapeID | FlowVertexTypeParam; icon?: string; form?: string; pos?: 't' | 'b'; diff --git a/packages/mermaid/src/rendering-util/rendering-elements/nodes.ts b/packages/mermaid/src/rendering-util/rendering-elements/nodes.ts index 071776df20..e2eea5e198 100644 --- a/packages/mermaid/src/rendering-util/rendering-elements/nodes.ts +++ b/packages/mermaid/src/rendering-util/rendering-elements/nodes.ts @@ -23,7 +23,7 @@ export async function insertNode(elem: SVGGroup, node: Node, renderOptions: Shap } } - const shapeHandler = shapes[(node.shape ?? 'undefined') as keyof typeof shapes]; + const shapeHandler = node.shape ? shapes[node.shape] : undefined; if (!shapeHandler) { throw new Error(`No such shape: ${node.shape}. Please check your syntax.`); diff --git a/packages/mermaid/src/rendering-util/rendering-elements/shapes.ts b/packages/mermaid/src/rendering-util/rendering-elements/shapes.ts index 89beb85e01..162619d997 100644 --- a/packages/mermaid/src/rendering-util/rendering-elements/shapes.ts +++ b/packages/mermaid/src/rendering-util/rendering-elements/shapes.ts @@ -490,4 +490,8 @@ const generateShapeMap = () => { export const shapes = generateShapeMap(); +export function isValidShape(shape: string): shape is ShapeID { + return shape in shapes; +} + export type ShapeID = keyof typeof shapes; diff --git a/packages/mermaid/src/rendering-util/types.ts b/packages/mermaid/src/rendering-util/types.ts index 55da679349..6f16571690 100644 --- a/packages/mermaid/src/rendering-util/types.ts +++ b/packages/mermaid/src/rendering-util/types.ts @@ -1,5 +1,6 @@ export type MarkdownWordType = 'normal' | 'strong' | 'em'; import type { MermaidConfig } from '../config.type.js'; +import type { ShapeID } from './rendering-elements/shapes.js'; export interface MarkdownWord { content: string; type: MarkdownWordType; @@ -37,7 +38,7 @@ export interface Node { linkTarget?: string; tooltip?: string; padding?: number; //REMOVE?, use from LayoutData.config - Keep, this could be shape specific - shape?: string; + shape?: ShapeID; isGroup: boolean; width?: number; height?: number; From 848be3d1297b833c4e8b2e25fabafec4d7c4db02 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 14:33:47 +0000 Subject: [PATCH 4/4] [autofix.ci] apply automated fixes --- docs/config/setup/interfaces/mermaid.LayoutData.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/config/setup/interfaces/mermaid.LayoutData.md b/docs/config/setup/interfaces/mermaid.LayoutData.md index 4e5b631ff3..c5f3c3cab9 100644 --- a/docs/config/setup/interfaces/mermaid.LayoutData.md +++ b/docs/config/setup/interfaces/mermaid.LayoutData.md @@ -20,7 +20,7 @@ #### Defined in -[packages/mermaid/src/rendering-util/types.ts:125](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L125) +[packages/mermaid/src/rendering-util/types.ts:126](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L126) --- @@ -30,7 +30,7 @@ #### Defined in -[packages/mermaid/src/rendering-util/types.ts:124](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L124) +[packages/mermaid/src/rendering-util/types.ts:125](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L125) --- @@ -40,4 +40,4 @@ #### Defined in -[packages/mermaid/src/rendering-util/types.ts:123](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L123) +[packages/mermaid/src/rendering-util/types.ts:124](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L124)