From 4bf5c9f3d844905bf0664c8116bb41dd5fc1ffd7 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Fri, 24 Feb 2023 17:27:24 +0530 Subject: [PATCH] feat: Ensure proper detection for flowcharts --- .../mermaid/src/diagram-api/detectType.ts | 4 +- .../diagram-api/diagram-orchestration.spec.ts | 37 +++++++++++++++++++ .../src/diagram-api/diagram-orchestration.ts | 4 +- .../src/diagram-api/diagramAPI.spec.ts | 4 +- packages/mermaid/src/diagram.spec.ts | 4 +- .../src/diagrams/flowchart/flowDetector-v2.ts | 10 ++--- .../src/diagrams/flowchart/flowDetector.ts | 8 ++-- packages/mermaid/src/mermaid.ts | 2 + packages/mermaid/src/mermaidAPI.spec.ts | 2 +- 9 files changed, 57 insertions(+), 18 deletions(-) diff --git a/packages/mermaid/src/diagram-api/detectType.ts b/packages/mermaid/src/diagram-api/detectType.ts index 6ffe5df85..3d55dc78f 100644 --- a/packages/mermaid/src/diagram-api/detectType.ts +++ b/packages/mermaid/src/diagram-api/detectType.ts @@ -46,7 +46,9 @@ export const detectType = function (text: string, config?: MermaidConfig): strin } } - throw new UnknownDiagramError(`No diagram type detected for text: ${text}`); + throw new UnknownDiagramError( + `No diagram type detected matching given configuration for text: ${text}` + ); }; export const registerLazyLoadedDiagrams = (...diagrams: ExternalDiagramDefinition[]) => { diff --git a/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts b/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts index f08f32391..81909fe5e 100644 --- a/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts +++ b/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts @@ -41,5 +41,42 @@ describe('diagram-orchestration', () => { expect(detectType(text)).toBe(expected); } ); + + it('should detect proper flowchart type based on config', () => { + // graph & dagre-d3 => flowchart + expect(detectType('graph TD; A-->B')).toBe('flowchart'); + // graph & dagre-d3 => flowchart + expect(detectType('graph TD; A-->B', { flowchart: { defaultRenderer: 'dagre-d3' } })).toBe( + 'flowchart' + ); + // flowchart & dagre-d3 => error + expect(() => + detectType('flowchart TD; A-->B', { flowchart: { defaultRenderer: 'dagre-d3' } }) + ).toThrowErrorMatchingInlineSnapshot( + '"No diagram type detected matching given configuration for text: flowchart TD; A-->B"' + ); + + // graph & dagre-wrapper => flowchart-v2 + expect( + detectType('graph TD; A-->B', { flowchart: { defaultRenderer: 'dagre-wrapper' } }) + ).toBe('flowchart-v2'); + // flowchart ==> flowchart-v2 + expect(detectType('flowchart TD; A-->B')).toBe('flowchart-v2'); + // flowchart && dagre-wrapper ==> flowchart-v2 + expect( + detectType('flowchart TD; A-->B', { flowchart: { defaultRenderer: 'dagre-wrapper' } }) + ).toBe('flowchart-v2'); + // flowchart && elk ==> flowchart-elk + expect(detectType('flowchart TD; A-->B', { flowchart: { defaultRenderer: 'elk' } })).toBe( + 'flowchart-elk' + ); + }); + + it('should not detect flowchart if pie contains flowchart', () => { + expect( + detectType(`pie title: "flowchart" + flowchart: 1 "pie" pie: 2 "pie"`) + ).toBe('pie'); + }); }); }); diff --git a/packages/mermaid/src/diagram-api/diagram-orchestration.ts b/packages/mermaid/src/diagram-api/diagram-orchestration.ts index da4c4d081..73bfcf084 100644 --- a/packages/mermaid/src/diagram-api/diagram-orchestration.ts +++ b/packages/mermaid/src/diagram-api/diagram-orchestration.ts @@ -68,10 +68,8 @@ export const addDiagrams = () => { requirement, sequence, flowchartElk, - // TODO @knsv: Should v2 come before flowchart? - // This will fail few unit tests as they expect graph to be detected as flowchart, but it is detected as flowchart-v2. - flowchart, flowchartV2, + flowchart, mindmap, timeline, git, diff --git a/packages/mermaid/src/diagram-api/diagramAPI.spec.ts b/packages/mermaid/src/diagram-api/diagramAPI.spec.ts index 9e04c861f..73453fa51 100644 --- a/packages/mermaid/src/diagram-api/diagramAPI.spec.ts +++ b/packages/mermaid/src/diagram-api/diagramAPI.spec.ts @@ -20,8 +20,8 @@ describe('DiagramAPI', () => { it('should handle diagram registrations', () => { expect(() => getDiagram('loki')).toThrow(); - expect(() => detectType('loki diagram')).toThrow( - 'No diagram type detected for text: loki diagram' + expect(() => detectType('loki diagram')).toThrowErrorMatchingInlineSnapshot( + '"No diagram type detected matching given configuration for text: loki diagram"' ); const detector: DiagramDetector = (str: string) => { return str.match('loki') !== null; diff --git a/packages/mermaid/src/diagram.spec.ts b/packages/mermaid/src/diagram.spec.ts index a862c7936..5a4718d0b 100644 --- a/packages/mermaid/src/diagram.spec.ts +++ b/packages/mermaid/src/diagram.spec.ts @@ -61,8 +61,8 @@ Expecting 'TXT', got 'NEWLINE'" }); test('should throw the right error for unregistered diagrams', async () => { - await expect(getDiagramFromText('thor TD; A-->B')).rejects.toThrowError( - 'No diagram type detected for text: thor TD; A-->B' + await expect(getDiagramFromText('thor TD; A-->B')).rejects.toThrowErrorMatchingInlineSnapshot( + '"No diagram type detected matching given configuration for text: thor TD; A-->B"' ); }); }); diff --git a/packages/mermaid/src/diagrams/flowchart/flowDetector-v2.ts b/packages/mermaid/src/diagrams/flowchart/flowDetector-v2.ts index 6162cc828..5b2aaf1bd 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDetector-v2.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDetector-v2.ts @@ -4,15 +4,15 @@ import type { ExternalDiagramDefinition } from '../../diagram-api/types'; const id = 'flowchart-v2'; const detector: DiagramDetector = (txt, config) => { - if (config?.flowchart?.defaultRenderer === 'dagre-d3') { - return false; - } - if (config?.flowchart?.defaultRenderer === 'elk') { + if ( + config?.flowchart?.defaultRenderer === 'dagre-d3' || + config?.flowchart?.defaultRenderer === 'elk' + ) { return false; } // If we have configured to use dagre-wrapper then we should return true in this function for graph code thus making it use the new flowchart diagram - if (txt.match(/^\s*graph/) !== null) { + if (txt.match(/^\s*graph/) !== null && config?.flowchart?.defaultRenderer === 'dagre-wrapper') { return true; } return txt.match(/^\s*flowchart/) !== null; diff --git a/packages/mermaid/src/diagrams/flowchart/flowDetector.ts b/packages/mermaid/src/diagrams/flowchart/flowDetector.ts index 9457ff469..a8b116ccd 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDetector.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDetector.ts @@ -5,10 +5,10 @@ const id = 'flowchart'; const detector: DiagramDetector = (txt, config) => { // If we have conferred to only use new flow charts this function should always return false // as in not signalling true for a legacy flowchart - if (config?.flowchart?.defaultRenderer === 'dagre-wrapper') { - return false; - } - if (config?.flowchart?.defaultRenderer === 'elk') { + if ( + config?.flowchart?.defaultRenderer === 'dagre-wrapper' || + config?.flowchart?.defaultRenderer === 'elk' + ) { return false; } return txt.match(/^\s*graph/) !== null; diff --git a/packages/mermaid/src/mermaid.ts b/packages/mermaid/src/mermaid.ts index 0b6807b3c..edcc5aef7 100644 --- a/packages/mermaid/src/mermaid.ts +++ b/packages/mermaid/src/mermaid.ts @@ -12,6 +12,7 @@ import type { ParseErrorFunction } from './Diagram'; import { isDetailedError } from './utils'; import type { DetailedError } from './utils'; import { ExternalDiagramDefinition } from './diagram-api/types'; +import { UnknownDiagramError } from './errors'; export type { MermaidConfig, @@ -20,6 +21,7 @@ export type { ParseErrorFunction, RenderResult, ParseOptions, + UnknownDiagramError, }; export interface RunOptions { diff --git a/packages/mermaid/src/mermaidAPI.spec.ts b/packages/mermaid/src/mermaidAPI.spec.ts index d87f035d0..1d54332fc 100644 --- a/packages/mermaid/src/mermaidAPI.spec.ts +++ b/packages/mermaid/src/mermaidAPI.spec.ts @@ -678,7 +678,7 @@ describe('mermaidAPI', () => { await expect( mermaidAPI.parse('this is not a mermaid diagram definition') ).rejects.toThrowErrorMatchingInlineSnapshot( - '"No diagram type detected for text: this is not a mermaid diagram definition"' + '"No diagram type detected matching given configuration for text: this is not a mermaid diagram definition"' ); }); it('returns false for invalid definition with silent option', async () => {