From b5f3cdc0b012c9872d2ff143ea05b62a79a0c845 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Sat, 18 Nov 2023 21:10:58 +0530 Subject: [PATCH] feat: 5043 Add priority support for registered diagrams Allows external diagrams to override internal diagrams, if necessary. This will help move ELK to a different package, without completely breaking rendering, by falling back to dagre, and supporting ELK if it's registered as an external diagram. --- packages/mermaid-flowchart-elk/package.json | 12 +++++++ packages/mermaid/src/Diagram.ts | 6 ++-- .../mermaid/src/diagram-api/detectType.ts | 34 +++++++++++++------ .../src/diagram-api/diagram-orchestration.ts | 3 +- .../src/diagram-api/diagramAPI.spec.ts | 1 + .../mermaid/src/diagram-api/diagramAPI.ts | 16 +++++---- .../mermaid/src/diagram-api/loadDiagram.ts | 4 +-- packages/mermaid/src/diagram-api/types.ts | 10 ++++++ packages/mermaid/src/diagram.spec.ts | 32 +++++++++++++++++ 9 files changed, 96 insertions(+), 22 deletions(-) create mode 100644 packages/mermaid-flowchart-elk/package.json diff --git a/packages/mermaid-flowchart-elk/package.json b/packages/mermaid-flowchart-elk/package.json new file mode 100644 index 0000000000..8b98016edd --- /dev/null +++ b/packages/mermaid-flowchart-elk/package.json @@ -0,0 +1,12 @@ +{ + "name": "@mermaid-js/flowchart-elk", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": "", + "license": "MIT" +} diff --git a/packages/mermaid/src/Diagram.ts b/packages/mermaid/src/Diagram.ts index b56697e9de..c162d9c0a6 100644 --- a/packages/mermaid/src/Diagram.ts +++ b/packages/mermaid/src/Diagram.ts @@ -1,7 +1,7 @@ import * as configApi from './config.js'; import { log } from './logger.js'; import { getDiagram, registerDiagram } from './diagram-api/diagramAPI.js'; -import { detectType, getDiagramLoader } from './diagram-api/detectType.js'; +import { detectType, getDiagramLoaderAndPriority } from './diagram-api/detectType.js'; import { UnknownDiagramError } from './errors.js'; import { encodeEntities } from './utils.js'; @@ -92,14 +92,14 @@ export const getDiagramFromText = async ( // Trying to find the diagram getDiagram(type); } catch (error) { - const loader = getDiagramLoader(type); + const { loader, priority } = getDiagramLoaderAndPriority(type); if (!loader) { throw new UnknownDiagramError(`Diagram ${type} not found.`); } // Diagram not available, loading it. // new diagram will try getDiagram again and if fails then it is a valid throw const { id, diagram } = await loader(); - registerDiagram(id, diagram); + registerDiagram(id, diagram, priority); } return new Diagram(text, metadata); }; diff --git a/packages/mermaid/src/diagram-api/detectType.ts b/packages/mermaid/src/diagram-api/detectType.ts index ba78dbe55e..2b618fa53f 100644 --- a/packages/mermaid/src/diagram-api/detectType.ts +++ b/packages/mermaid/src/diagram-api/detectType.ts @@ -61,23 +61,37 @@ export const detectType = function (text: string, config?: MermaidConfig): strin * The first detector to return `true` is the diagram that will be loaded * and used, so put more specific detectors at the beginning! * + * If two diagrams are registered with the same id, + * the one with higher `priority` property will be used. + * * @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); + for (const { id, detector, priority, loader } of diagrams) { + addDetector(id, detector, priority ?? 0, loader); } }; -export const addDetector = (key: string, detector: DiagramDetector, loader?: DiagramLoader) => { - if (detectors[key]) { - log.error(`Detector with key ${key} already exists`); - } else { - detectors[key] = { detector, loader }; +export const addDetector = ( + key: string, + detector: DiagramDetector, + priority: number, + loader?: DiagramLoader +) => { + if (detectors[key] && priority <= detectors[key].priority) { + log.error( + `Detector with key ${key} already exists with priority ${detectors[key].priority}. Cannot add new detector with priority ${priority}` + ); + return; } - log.debug(`Detector with key ${key} added${loader ? ' with loader' : ''}`); + + detectors[key] = { detector, loader, priority }; + log.debug( + `Detector with key ${key} added with priority ${priority} ${loader ? 'and loader' : ''}` + ); }; -export const getDiagramLoader = (key: string) => { - return detectors[key].loader; +export const getDiagramLoaderAndPriority = (key: string) => { + const { loader, priority } = detectors[key]; + return { loader, priority }; }; diff --git a/packages/mermaid/src/diagram-api/diagram-orchestration.ts b/packages/mermaid/src/diagram-api/diagram-orchestration.ts index d6b9c00d8f..4beced7d55 100644 --- a/packages/mermaid/src/diagram-api/diagram-orchestration.ts +++ b/packages/mermaid/src/diagram-api/diagram-orchestration.ts @@ -31,7 +31,7 @@ export const addDiagrams = () => { // This is added here to avoid race-conditions. // We could optimize the loading logic somehow. hasLoadedDiagrams = true; - registerDiagram('error', errorDiagram, (text) => { + registerDiagram('error', errorDiagram, 0, (text) => { return text.toLowerCase().trim() === 'error'; }); registerDiagram( @@ -60,6 +60,7 @@ export const addDiagrams = () => { }, init: () => null, // no op }, + 0, (text) => { return text.toLowerCase().trimStart().startsWith('---'); } diff --git a/packages/mermaid/src/diagram-api/diagramAPI.spec.ts b/packages/mermaid/src/diagram-api/diagramAPI.spec.ts index 3b6bce683f..8bf0c812c3 100644 --- a/packages/mermaid/src/diagram-api/diagramAPI.spec.ts +++ b/packages/mermaid/src/diagram-api/diagramAPI.spec.ts @@ -47,6 +47,7 @@ describe('DiagramAPI', () => { }, styles: {}, }, + 0, detector ); expect(getDiagram('loki')).not.toBeNull(); diff --git a/packages/mermaid/src/diagram-api/diagramAPI.ts b/packages/mermaid/src/diagram-api/diagramAPI.ts index 7ca9d58043..ba718c561d 100644 --- a/packages/mermaid/src/diagram-api/diagramAPI.ts +++ b/packages/mermaid/src/diagram-api/diagramAPI.ts @@ -29,7 +29,7 @@ export const getCommonDb = () => { return _commonDb; }; -const diagrams: Record = {}; +const diagrams: Record = {}; export interface Detectors { [key: string]: DiagramDetector; } @@ -37,7 +37,8 @@ export interface Detectors { /** * Registers the given diagram with Mermaid. * - * Can be used for third-party custom diagrams. + * To be used internally by Mermaid. + * Use `mermaid.registerExternalDiagrams` to register external diagrams. * * @param id - A unique ID for the given diagram. * @param diagram - The diagram definition. @@ -46,14 +47,17 @@ export interface Detectors { export const registerDiagram = ( id: string, diagram: DiagramDefinition, + priority: number, detector?: DiagramDetector ) => { - if (diagrams[id]) { - throw new Error(`Diagram ${id} already registered.`); + if (diagrams[id] && priority <= diagrams[id].priority) { + throw new Error( + `Diagram ${id} already registered with priority ${diagrams[id].priority}. Cannot add new diagram with priority ${priority}` + ); } - diagrams[id] = diagram; + diagrams[id] = { ...diagram, priority }; if (detector) { - addDetector(id, detector); + addDetector(id, detector, priority); } addStylesForDiagram(id, diagram.styles); diff --git a/packages/mermaid/src/diagram-api/loadDiagram.ts b/packages/mermaid/src/diagram-api/loadDiagram.ts index c1b445bf64..eda192e611 100644 --- a/packages/mermaid/src/diagram-api/loadDiagram.ts +++ b/packages/mermaid/src/diagram-api/loadDiagram.ts @@ -6,7 +6,7 @@ export const loadRegisteredDiagrams = async () => { log.debug(`Loading registered diagrams`); // Load all lazy loaded diagrams in parallel const results = await Promise.allSettled( - Object.entries(detectors).map(async ([key, { detector, loader }]) => { + Object.entries(detectors).map(async ([key, { detector, loader, priority }]) => { if (loader) { try { getDiagram(key); @@ -14,7 +14,7 @@ export const loadRegisteredDiagrams = async () => { try { // Register diagram if it is not already registered const { diagram, id } = await loader(); - registerDiagram(id, diagram, detector); + registerDiagram(id, diagram, priority, detector); } catch (err) { // Remove failed diagram from detectors log.error(`Failed to load external diagram with key ${key}. Removing from detectors.`); diff --git a/packages/mermaid/src/diagram-api/types.ts b/packages/mermaid/src/diagram-api/types.ts index d4f34de702..c9b7735659 100644 --- a/packages/mermaid/src/diagram-api/types.ts +++ b/packages/mermaid/src/diagram-api/types.ts @@ -76,13 +76,23 @@ export interface DiagramDefinition { export interface DetectorRecord { detector: DiagramDetector; + priority: number; loader?: DiagramLoader; } +/** + * External diagrams, which are not bundled with mermaid should expose the following to be registered using the `mermaid.registerExternalDiagrams` function. + * + * @param id - An ID for the given diagram. If two diagrams are registered with the same ID, the one with the higher priority will be used. + * @param detector - Function that returns `true` if a given mermaid text satisfies with this diagram definition. + * @param loader - Function that returns a promise of the diagram definition. + * @param priority - The priority of the diagram. Optional, defaults to 0. + */ export interface ExternalDiagramDefinition { id: string; detector: DiagramDetector; loader: DiagramLoader; + priority?: number; } export type DiagramDetector = (text: string, config?: MermaidConfig) => boolean; diff --git a/packages/mermaid/src/diagram.spec.ts b/packages/mermaid/src/diagram.spec.ts index d116399ac1..13e5fb0a55 100644 --- a/packages/mermaid/src/diagram.spec.ts +++ b/packages/mermaid/src/diagram.spec.ts @@ -21,6 +21,7 @@ describe('diagram detection', () => { addDetector( 'loki', (str) => str.startsWith('loki'), + 0, () => Promise.resolve({ id: 'loki', @@ -45,6 +46,37 @@ describe('diagram detection', () => { expect(diagram.type).toBe('loki'); }); + test('should allow external diagrams to override internal ones with same ID', async () => { + addDetector( + 'flowchart-elk', + (str) => str.startsWith('flowchart-elk'), + 1, + () => + Promise.resolve({ + id: 'flowchart-elk', + diagram: { + db: { + getDiagramTitle: () => 'overridden', + }, + parser: { + parse: () => { + // no-op + }, + }, + renderer: { + draw: () => { + // no-op + }, + }, + styles: {}, + }, + }) + ); + const diagram = (await getDiagramFromText('flowchart-elk TD; A-->B')) as Diagram; + expect(diagram).toBeInstanceOf(Diagram); + expect(diagram.db.getDiagramTitle?.()).toBe('overridden'); + }); + test('should throw the right error for incorrect diagram', async () => { await expect(getDiagramFromText('graph TD; A-->')).rejects.toThrowErrorMatchingInlineSnapshot(` "Parse error on line 2: