Feature/2910 treeview root config#7698
Conversation
🦋 Changeset detectedLatest commit: 3719535 The changes in this PR will be included in the next version bump. 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 |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7698 +/- ##
==========================================
- Coverage 3.33% 3.33% -0.01%
==========================================
Files 542 542
Lines 56881 56897 +16
Branches 839 839
==========================================
Hits 1899 1899
- Misses 54982 54998 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
knsv-bot
left a comment
knsv-bot
left a comment
There was a problem hiding this comment.
Thanks for tackling #7630 — the three-mode design (default /, custom string, hidden) is the right shape for this feature, and the changeset + test scenarios cover the cases cleanly. 🎉
That said, there are a few things to fix before this can land — the failing Argos tests trace to two of them.
Things to address
🔴 [blocking] Schema default is the string "null", not the null value
packages/mermaid/src/schemas/config.schema.yaml:2429:
root:
...
type: [string, boolean, 'null']
default: 'null'In YAML, 'null' (quoted) is the literal string "null" — not the null value. With this default in place, when no user override is set, config.root becomes the string "null". Then in db.ts updateRootName, the typeof config.root === 'string' branch fires and renders the root label as the word "null" — which is exactly what the Argos test diff is showing.
Fix: drop the default: line entirely so root is undefined (which the runtime's else branch already handles correctly by emitting /). Repo precedent agrees — there's no default: null anywhere else in config.schema.yaml; absent defaults are conveyed by omission, not by literal null.
After fixing, regenerate config.type.ts with pnpm run --filter mermaid types:build-config.
🔴 [blocking] Hide-root renderer logic skips all descendants
packages/mermaid/src/diagrams/treeView/renderer.ts:82-88:
if (depth === 0 && (config.root === false || node.name === '')) {
node.children.forEach((child) => {
processNode(child, depth); // depth stays at 0
});
return;
}When config.root === false, the recursion keeps depth at 0, so the very same condition matches on every recursive call — the function returns without ever calling drawNode. Net effect: when root: false, nothing renders. The root: false Argos snapshot is almost certainly failing for this reason too.
A clean fix is to handle the root-skip once at the entrypoint and let normal recursion proceed for children:
const drawTree = (...) => {
const rootNode = db.getRoot();
const hideRoot = config.root === false || rootNode.name === '';
if (hideRoot) {
rootNode.children.forEach((c) => processNode(c, 0));
} else {
processNode(rootNode, 0);
}
};
const processNode = (node, depth = 0) => {
drawNode(elem, node, config, depth);
node.children.forEach((c) => processNode(c, depth + 1));
};Worth adding a unit/snapshot assertion that confirms the children render when root: false, not just that the root itself is absent — the current bug would silently pass a "root not present" check.
🔴 [blocking] commonClear() was dropped
packages/mermaid/src/diagrams/treeView/db.ts:
The previous clear function called both state.reset() and commonClear(). The new inline clear: () => { state.reset(); } drops commonClear() — meaning accessibility title, accessibility description, and diagram title will now leak between consecutive renders of treeView diagrams. This is a regression unrelated to the feature.
Restore the commonClear() call.
🟡 [important] Side effects in getConfig()
db.ts:53 — getConfig() now calls updateRootName(config), which mutates state.records.stack[0].name. Getters that mutate state are surprising and make ordering matter (callers must invoke getConfig before reading the root). Two cleaner options:
- Move the root-name resolution into the renderer (it already has
configanddb.getRoot()in scope), or - Compute it on read inside
getRoot()instead of caching it on the node.
🟡 [important] Branch / issue number mismatch
The branch name is feature/2910_treeview-root-config, but #2910 is unrelated (a docs restructuring issue). The PR description correctly references #7630, which is the right issue for this work. Cosmetic, but worth flagging so future archeology is easier.
💡 [suggestion] Type design
The schema declares string | boolean | null, but the runtime only handles false, string, and falls through everything else to /. That makes root: true quietly meaningless. Worth either narrowing to string | false (or string | false | null) in the schema, or adding an explicit branch for true so the type fully describes the behaviour.
Self-check
- 🔴 blocking: 3
- 🟡 important: 2
- 💡 suggestion: 1
- 🎉 praise: 1
Verdict: REQUEST_CHANGES — the schema default and renderer recursion fixes should land together, since each on its own would still leave one of the Argos snapshots wrong. Once those are in plus the commonClear() restore, this should be ready.
Thanks again for the contribution — the feature is genuinely useful and the bugs are all small mechanical fixes. 🙏
2 participants
Resolves issue #7630
TreeView Root Feature Work
What Was Implemented
I added a configurable root node feature for treeView diagrams, allowing users to customize the root node behavior through a new config.treeView.root option.
Feature Details
The feature provides three root node configurations:
Default behavior (undefined): Shows "/" as the root node
Custom label (string): Displays a custom label for the root node
Hidden root (false): Completely hides the root node