From 5fabd414fbee01e43bf6c900907ffc1511ca7440 Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Fri, 18 Oct 2024 19:36:08 +0900 Subject: [PATCH] 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;