Merge pull request #4124 from mermaid-js/sidv/fixDetectorOrder

fix: Detector order

packages/mermaid/src/diagram-api/detectType.ts

@@ -46,9 +46,24 @@ 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}`
+  );
 };
 
+/**
+ * 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);

packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts

@@ -0,0 +1,82 @@
+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 }: { text: string; expected: string }) => {
+        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');
+    });
+  });
+});

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,26 @@ 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,
-    flowchart,
-    flowchartV2,
     flowchartElk,
+    flowchartV2,
+    flowchart,
     mindmap,
     timeline,
     git,
-    state,
     stateV2,
+    state,
     journey
   );
 };

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;

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"'
     );
   });
 });

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;

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;

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 {

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 () => {
@@ -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 () => {