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 8d8245e67..23a066523 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 23a066523..a2956b5d8 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 770ee24b4..2767f09b4 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 000000000..5e29c42d6 --- /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 a2956b5d8..9e3f64a6c 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 42e3bbbb4..1669cfada 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 2767f09b4..b2c5cf620 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 071776df2..e2eea5e19 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 89beb85e0..162619d99 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 55da67934..6f1657169 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 4e5b631ff..c5f3c3cab 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)