Skip to content

Commit 5fabd41

Browse files
committedOct 28, 2024·
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: #5976
1 parent 17e2f9e commit 5fabd41

File tree

7 files changed

+42
-10
lines changed

7 files changed

+42
-10
lines changed
 

‎.changeset/thick-elephants-search.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'mermaid': patch
3+
---
4+
5+
fix: error `mermaid.parse` on an invalid shape, so that it matches the errors thrown by `mermaid.render`

‎packages/mermaid/src/diagrams/flowchart/flowDb.ts

+12-7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { select } from 'd3';
22
import utils, { getEdgeId } from '../../utils.js';
33
import { getConfig, defaultConfig } from '../../diagram-api/diagramAPI.js';
44
import common from '../common/common.js';
5+
import { isValidShape, type ShapeID } from '../../rendering-util/rendering-elements/shapes.js';
56
import type { Node, Edge } from '../../rendering-util/types.js';
67
import { log } from '../../logger.js';
78
import * as yaml from 'js-yaml';
@@ -140,14 +141,15 @@ export const addVertex = function (
140141
}
141142
// console.log('yamlData', yamlData);
142143
const doc = yaml.load(yamlData, { schema: yaml.JSON_SCHEMA }) as NodeMetaData;
143-
if (doc.shape && (doc.shape !== doc.shape.toLowerCase() || doc.shape.includes('_'))) {
144-
throw new Error(`No such shape: ${doc.shape}. Shape names should be lowercase.`);
145-
}
146-
147-
// console.log('yamlData doc', doc);
148-
if (doc?.shape) {
144+
if (doc.shape) {
145+
if (doc.shape !== doc.shape.toLowerCase() || doc.shape.includes('_')) {
146+
throw new Error(`No such shape: ${doc.shape}. Shape names should be lowercase.`);
147+
} else if (!isValidShape(doc.shape)) {
148+
throw new Error(`No such shape: ${doc.shape}.`);
149+
}
149150
vertex.type = doc?.shape;
150151
}
152+
151153
if (doc?.label) {
152154
vertex.text = doc?.label;
153155
}
@@ -823,7 +825,7 @@ export const lex = {
823825
firstGraph,
824826
};
825827

826-
const getTypeFromVertex = (vertex: FlowVertex) => {
828+
const getTypeFromVertex = (vertex: FlowVertex): ShapeID => {
827829
if (vertex.img) {
828830
return 'imageSquare';
829831
}
@@ -845,6 +847,9 @@ const getTypeFromVertex = (vertex: FlowVertex) => {
845847
return 'squareRect';
846848
case 'round':
847849
return 'roundedRect';
850+
case 'ellipse':
851+
// @ts-expect-error -- Ellipses are broken, see https://github.com/mermaid-js/mermaid/issues/5976
852+
return 'ellipse';
848853
default:
849854
return vertex.type;
850855
}

‎packages/mermaid/src/diagrams/flowchart/parser/flow-node-data.spec.js

+15
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,21 @@ describe('when parsing directions', function () {
197197
expect(data4Layout.nodes[0].shape).toEqual('squareRect');
198198
expect(data4Layout.nodes[0].label).toEqual('This is }');
199199
});
200+
it('should error on non-existent shape', function () {
201+
expect(() => {
202+
flow.parser.parse(`flowchart TB
203+
A@{ shape: this-shape-does-not-exist }
204+
`);
205+
}).toThrow('No such shape: this-shape-does-not-exist.');
206+
});
207+
it('should error on internal-only shape', function () {
208+
expect(() => {
209+
// this shape does exist, but it's only supposed to be for internal/backwards compatibility use
210+
flow.parser.parse(`flowchart TB
211+
A@{ shape: rect_left_inv_arrow }
212+
`);
213+
}).toThrow('No such shape: rect_left_inv_arrow. Shape names should be lowercase.');
214+
});
200215
it('Diamond shapes should work as usual', function () {
201216
const res = flow.parser.parse(`flowchart TB
202217
A{This is a label}

‎packages/mermaid/src/diagrams/flowchart/types.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type { ShapeID } from '../../rendering-util/rendering-elements/shapes.js';
2+
13
/**
24
* Valid `type` args to `yy.addVertex` taken from
35
* `packages/mermaid/src/diagrams/flowchart/parser/flow.jison`
@@ -33,7 +35,7 @@ export interface FlowVertex {
3335
props?: any;
3436
styles: string[];
3537
text?: string;
36-
type?: string;
38+
type?: ShapeID | FlowVertexTypeParam;
3739
icon?: string;
3840
form?: string;
3941
pos?: 't' | 'b';

‎packages/mermaid/src/rendering-util/rendering-elements/nodes.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export async function insertNode(elem: SVGGroup, node: Node, renderOptions: Shap
2323
}
2424
}
2525

26-
const shapeHandler = shapes[(node.shape ?? 'undefined') as keyof typeof shapes];
26+
const shapeHandler = node.shape ? shapes[node.shape] : undefined;
2727

2828
if (!shapeHandler) {
2929
throw new Error(`No such shape: ${node.shape}. Please check your syntax.`);

‎packages/mermaid/src/rendering-util/rendering-elements/shapes.ts

+4
Original file line numberDiff line numberDiff line change
@@ -490,4 +490,8 @@ const generateShapeMap = () => {
490490

491491
export const shapes = generateShapeMap();
492492

493+
export function isValidShape(shape: string): shape is ShapeID {
494+
return shape in shapes;
495+
}
496+
493497
export type ShapeID = keyof typeof shapes;

‎packages/mermaid/src/rendering-util/types.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
export type MarkdownWordType = 'normal' | 'strong' | 'em';
22
import type { MermaidConfig } from '../config.type.js';
3+
import type { ShapeID } from './rendering-elements/shapes.js';
34
export interface MarkdownWord {
45
content: string;
56
type: MarkdownWordType;
@@ -37,7 +38,7 @@ export interface Node {
3738
linkTarget?: string;
3839
tooltip?: string;
3940
padding?: number; //REMOVE?, use from LayoutData.config - Keep, this could be shape specific
40-
shape?: string;
41+
shape?: ShapeID;
4142
isGroup: boolean;
4243
width?: number;
4344
height?: number;

0 commit comments

Comments
 (0)
Please sign in to comment.