From 30325d4984f48fd401c5d5bbb950879df2b3158d Mon Sep 17 00:00:00 2001 From: Filip Sajdak Date: Fri, 21 Aug 2026 11:15:47 +0200 Subject: [PATCH 1/4] fix(c4): stop wrapping non-text named attributes that land in a text slot A named attribute lands in a positional slot when the argument before it is omitted, and the handlers for the text slots (`label`, `descr`, `techn`, `type`) wrapped whatever arrived in `{ text: value }` regardless of which field it was actually for. So System(s, "S", $tags="cylinder") stored `tags` as `{ text: 'cylinder' }`, while System(s, "S", "desc", $tags="cylinder") stored the string. Consumers read `$tags` as a string - `resolveNodeShape` calls `.split(',')` on it - so the first form threw `shape.tags.split is not a function` and the diagram failed to render. Only a text field belongs in a `{ text }` wrapper, so the eleven slot handlers now consult one set of field names and assign anything else raw, matching what `assignAttributes` already did for the attributes that arrive in their own slots. Found while building `AddElementTag` support: the existing e2e case passes only because its fixture supplies a description before `$tags`, so the broken form was never exercised. Regression tests cover both forms of `$tags`, a named `$descr` (which must still be wrapped), `$sprite`/`$link`, and a relationship `$tags`; three of the five fail without this change. --- .changeset/c4-named-attr-text-slot.md | 10 ++++ packages/mermaid/src/diagrams/c4/c4Db.ts | 29 ++++++---- .../c4/parser/c4NamedAttributes.spec.ts | 55 +++++++++++++++++++ 3 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 .changeset/c4-named-attr-text-slot.md create mode 100644 packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts diff --git a/.changeset/c4-named-attr-text-slot.md b/.changeset/c4-named-attr-text-slot.md new file mode 100644 index 00000000000..3f6268ada64 --- /dev/null +++ b/.changeset/c4-named-attr-text-slot.md @@ -0,0 +1,10 @@ +--- +'mermaid': patch +--- + +fix(c4): stop wrapping non-text named attributes that land in a text slot + +A named attribute such as `$tags` or `$sprite` can arrive in the positional +slot of a text field when the argument before it is omitted. It was then +stored as `{ text: value }` rather than a string, so `$tags` given without a +description crashed rendering. diff --git a/packages/mermaid/src/diagrams/c4/c4Db.ts b/packages/mermaid/src/diagrams/c4/c4Db.ts index f08f3522c43..9b4c5317120 100644 --- a/packages/mermaid/src/diagrams/c4/c4Db.ts +++ b/packages/mermaid/src/diagrams/c4/c4Db.ts @@ -15,6 +15,13 @@ import type { C4Boundary, C4Rel, C4Shape } from './c4Types.js'; */ type ParserAttribute = string | Record; +/** + * The fields a C4 element, boundary or relationship stores as `{ text }`. Everything else + * a named attribute can set - `$tags`, `$sprite`, `$link`, colours, `$shape` - is a plain + * string. + */ +const TEXT_FIELDS = new Set(['label', 'descr', 'techn', 'type']); + /** * Apply optional C4 attributes to `bag` from the parser. * @@ -114,7 +121,7 @@ export const addRel = function ( } else { if (typeof techn === 'object') { const [key, value] = Object.entries(techn)[0]; - rel[key] = { text: value }; + rel[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { rel.techn = { text: techn }; } @@ -125,7 +132,7 @@ export const addRel = function ( } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; - rel[key] = { text: value }; + rel[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { rel.descr = { text: descr }; } @@ -171,7 +178,7 @@ export const addPersonOrSystem = function ( } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; - personOrSystem[key] = { text: value }; + personOrSystem[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { personOrSystem.descr = { text: descr }; } @@ -220,7 +227,7 @@ export const addContainer = function ( } else { if (typeof techn === 'object') { const [key, value] = Object.entries(techn)[0]; - container[key] = { text: value }; + container[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { container.techn = { text: techn }; } @@ -231,7 +238,7 @@ export const addContainer = function ( } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; - container[key] = { text: value }; + container[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { container.descr = { text: descr }; } @@ -280,7 +287,7 @@ export const addComponent = function ( } else { if (typeof techn === 'object') { const [key, value] = Object.entries(techn)[0]; - component[key] = { text: value }; + component[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { component.techn = { text: techn }; } @@ -291,7 +298,7 @@ export const addComponent = function ( } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; - component[key] = { text: value }; + component[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { component.descr = { text: descr }; } @@ -339,7 +346,7 @@ export const addPersonOrSystemBoundary = function ( } else { if (typeof type === 'object') { const [key, value] = Object.entries(type)[0]; - boundary[key] = { text: value }; + boundary[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { boundary.type = { text: type }; } @@ -390,7 +397,7 @@ export const addContainerBoundary = function ( } else { if (typeof type === 'object') { const [key, value] = Object.entries(type)[0]; - boundary[key] = { text: value }; + boundary[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { boundary.type = { text: type }; } @@ -444,7 +451,7 @@ export const addDeploymentNode = function ( } else { if (typeof type === 'object') { const [key, value] = Object.entries(type)[0]; - boundary[key] = { text: value }; + boundary[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { boundary.type = { text: type }; } @@ -455,7 +462,7 @@ export const addDeploymentNode = function ( } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; - boundary[key] = { text: value }; + boundary[key] = TEXT_FIELDS.has(key) ? { text: value } : value; } else { boundary.descr = { text: descr }; } diff --git a/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts b/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts new file mode 100644 index 00000000000..3fa0e978636 --- /dev/null +++ b/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts @@ -0,0 +1,55 @@ +import c4Db from '../c4Db.js'; +// @ts-ignore: JISON doesn't support types +import c4 from './c4Diagram.jison'; +import { setConfig } from '../../../config.js'; + +setConfig({ securityLevel: 'strict' }); + +/** + * A named attribute can land in a positional slot when the argument before it is omitted. + * Only the slot's own field is text, so the value must not be wrapped in `{ text }` just + * because it arrived through a text slot - consumers read `$tags` and `$sprite` as strings. + */ +describe('named attributes arriving in a positional text slot', function () { + beforeEach(function () { + c4.parser.yy = c4Db; + c4.parser.yy.clear(); + }); + + it('keeps $tags a string when the description is omitted', function () { + c4.parser.parse(`C4Context\nSystem(s, "S", $tags="cylinder")`); + + expect(c4.parser.yy.getC4ShapeArray()[0].tags).toBe('cylinder'); + }); + + it('keeps $tags a string when the description is given', function () { + c4.parser.parse(`C4Context\nSystem(s, "S", "desc", $tags="cylinder")`); + + const [shape] = c4.parser.yy.getC4ShapeArray(); + expect(shape.tags).toBe('cylinder'); + expect(shape.descr.text).toBe('desc'); + }); + + it('still stores the description as text when it is the one named', function () { + c4.parser.parse(`C4Context\nSystem(s, "S", $descr="a description")`); + + expect(c4.parser.yy.getC4ShapeArray()[0].descr.text).toBe('a description'); + }); + + it('keeps $sprite and $link strings on a container', function () { + c4.parser.parse(`C4Container\nContainer(c, "C", $sprite="browser", $link="https://x.test")`); + + const [shape] = c4.parser.yy.getC4ShapeArray(); + expect(shape.sprite).toBe('browser'); + expect(shape.link).toBe('https://x.test'); + }); + + it('keeps a relationship $tags a string when techn and descr are omitted', function () { + c4.parser.parse(`C4Context +Person(a, "A") +Person(b, "B") +Rel(a, b, "uses", $tags="async")`); + + expect(c4.parser.yy.getRels()[0].tags).toBe('async'); + }); +}); From 0e3ce32a9db8b0f8430c5e035c68933b1cc29014 Mon Sep 17 00:00:00 2001 From: Filip Sajdak Date: Fri, 21 Aug 2026 14:09:08 +0200 Subject: [PATCH 2/4] fix(c4): stop a later slot's default from clobbering a named attribute Addresses the review point on the previous commit, which turned out to describe a live bug rather than only a test gap. `Container` takes `techn` before `descr`, so in Container(c, "C", $descr="a description") the named attribute lands in the `techn` slot. That handler assigns `descr` correctly - and then the `descr` handler runs, finds its own argument absent, and overwrites the value with `{ text: '' }`. The description was silently lost, not merely stored in the wrong shape, which is why the previous commit's tests did not catch it: they exercise `System`, where `descr` is the first optional slot and nothing follows it. The "argument absent" branches now only fill in a default when the field has not already been set, so an earlier handler's assignment survives. The same applies to `techn` and `type`, and to relationships, where `techn` likewise precedes `descr`. Three tests added: a named `$descr` landing in an earlier slot on a container and on a relationship, and a check that `descr` still defaults to empty when it is genuinely absent. The first two fail without this change. --- packages/mermaid/src/diagrams/c4/c4Db.ts | 16 +++++------ .../c4/parser/c4NamedAttributes.spec.ts | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/mermaid/src/diagrams/c4/c4Db.ts b/packages/mermaid/src/diagrams/c4/c4Db.ts index 9b4c5317120..43d2eb963e8 100644 --- a/packages/mermaid/src/diagrams/c4/c4Db.ts +++ b/packages/mermaid/src/diagrams/c4/c4Db.ts @@ -117,7 +117,7 @@ export const addRel = function ( rel.label = { text: label }; if (techn === undefined || techn === null) { - rel.techn = { text: '' }; + rel.techn ??= { text: '' }; } else { if (typeof techn === 'object') { const [key, value] = Object.entries(techn)[0]; @@ -128,7 +128,7 @@ export const addRel = function ( } if (descr === undefined || descr === null) { - rel.descr = { text: '' }; + rel.descr ??= { text: '' }; } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; @@ -174,7 +174,7 @@ export const addPersonOrSystem = function ( } if (descr === undefined || descr === null) { - personOrSystem.descr = { text: '' }; + personOrSystem.descr ??= { text: '' }; } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; @@ -223,7 +223,7 @@ export const addContainer = function ( } if (techn === undefined || techn === null) { - container.techn = { text: '' }; + container.techn ??= { text: '' }; } else { if (typeof techn === 'object') { const [key, value] = Object.entries(techn)[0]; @@ -234,7 +234,7 @@ export const addContainer = function ( } if (descr === undefined || descr === null) { - container.descr = { text: '' }; + container.descr ??= { text: '' }; } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; @@ -283,7 +283,7 @@ export const addComponent = function ( } if (techn === undefined || techn === null) { - component.techn = { text: '' }; + component.techn ??= { text: '' }; } else { if (typeof techn === 'object') { const [key, value] = Object.entries(techn)[0]; @@ -294,7 +294,7 @@ export const addComponent = function ( } if (descr === undefined || descr === null) { - component.descr = { text: '' }; + component.descr ??= { text: '' }; } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; @@ -458,7 +458,7 @@ export const addDeploymentNode = function ( } if (descr === undefined || descr === null) { - boundary.descr = { text: '' }; + boundary.descr ??= { text: '' }; } else { if (typeof descr === 'object') { const [key, value] = Object.entries(descr)[0]; diff --git a/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts b/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts index 3fa0e978636..bcb6e4b4066 100644 --- a/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts +++ b/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts @@ -52,4 +52,31 @@ Rel(a, b, "uses", $tags="async")`); expect(c4.parser.yy.getRels()[0].tags).toBe('async'); }); + + // `Container` takes `techn` before `descr`, so a named `$descr` here lands in the + // `techn` slot. The techn handler assigned it correctly, and then the descr handler's + // "argument absent" branch overwrote it with an empty default - the description was + // silently lost rather than misshaped. + it('keeps a named $descr that lands in an earlier slot', function () { + c4.parser.parse(`C4Container\nContainer(c, "C", $descr="a description")`); + + expect(c4.parser.yy.getC4ShapeArray()[0].descr.text).toBe('a description'); + }); + + it('still defaults descr to empty when it is genuinely absent', function () { + c4.parser.parse(`C4Container\nContainer(c, "C", $techn="Java")`); + + const [shape] = c4.parser.yy.getC4ShapeArray(); + expect(shape.techn.text).toBe('Java'); + expect(shape.descr.text).toBe(''); + }); + + it('keeps a named $descr on a relationship, where techn also comes first', function () { + c4.parser.parse(`C4Context +Person(a, "A") +Person(b, "B") +Rel(a, b, "uses", $descr="how it is used")`); + + expect(c4.parser.yy.getRels()[0].descr.text).toBe('how it is used'); + }); }); From eb9ec8ad74a8732e13df2b9bbe79413f2a2afc73 Mon Sep 17 00:00:00 2001 From: Filip Sajdak Date: Fri, 21 Aug 2026 19:14:13 +0200 Subject: [PATCH 3/4] fix(c4): store a text field as text however it reaches the db Completes the pair of fixes above by making `assignAttributes` follow the same rule as the positional slot handlers. `System_Boundary` and `Container_Boundary` splice their kind in as a positional argument, so an explicit `$type` shifts one slot along and arrives as a named attribute in the `tags` slot. `assignAttributes` assigned every named attribute raw, so it overwrote the `{ text: 'SYSTEM' }` the type slot had just set with the bare string `'V'` - and every consumer reads `boundary.type.text`, so the type silently vanished. `Node` was unaffected, because its type is a real positional slot. Found by enumerating the combinations rather than by chance: this is the third defect in this area, and each one only appeared for one particular declaration kind and attribute. The tests now walk every kind of declaration against every attribute that can reach its first optional slot - 34 combinations - and assert the stored shape. The two boundary `$type` cases fail without this change. --- packages/mermaid/src/diagrams/c4/c4Db.ts | 7 +- .../c4/parser/c4NamedAttributes.spec.ts | 64 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/packages/mermaid/src/diagrams/c4/c4Db.ts b/packages/mermaid/src/diagrams/c4/c4Db.ts index 43d2eb963e8..5a7b057cab4 100644 --- a/packages/mermaid/src/diagrams/c4/c4Db.ts +++ b/packages/mermaid/src/diagrams/c4/c4Db.ts @@ -38,7 +38,12 @@ const assignAttributes = ( } if (typeof value === 'object') { const [key, val] = Object.entries(value)[0]; - bag[key] = val; + // Same rule as the positional text slots: a text field is stored as `{ text }` + // whichever slot it arrived through. `System_Boundary` and friends splice their + // kind in as a positional argument, so an explicit `$type` shifts along into this + // one and would otherwise land here as a bare string and overwrite the wrapped + // value the type slot just set. + bag[key] = TEXT_FIELDS.has(key) ? { text: val } : val; } else { bag[field] = value; } diff --git a/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts b/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts index bcb6e4b4066..3a927b785ae 100644 --- a/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts +++ b/packages/mermaid/src/diagrams/c4/parser/c4NamedAttributes.spec.ts @@ -79,4 +79,68 @@ Rel(a, b, "uses", $descr="how it is used")`); expect(c4.parser.yy.getRels()[0].descr.text).toBe('how it is used'); }); + + /** + * Every kind of declaration, given exactly one named attribute so that it lands in the + * first optional slot - which is rarely its own. Each bug found in this area so far + * only showed up for one such combination, so they are enumerated rather than sampled. + * + * `descr`, `techn` and `type` are stored as `{ text }`; `tags`, `sprite` and `link` are + * plain strings. Which slot the value travelled through must not change that. + */ + describe('one named attribute, whichever slot it lands in', function () { + const TEXT_FIELDS = new Set(['descr', 'techn', 'type']); + + const expectField = (subject: Record, field: string) => { + if (TEXT_FIELDS.has(field)) { + expect(subject[field]).toEqual({ text: 'V' }); + } else { + expect(subject[field]).toBe('V'); + } + }; + + const shape = () => c4.parser.yy.getC4ShapeArray()[0]; + const boundary = () => + c4.parser.yy.getBoundaries().find((b: { alias: string }) => b.alias !== 'global'); + + const elements: [string, string, string[]][] = [ + ['C4Context', 'System(x, "L", $F="V")', ['descr', 'tags', 'sprite', 'link']], + ['C4Context', 'Person(x, "L", $F="V")', ['descr', 'tags', 'sprite', 'link']], + ['C4Container', 'Container(x, "L", $F="V")', ['descr', 'techn', 'tags', 'sprite', 'link']], + ['C4Component', 'Component(x, "L", $F="V")', ['descr', 'techn', 'tags', 'sprite', 'link']], + ]; + for (const [header, template, fields] of elements) { + it.each(fields)(`${template.replace('$F', '$%s')}`, function (field) { + c4.parser.parse(`${header}\n${template.replace('$F', `$${field}`)}`); + expectField(shape(), field); + }); + } + + // `System_Boundary` and `Container_Boundary` splice their kind in as a positional + // argument, so an explicit `$type` shifts one slot along into `tags`. + const boundaries: [string, string, string[]][] = [ + ['C4Context', 'System_Boundary(x, "L", $F="V")', ['type', 'tags', 'link']], + ['C4Container', 'Container_Boundary(x, "L", $F="V")', ['type', 'tags', 'link']], + ['C4Deployment', 'Node(x, "L", $F="V")', ['type', 'descr', 'tags', 'sprite', 'link']], + ]; + for (const [header, template, fields] of boundaries) { + it.each(fields)(`${template.replace('$F', '$%s')}`, function (field) { + c4.parser.parse( + `${header}\n${template.replace('$F', `$${field}`)} {\nContainer(i, "I")\n}` + ); + expectField(boundary(), field); + }); + } + + it.each(['techn', 'descr', 'tags', 'sprite', 'link'])( + 'Rel(a, b, "uses", $%s="V")', + function (field) { + c4.parser.parse(`C4Context +Person(a, "A") +Person(b, "B") +Rel(a, b, "uses", $${field}="V")`); + expectField(c4.parser.yy.getRels()[0], field); + } + ); + }); }); From 8ba08e417754ef950161d475e333c1682cde0c3a Mon Sep 17 00:00:00 2001 From: Filip Sajdak Date: Thu, 27 Aug 2026 13:30:20 +0200 Subject: [PATCH 4/4] docs(c4): say that the example's red styling is a demonstration The canonical C4Context example doubles as an UpdateElementStyle demo: it restyles `customerA` red, and colours the relationships touching it, without saying so anywhere nearby. Read as a picture of the defaults, it looks like one Person renders differently from the others for no reason, which is how it was reported in #7491. A note under each of the two copies of that example, rather than a change to the example itself, so the demonstration stays where it is. --- docs/syntax/c4.md | 4 ++++ packages/mermaid/src/docs/syntax/c4.md | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/docs/syntax/c4.md b/docs/syntax/c4.md index 4d246e7399f..e17baa6aea1 100644 --- a/docs/syntax/c4.md +++ b/docs/syntax/c4.md @@ -104,6 +104,8 @@ Mermaid's C4 diagram syntax is compatible with plantUML. See example below: ``` +The colours in this example are a demonstration of custom styling, not the defaults. `UpdateElementStyle` turns "Banking Customer A" red, and the `UpdateRelStyle` statements beside it recolour four relationships - three blue, including the one leaving "Banking Customer A", and the one arriving from the e-mail system red. Remove those statements and every element and relationship renders in the standard C4 palette. + For an example, see the source code demos/index.html 5 types of C4 charts are supported. @@ -318,6 +320,8 @@ config: ``` +As above, "Banking Customer A" is red because the example restyles it with `UpdateElementStyle`, not because that is the default appearance of a `Person`. + ## C4 Container diagram (C4Container) ```mermaid-example diff --git a/packages/mermaid/src/docs/syntax/c4.md b/packages/mermaid/src/docs/syntax/c4.md index e75b205a377..f7c1ac6eba4 100644 --- a/packages/mermaid/src/docs/syntax/c4.md +++ b/packages/mermaid/src/docs/syntax/c4.md @@ -51,6 +51,8 @@ Mermaid's C4 diagram syntax is compatible with plantUML. See example below: ``` +The colours in this example are a demonstration of custom styling, not the defaults. `UpdateElementStyle` turns "Banking Customer A" red, and the `UpdateRelStyle` statements beside it recolour four relationships - three blue, including the one leaving "Banking Customer A", and the one arriving from the e-mail system red. Remove those statements and every element and relationship renders in the standard C4 palette. + For an example, see the source code demos/index.html 5 types of C4 charts are supported. @@ -216,6 +218,8 @@ config: ``` +As above, "Banking Customer A" is red because the example restyles it with `UpdateElementStyle`, not because that is the default appearance of a `Person`. + ## C4 Container diagram (C4Container) ```mermaid-example