Repository navigation
fix(theme-mermaid): serialize initialize and render calls to prevent empty SVGs (#12757) #12923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| /** | ||
| * Copyright (c) Facebook, Inc. and its affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import {describe, expect, it, vi, beforeEach} from 'vitest'; | ||
| import {renderMermaid, resetRenderQueueForTest} from '../client/index'; | ||
| import * as loadMermaidModule from '../client/loadMermaid'; | ||
| import type {MermaidConfig, RenderResult} from 'mermaid'; | ||
|
|
||
| describe('renderMermaid', () => { | ||
| beforeEach(() => { | ||
| resetRenderQueueForTest(); | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('serialize concurrent renders so initialize and render never overlap', async () => { | ||
| const executionTrace: string[] = []; | ||
| let activeRenders = 0; | ||
| let maxConcurrentRenders = 0; | ||
|
|
||
| const mockMermaid = { | ||
| initialize: vi.fn((config: MermaidConfig) => { | ||
| executionTrace.push(`initialize:${config.theme}`); | ||
| }), | ||
| render: vi.fn(async (id: string, text: string): Promise<RenderResult> => { | ||
| activeRenders++; | ||
| if (activeRenders > maxConcurrentRenders) { | ||
| maxConcurrentRenders = activeRenders; | ||
| } | ||
| executionTrace.push(`render:start:${id}`); | ||
|
|
||
| // Simulate async layout/rendering time | ||
| await new Promise((resolve) => setTimeout(resolve, 20)); | ||
|
|
||
| executionTrace.push(`render:end:${id}`); | ||
| activeRenders--; | ||
| return { | ||
| svg: `<svg id="${id}">${text}</svg>`, | ||
| diagramType: 'flowchart', | ||
| }; | ||
| }), | ||
| }; | ||
|
|
||
| vi.spyOn(loadMermaidModule, 'loadMermaid').mockResolvedValue( | ||
| mockMermaid as unknown as typeof loadMermaidModule.loadMermaid extends () => Promise< | ||
| infer M | ||
| > | ||
| ? M | ||
| : never, | ||
| ); | ||
|
|
||
| // Call 3 renders concurrently without awaiting | ||
| const p1 = renderMermaid({ | ||
| id: 'd1', | ||
| text: 'graph TD; A-->B', | ||
| config: {theme: 'default'}, | ||
| }); | ||
| const p2 = renderMermaid({ | ||
| id: 'd2', | ||
| text: 'graph TD; C-->D', | ||
| config: {theme: 'forest'}, | ||
| }); | ||
| const p3 = renderMermaid({ | ||
| id: 'd3', | ||
| text: 'graph TD; E-->F', | ||
| config: {theme: 'dark'}, | ||
| }); | ||
|
|
||
| const [r1, r2, r3] = await Promise.all([p1, p2, p3]); | ||
|
|
||
| expect(r1.svg).toBe('<svg id="d1">graph TD; A-->B</svg>'); | ||
| expect(r2.svg).toBe('<svg id="d2">graph TD; C-->D</svg>'); | ||
| expect(r3.svg).toBe('<svg id="d3">graph TD; E-->F</svg>'); | ||
|
|
||
| // Must never have more than 1 render in flight simultaneously | ||
| expect(maxConcurrentRenders).toBe(1); | ||
|
|
||
| // Strict sequential execution: | ||
| // initialize -> render start -> render end -> initialize next... | ||
| expect(executionTrace).toEqual([ | ||
| 'initialize:default', | ||
| 'render:start:d1', | ||
| 'render:end:d1', | ||
| 'initialize:forest', | ||
| 'render:start:d2', | ||
| 'render:end:d2', | ||
| 'initialize:dark', | ||
| 'render:start:d3', | ||
| 'render:end:d3', | ||
| ]); | ||
| }); | ||
|
|
||
| it('continue executing queued renders even if an earlier render throws', async () => { | ||
| const mockMermaid = { | ||
| initialize: vi.fn(), | ||
| render: vi.fn(async (id: string, text: string): Promise<RenderResult> => { | ||
| if (id === 'failing-diagram') { | ||
| throw new Error('Syntax error in diagram'); | ||
| } | ||
| return { | ||
| svg: `<svg id="${id}">${text}</svg>`, | ||
| diagramType: 'flowchart', | ||
| }; | ||
| }), | ||
| }; | ||
|
|
||
| vi.spyOn(loadMermaidModule, 'loadMermaid').mockResolvedValue( | ||
| mockMermaid as unknown as typeof loadMermaidModule.loadMermaid extends () => Promise< | ||
| infer M | ||
| > | ||
| ? M | ||
| : never, | ||
| ); | ||
|
|
||
| const p1 = renderMermaid({ | ||
| id: 'failing-diagram', | ||
| text: 'syntax error', | ||
| config: {theme: 'default'}, | ||
| }); | ||
| const p2 = renderMermaid({ | ||
| id: 'succeeding-diagram', | ||
| text: 'graph TD; A-->B', | ||
| config: {theme: 'default'}, | ||
| }); | ||
|
|
||
| await expect(p1).rejects.toThrow('Syntax error in diagram'); | ||
| const r2 = await p2; | ||
| expect(r2.svg).toBe('<svg id="succeeding-diagram">graph TD; A-->B</svg>'); | ||
| }); | ||
|
|
||
| it('clean up error container from document on error if present', async () => { | ||
| const removeFn = vi.fn(); | ||
| const getElementByIdSpy = vi.fn().mockReturnValue({remove: removeFn}); | ||
| vi.stubGlobal('document', {getElementById: getElementByIdSpy}); | ||
|
|
||
| const mockMermaid = { | ||
| initialize: vi.fn(), | ||
| render: vi.fn(async () => { | ||
| throw new Error('Render failed'); | ||
| }), | ||
| }; | ||
|
|
||
| vi.spyOn(loadMermaidModule, 'loadMermaid').mockResolvedValue( | ||
| mockMermaid as unknown as typeof loadMermaidModule.loadMermaid extends () => Promise< | ||
| infer M | ||
| > | ||
| ? M | ||
| : never, | ||
| ); | ||
|
|
||
| // Verify handling of React useId() IDs containing colons (e.g. :r0:-0) | ||
| await expect( | ||
| renderMermaid({ | ||
| id: ':r0:-0', | ||
| text: 'invalid', | ||
| config: {theme: 'default'}, | ||
| }), | ||
| ).rejects.toThrow('Render failed'); | ||
|
|
||
| expect(getElementByIdSpy).toHaveBeenCalledWith('d:r0:-0'); | ||
| expect(removeFn).toHaveBeenCalled(); | ||
|
|
||
| vi.unstubAllGlobals(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,13 @@ export function useMermaidConfig(): MermaidConfig { | |
| ); | ||
| } | ||
|
|
||
| async function renderMermaid({ | ||
| let renderQueue: Promise<unknown> = Promise.resolve(); | ||
|
|
||
| export function resetRenderQueueForTest(): void { | ||
| renderQueue = Promise.resolve(); | ||
| } | ||
|
|
||
| export async function renderMermaid({ | ||
| id, | ||
| text, | ||
| config, | ||
|
|
@@ -41,36 +47,49 @@ async function renderMermaid({ | |
| text: string; | ||
| config: MermaidConfig; | ||
| }): Promise<RenderResult> { | ||
| const mermaid = await loadMermaid(); | ||
|
|
||
| /* | ||
| Mermaid API is really weird :s | ||
| It is a big mutable singleton with multiple config levels | ||
| Note: most recent API type definitions are missing | ||
|
|
||
| There are 2 kind of configs: | ||
|
|
||
| - siteConfig: some kind of global/protected shared config | ||
| you can only set with "initialize" | ||
|
|
||
| - config/currentConfig | ||
| the config the renderer will use | ||
| it is reset to siteConfig before each render | ||
| but it can be altered by the mermaid txt content itself through directives | ||
|
|
||
| To use a new mermaid config (on colorMode change for example) we should | ||
| update siteConfig, and it can only be done with initialize() | ||
| */ | ||
| mermaid.initialize(config); | ||
|
|
||
| try { | ||
| return await mermaid.render(id, text); | ||
| } catch (e) { | ||
| // Because Mermaid add a weird SVG/Message to the DOM on error | ||
| // https://github.com/mermaid-js/mermaid/issues/3205#issuecomment-1719620183 | ||
| document.querySelector(`#d${id}`)?.remove(); | ||
| throw e; | ||
| } | ||
| const run = async (): Promise<RenderResult> => { | ||
| const mermaid = await loadMermaid(); | ||
|
|
||
| /* | ||
| Mermaid API is really weird :s | ||
| It is a big mutable singleton with multiple config levels | ||
| Note: most recent API type definitions are missing | ||
|
|
||
| There are 2 kind of configs: | ||
|
|
||
| - siteConfig: some kind of global/protected shared config | ||
| you can only set with "initialize" | ||
|
|
||
| - config/currentConfig | ||
| the config the renderer will use | ||
| it is reset to siteConfig before each render | ||
| but it can be altered by the mermaid txt content itself through directives | ||
|
|
||
| To use a new mermaid config (on colorMode change for example) we should | ||
| update siteConfig, and it can only be done with initialize() | ||
| */ | ||
| mermaid.initialize(config); | ||
|
|
||
| try { | ||
| return await mermaid.render(id, text); | ||
| } catch (e) { | ||
| // Because Mermaid add a weird SVG/Message to the DOM on error | ||
| // https://github.com/mermaid-js/mermaid/issues/3205#issuecomment-1719620183 | ||
| // Use getElementById instead of querySelector since IDs generated by useId() | ||
| // contain colons (e.g. :r0:-0) which break CSS selector parsing. | ||
| if (typeof document !== 'undefined') { | ||
| document.getElementById(`d${id}`)?.remove(); | ||
|
Comment on lines
+78
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| throw e; | ||
| } | ||
| }; | ||
|
|
||
| // Multiple diagrams on a page mount concurrently. Mermaid is a mutable | ||
| // singleton, so concurrent initialize() + render() calls can corrupt | ||
| // internal state and produce empty SVGs. We serialize renders in a queue. | ||
| const result = renderQueue.then(run, run); | ||
| renderQueue = result.catch(() => {}); | ||
| return result; | ||
| } | ||
|
|
||
| export function useMermaidRenderResult({ | ||
|
|
@@ -92,20 +111,32 @@ export function useMermaidRenderResult({ | |
| const config = providedConfig ?? defaultMermaidConfig; | ||
|
|
||
| useEffect(() => { | ||
| let isCancelled = false; | ||
|
|
||
| // Mermaid removes existing DOM elements with the requested ID. Reusing an | ||
| // ID can delete the displayed SVG, which React won't restore if the new | ||
| // SVG markup is identical. Give every render its own ID instead. | ||
| renderMermaid({id: `${id}-${renderCount.current++}`, text, config}) | ||
| // TODO maybe try to use Suspense here and throw the promise? | ||
| // See also https://github.com/pmndrs/suspend-react | ||
| .then(setResult) | ||
| .then((res) => { | ||
| if (!isCancelled) { | ||
| setResult(res); | ||
| } | ||
| }) | ||
| .catch((e) => { | ||
| // Funky way to trigger parent React error boundary | ||
| // See https://x.com/sebastienlorber/status/1628340871899893768 | ||
| setResult(() => { | ||
| throw e; | ||
| }); | ||
| if (!isCancelled) { | ||
| // Funky way to trigger parent React error boundary | ||
| // See https://x.com/sebastienlorber/status/1628340871899893768 | ||
| setResult(() => { | ||
| throw e; | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| return () => { | ||
| isCancelled = true; | ||
| }; | ||
| }, [id, text, config]); | ||
|
|
||
| return result; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.