From 1ac219282bd4360059516a17cf43a1924f98add8 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 21 Feb 2023 23:00:03 +0530 Subject: [PATCH 1/4] fix: Detector order --- .../diagram-api/diagram-orchestration.spec.ts | 42 +++++++++++++++++++ .../src/diagram-api/diagram-orchestration.ts | 11 +++-- packages/mermaid/src/mermaidAPI.spec.ts | 2 +- 3 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts diff --git a/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts b/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts new file mode 100644 index 000000000..4c7fc142d --- /dev/null +++ b/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts @@ -0,0 +1,42 @@ +import { it, describe, expect } from 'vitest'; +import { detectType } from './detectType'; +import { addDiagrams } from './diagram-orchestration'; + +describe('diagram-orchestration', () => { + it('should register diagrams', () => { + expect(() => detectType('graph TD; A-->B')).toThrow(); + addDiagrams(); + expect(detectType('graph TD; A-->B')).toBe('flowchart'); + }); + + describe('proper diagram types should be detetced', () => { + beforeAll(() => { + addDiagrams(); + }); + + it.each([ + { text: 'graph TD;', expected: 'flowchart' }, + { text: 'flowchart TD;', expected: 'flowchart-v2' }, + { text: 'flowchart-v2 TD;', expected: 'flowchart-v2' }, + { text: 'flowchart-elk TD;', expected: 'flowchart-elk' }, + { text: 'error', expected: 'error' }, + { text: 'C4Context;', expected: 'c4' }, + { text: 'classDiagram', expected: 'class' }, + { text: 'classDiagram-v2', expected: 'classDiagram' }, + { text: 'erDiagram', expected: 'er' }, + { text: 'journey', expected: 'journey' }, + { text: 'gantt', expected: 'gantt' }, + { text: 'pie', expected: 'pie' }, + { text: 'requirementDiagram', expected: 'requirement' }, + { text: 'info', expected: 'info' }, + { text: 'sequenceDiagram', expected: 'sequence' }, + { text: 'mindmap', expected: 'mindmap' }, + { text: 'timeline', expected: 'timeline' }, + { text: 'gitGraph', expected: 'gitGraph' }, + { text: 'stateDiagram', expected: 'state' }, + { text: 'stateDiagram-v2', expected: 'stateDiagram' }, + ])('should $text be detected as $expected', ({ text, expected }) => { + expect(detectType(text)).toBe(expected); + }); + }); +}); diff --git a/packages/mermaid/src/diagram-api/diagram-orchestration.ts b/packages/mermaid/src/diagram-api/diagram-orchestration.ts index d06ce846a..da4c4d081 100644 --- a/packages/mermaid/src/diagram-api/diagram-orchestration.ts +++ b/packages/mermaid/src/diagram-api/diagram-orchestration.ts @@ -45,7 +45,7 @@ export const addDiagrams = () => { throw new Error( 'Diagrams beginning with --- are not valid. ' + 'If you were trying to use a YAML front-matter, please ensure that ' + - "you've correctly opened and closed the YAML front-matter with unindented `---` blocks" + "you've correctly opened and closed the YAML front-matter with un-indented `---` blocks" ); }, }, @@ -55,25 +55,28 @@ export const addDiagrams = () => { return text.toLowerCase().trimStart().startsWith('---'); } ); + // Ordering of detectors is important. The first one to return true will be used. registerLazyLoadedDiagrams( error, c4, - classDiagram, classDiagramV2, + classDiagram, er, gantt, info, pie, 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, - flowchartElk, mindmap, timeline, git, - state, stateV2, + state, journey ); }; diff --git a/packages/mermaid/src/mermaidAPI.spec.ts b/packages/mermaid/src/mermaidAPI.spec.ts index cf858b58e..d87f035d0 100644 --- a/packages/mermaid/src/mermaidAPI.spec.ts +++ b/packages/mermaid/src/mermaidAPI.spec.ts @@ -666,7 +666,7 @@ describe('mermaidAPI', () => { ).rejects.toThrow( 'Diagrams beginning with --- are not valid. ' + 'If you were trying to use a YAML front-matter, please ensure that ' + - "you've correctly opened and closed the YAML front-matter with unindented `---` blocks" + "you've correctly opened and closed the YAML front-matter with un-indented `---` blocks" ); }); it('does not throw for a valid definition', async () => { From eca41633635b0460442452ce89f5cfdfae4cfa7b Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 21 Feb 2023 23:24:11 +0530 Subject: [PATCH 2/4] Fix types --- .../src/diagram-api/diagram-orchestration.spec.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts b/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts index 4c7fc142d..f08f32391 100644 --- a/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts +++ b/packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts @@ -35,8 +35,11 @@ describe('diagram-orchestration', () => { { text: 'gitGraph', expected: 'gitGraph' }, { text: 'stateDiagram', expected: 'state' }, { text: 'stateDiagram-v2', expected: 'stateDiagram' }, - ])('should $text be detected as $expected', ({ text, expected }) => { - expect(detectType(text)).toBe(expected); - }); + ])( + 'should $text be detected as $expected', + ({ text, expected }: { text: string; expected: string }) => { + expect(detectType(text)).toBe(expected); + } + ); }); }); From 4bf5c9f3d844905bf0664c8116bb41dd5fc1ffd7 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Fri, 24 Feb 2023 17:27:24 +0530 Subject: [PATCH 3/4] 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 () => { From 7b4ce7c6ea41241f7250018a1accb27fb3b8ce2c Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Fri, 24 Feb 2023 17:32:39 +0530 Subject: [PATCH 4/4] chore: Add tsdoc for registerLazyLoadedDiagrams Co-authored-by: Alois Klink --- packages/mermaid/src/diagram-api/detectType.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/mermaid/src/diagram-api/detectType.ts b/packages/mermaid/src/diagram-api/detectType.ts index 3d55dc78f..6580cb784 100644 --- a/packages/mermaid/src/diagram-api/detectType.ts +++ b/packages/mermaid/src/diagram-api/detectType.ts @@ -51,6 +51,19 @@ export const detectType = function (text: string, config?: MermaidConfig): strin ); }; +/** + * Registers lazy-loaded diagrams to Mermaid. + * + * The diagram function is loaded asynchronously, so that diagrams are only loaded + * if the diagram is detected. + * + * @remarks + * Please note that the order of diagram detectors is important. + * The first detector to return `true` is the diagram that will be loaded + * and used, so put more specific detectors at the beginning! + * + * @param diagrams - Diagrams to lazy load, and their detectors, in order of importance. + */ export const registerLazyLoadedDiagrams = (...diagrams: ExternalDiagramDefinition[]) => { for (const { id, detector, loader } of diagrams) { addDetector(id, detector, loader);