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.

Author: filipsajdak

.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.

packages/mermaid/src/diagrams/c4/c4Db.ts

@@ -15,6 +15,13 @@ import type { C4Boundary, C4Rel, C4Shape } from './c4Types.js';
  */
 type ParserAttribute = string | Record<string, string>;
 
+/**
+ * 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 };
     }

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');
+  });
+});