fix: read block.padding and sanitizeText config dynamically instead of at import time

[OfirHaf]

What

Three diagrams were capturing config at module scope, so any config updates applied after import/initialize() were silently ignored:

• block/layout.ts — padding was computed once at the top of the file. The existing TODO comment (// TODO: This means the number we provide in diagram's config will never be used. Should fix.) called this out but it was never addressed.
• block/blockDB.ts — config snapshot was used for sanitizeText, so dompurifyConfig changes had no effect on block label sanitization.
• quadrant-chart/quadrantDb.ts — same stale-config pattern in textSanitizer.

How

layout.ts: Removed the module-level const padding. Instead, layout() reads getConfig()?.block?.padding ?? 8 and passes the value into setBlockSizes and layoutBlocks (and their recursive calls) via a new padding parameter with the same default.

blockDB.ts: Replaced common.sanitizeText(txt, config) with common.sanitizeText(txt, getConfig()) so every sanitization call reflects the live config.

quadrantDb.ts: Same — removed the module-scope config variable and call getConfig() directly inside textSanitizer.

Testing

Existing unit tests for layout.spec.ts, blockDB.spec.ts, and quadrantDb.spec.ts continue to pass. TypeScript compilation is clean.

Closes #7621
Closes #7622
Closes #7623

[changeset-bot]

🦋 Changeset detected

Latest commit: 66a28ab

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
mermaid	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	66a28ab
🔍 Latest deploy log	https://app.netlify.com/projects/mermaid-js/deploys/6a169bcec841df0008c9556c
😎 Deploy Preview	https://deploy-preview-7734--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[pkg-pr-new]

Open in StackBlitz

@mermaid-js/examples

npm i https://pkg.pr.new/@mermaid-js/examples@7734

mermaid

npm i https://pkg.pr.new/mermaid@7734

@mermaid-js/layout-elk

npm i https://pkg.pr.new/@mermaid-js/layout-elk@7734

@mermaid-js/layout-tidy-tree

npm i https://pkg.pr.new/@mermaid-js/layout-tidy-tree@7734

@mermaid-js/mermaid-zenuml

npm i https://pkg.pr.new/@mermaid-js/mermaid-zenuml@7734

@mermaid-js/parser

npm i https://pkg.pr.new/@mermaid-js/parser@7734

@mermaid-js/tiny

npm i https://pkg.pr.new/@mermaid-js/tiny@7734

commit: 66a28ab

[codecov]

Codecov Report

❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 3.25%. Comparing base (2695382) to head (66a28ab).

Files with missing lines	Patch %	Lines
packages/mermaid/src/diagrams/block/layout.ts	0.00%	13 Missing ⚠️
packages/mermaid/src/diagrams/block/blockDB.ts	0.00%	1 Missing ⚠️
.../mermaid/src/diagrams/quadrant-chart/quadrantDb.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #7734      +/-   ##
==========================================
- Coverage     3.25%   3.25%   -0.01%     
==========================================
  Files          599     600       +1     
  Lines        60946   60960      +14     
  Branches       917     917              
==========================================
  Hits          1986    1986              
- Misses       58960   58974      +14     

Flag	Coverage Δ
unit	3.25% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/mermaid/src/diagrams/block/blockDB.ts	0.00% <0.00%> (ø)
.../mermaid/src/diagrams/quadrant-chart/quadrantDb.ts	0.00% <0.00%> (ø)
packages/mermaid/src/diagrams/block/layout.ts	0.33% <0.00%> (-0.01%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[argos-ci]

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	✅ No changes detected	-	May 27, 2026, 7:28 AM

[knsv-bot]

[sisyphus-bot]

Thanks for picking this up @OfirHaf — closing out three stale-config bugs in one focused PR is a really nice batch. The diagnosis is on the money and the fix is exactly the right shape. A few observations below, only one of which is worth acting on before merge.

What's working well

• 🎉 [praise] This addresses the long-standing TODO at block/layout.ts:5 — that comment had been sitting there flagging the exact bug. Good to see it finally cleared.
• 🎉 [praise] Bundling #7621 / #7622 / #7623 together is the right call. They share a single root cause (module-scope config snapshots), so reviewing and landing them as one cohesive change is much cleaner than three separate PRs.
• 🎉 [praise] The threading of padding through setBlockSizes and layoutBlocks (including their recursive calls) preserves the existing default of 8 exactly, so behavior is unchanged for users who don't override the config — only those who do override it see the fix. That's the ideal shape for this kind of fix.
• 🎉 [praise] Security side-effect: the blockDB.ts and quadrantDb.ts changes mean dompurifyConfig overrides (e.g., FORBID_TAGS) now actually apply to those diagrams' label sanitization at runtime. That was a quiet sanitization bypass before, so this strengthens defense-in-depth meaningfully.

Things to address

• 🟡 [important] Missing regression tests. The PR description says "Existing unit tests continue to pass" — but those tests passed before this fix too, which means they don't actually exercise the bug. Without a regression test, it would be easy for someone to accidentally reintroduce a module-scope const config = getConfig() in a future refactor and not notice. All three modified files already have spec files (blockDB.spec.ts, layout.spec.ts, quadrantDb.spec.ts), so adding test cases is low-friction. Something like:

  • In layout.spec.ts: set a custom block.padding via setSiteConfig/initialize after the module is loaded, run layout() on a small block tree, and assert the spacing reflects the new padding.
  • In blockDB.spec.ts and quadrantDb.spec.ts: set dompurifyConfig: { FORBID_TAGS: ['b'] } (matching the issue repro snippets), feed in a <b>Bold</b> label, and assert the sanitized output reflects the runtime config.

  These would directly mirror the repro steps in the linked issues, which is the cleanest way to lock the fix in place.

Nits and suggestions

• 🟢 [nit] The default value 8 for padding is now duplicated in three spots: setBlockSizes signature, layoutBlocks signature, and the getConfig()?.block?.padding ?? 8 call inside layout(). Not a problem today, but a const DEFAULT_BLOCK_PADDING = 8 at the top of the file would make future updates one-place-only. Totally optional.
• 💡 [suggestion] Heads up that block/renderHelpers.ts:114 has a related-but-different pattern: padding ?? getConfig()?.block?.padding ?? 0. It already reads config dynamically (good), but its fallback is 0 while layout.ts falls back to 8. That inconsistency is pre-existing and out of scope for this PR — flagging only so it doesn't get lost. Could be worth a follow-up.

Self-check

Severity	Count
🔴 blocking	0
🟡 important	1
🟢 nit	1
💡 suggestion	1
🎉 praise	4

Verdict: COMMENT — the fix is correct and well-targeted; the only meaningful ask is regression tests so the bug can't quietly come back. Let's get this across the finish line. 🙌

[OfirHaf]

Added regression tests to all three files as requested:

• blockDB.spec.ts: new describe('block db runtime config') block with a test that spies on getConfig and verifies it's called during setHierarchy (i.e., at sanitization time, not at module load time)
• quadrantDb.spec.ts: same pattern — spies on getConfig, calls setXAxisLeftText, confirms the spy fires
• layout.spec.ts: new describe('layout runtime config') block that mocks getConfig with two different padding values (4 and 20), calls layout() for each, and asserts the returned widths differ — proving the padding is read fresh on each call

All 11 tests pass. Let me know if you'd prefer a different style or more granular coverage.

[pbrolin47]

Hi @OfirHaf,
Thanks for this PR and addressing the comments from the review. Can you please have a look at the errors in the CI, and we can proceed with this PR

[OfirHaf]

Thanks for the heads up! The CI was failing because I used 'rect' as the block type in the test mocks, which isn't actually part of the BlockType union. Fixed it to 'square' — just pushed the correction. CI should be green now.

[OfirHaf]

Apologies, missed two more 'rect' literals in the child block definitions inside layout.spec.ts. Fixed those to 'square' as well and pushed again — should be clean now.

[pbrolin47]

Hi @OfirHaf,
Thanks for the last fix in 8688086.
Can you update the branch from develop please and hopefully the PR is good to go

[OfirHaf]

Done, rebased onto develop just now. Thanks for guiding this through!

[ashishjain0512]

Thanks for picking this up @OfirHaf — and especially for going after the dangling TODO at layout.ts:5. This is a small change with surprisingly broad reach (any user who reconfigured block.padding or dompurifyConfig after the initial import was being silently ignored), and it's good to see it land.

What's working well

🎉 Cohesive root-cause fix. All three diagrams suffered from the same module-load-time snapshot anti-pattern; addressing them together rather than one-off is exactly the right call.

🎉 layout.spec.ts:21-45 is the kind of test I want to see. It mocks getConfig to return different padding values, calls layout() twice, and asserts the output width changes (212 vs 260 — math checks out). That's a behavior assertion, not a call-count assertion, and it catches the bug at its real consequence.

🎉 Security-positive side-effect. Inlining getConfig() into the sanitizeText helpers means any post-initialize() dompurifyConfig or securityLevel override now actually applies. Previously those overrides were silently ignored — a real (if quiet) hardening for downstream embedders.

🎉 Changeset is present, correctly typed patch, and the prefix is fix:. Resolves #7621 — the linked issue is captured in the description; would be lovely to also add Resolves #7621 at the bottom of the PR body so GitHub auto-closes the issue on squash-merge.

Things to consider (non-blocking)

🟢 [nit] setBlockSizes and layoutBlocks have padding = 8 as their function-level default (layout.ts:75, 209). Since the only caller, layout() at line 357, always reads the value from config and passes it down, those defaults are now dead code. Either drop them (and require padding as a positional arg, which removes the chance of an internal caller forgetting it) or add a one-line comment that they're a safety net. Tiny — pick whichever you prefer.

🟢 [nit] The other two specs could be a touch stronger. blockDB.spec.ts:38-47 and quadrantDb.spec.ts:59-68 both assert spy.toHaveBeenCalled(), which would also pass for a "fix" that called getConfig() once and cached it across calls. Following layout.spec.ts's pattern — set two different mocked configs, invoke the sanitizer twice, assert the output differs — would lock in the regression more tightly. Not blocking; the current tests do distinguish "fixed" from the original module-load-time bug.

💡 [suggestion] Other diagrams. A quick grep for ^const config = getConfig\(\) across packages/mermaid/src/diagrams/ only surfaces these two (block, quadrant-chart), so there's nothing else to chase with this exact shape. But broader patterns like getConfig()?.X?.Y evaluated at module scope might exist elsewhere — happy follow-up if you (or anyone) wants to sweep, definitely out of scope for this PR.

Verification I ran

• Read all six source/test diffs end-to-end (the changeset too).
• Ran a focused XSS audit on the changed code paths: common.sanitizeText still always invokes DOMPurify regardless of config shape; padding is only used in arithmetic and log.debug calls (no DOM-sink reach); no memoization or new timing windows reintroduce the captured-snapshot bug.
• Confirmed via grep that no other diagram in the tree shares this exact const config = getConfig() module-scope pattern.

Severity tally: 🔴 0 / 🟡 0 / 🟢 2 / 💡 1 / 🎉 4

Approving — let's get this across the finish line. Thanks again!

[OfirHaf]

Hey @pbrolin47 — just checking in. The branch is rebased on develop and all CI is green. Is there anything else needed from my side before this can be merged?

[pbrolin47]

Hi @OfirHaf, I think the PR is good to go, but I called out for some help of other maintainers to get it merged

[OfirHaf]

Hi @pbrolin47 — just following up again. The branch is up to date with develop and CI is green. Would love to get this across the finish line if there's nothing else needed from my side!

[OfirHaf]

Hey @ashishjain0512 — saw you just merged develop into the branch, really appreciate you keeping it in sync! Just wanted to check in — is there anything still needed from my side, or is this close to being merged? Happy to help with whatever's left 🙂

[mermaid-bot]

@OfirHaf, Thank you for the contribution!
You are now eligible for a year of Premium account on MermaidChart.
Sign up with your GitHub account to activate.