feat: add Structurizr DSL export for C4 diagrams (#264) #320
Conversation
- Add convertToStructurizrDsl utility with smart C4 element detection - Create export dialog with copy and download functionality - Integrate export button in chat toolbar - Support automatic hierarchy detection for systems and containers - Add README documentation for new feature Resolves DayuanJiang#264
|
@godhaniripal is attempting to deploy a commit to the dayuanjiang's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR adds Structurizr DSL export functionality for C4 model diagrams, enabling users to export draw.io diagrams as code for use with Structurizr tools.
Key Changes:
- Implemented intelligent C4 element detection (person, system, container, database) based on shapes and labels
- Created export dialog with copy-to-clipboard and download capabilities
- Added export button to chat toolbar with appropriate disabled states
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
| lib/structurizr-utils.ts | Core utility providing XML parsing, element classification, hierarchy detection, and DSL generation logic |
| components/export-structurizr-dialog.tsx | Dialog component with DSL preview, copy, and download functionality |
| components/chat-input.tsx | Integration of export button and dialog into the chat toolbar |
| README.md | Documentation of new Structurizr DSL export feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Person/Actor detection | ||
| if ( | ||
| style.includes("shape=actor") || | ||
| style.includes("shape=person") || | ||
| style.includes("shape=umlactor") || | ||
| value.includes("user") || | ||
| value.includes("customer") || | ||
| value.includes("admin") || | ||
| value.includes("actor") | ||
| ) { | ||
| return "person" | ||
| } | ||
|
|
||
| // Database detection | ||
| if ( | ||
| style.includes("shape=cylinder") || | ||
| style.includes("shape=datastore") || | ||
| style.includes("database") || | ||
| value.includes("database") || | ||
| value.includes(" db") || | ||
| value.endsWith("db") | ||
| ) { | ||
| return "database" | ||
| } | ||
|
|
||
| // System boundary detection (rounded rectangles, clouds, larger containers) | ||
| if ( | ||
| style.includes("rounded=1") || | ||
| style.includes("shape=hexagon") || | ||
| style.includes("shape=cloud") || | ||
| value.includes("system") || | ||
| value.includes("platform") || | ||
| (value.includes("service") && !value.includes("micro")) | ||
| ) { | ||
| return "system" | ||
| } |
There was a problem hiding this comment.
The string matching for element classification is overly broad and can lead to false positives. For example, any element containing the word "user" (e.g., "User Service", "User Data Store") will be classified as a "person" even if it's clearly a technical component. Similarly, "Microservice" would not be classified as a "system" due to the micro exclusion on line 152, but "Payment Service" would be.
Consider using more specific heuristics, such as checking if the value is exactly a person name, or looking for compound patterns. Alternatively, add configuration options or allow users to override classifications.
| useEffect(() => { | ||
| if (open && xml) { | ||
| try { | ||
| const result = convertToStructurizrDsl(xml) | ||
| setDsl(result) | ||
| setError("") | ||
| } catch (err) { | ||
| setError("Failed to convert diagram to DSL format") | ||
| setDsl("") | ||
| console.error("DSL conversion error:", err) | ||
| } | ||
| } | ||
| }, [open, xml]) |
There was a problem hiding this comment.
The copied state is not reset when the dialog is closed or when the xml prop changes. If a user copies the DSL, closes the dialog, then reopens it (either with the same or different diagram), the button will still show "Copied" for up to 2 seconds. This creates a confusing UX where the button state doesn't match the actual clipboard content.
Consider resetting the copied state when the dialog opens or when the xml/dsl changes.
| // Choose appropriate view type based on content | ||
| if ( | ||
| systems.length > 0 && | ||
| (systemContainersMap.size > 0 || orphanContainers.length > 0) | ||
| ) { | ||
| const firstSystem = systems[0] | ||
| lines.push(` container ${firstSystem.id} "Containers" {`) | ||
| lines.push(" include *") | ||
| lines.push(" autolayout lr") | ||
| lines.push(" }") | ||
| } else { | ||
| lines.push(" systemContext {") | ||
| lines.push(" include *") | ||
| lines.push(" autolayout lr") | ||
| lines.push(" }") | ||
| } |
There was a problem hiding this comment.
When orphan containers are placed in a default system with ID "system" (line 341), the view generation logic on line 379 tries to create a container view using systems[0].id. However, the default system with ID "system" is not added to the systems array - it's only added to the DSL output. This means the systems array is still empty, so the condition on line 374-377 evaluates to false even though there are containers. The generated DSL will have a systemContext view instead of a container view, which won't properly display the containers that were created.
Either add the default system to the systems array, or adjust the view logic to check for the existence of containers/orphanContainers when deciding on the view type.
| containers.forEach((container) => { | ||
| const cellId = Array.from(idMap.entries()).find( | ||
| ([, id]) => id === container.id, | ||
| )?.[0] | ||
| if (cellId) { | ||
| const parentId = parentMap.get(cellId) | ||
| if (parentId) { | ||
| const parentElement = cellIdToElement.get(parentId) | ||
| if (parentElement && parentElement.type === "system") { | ||
| if (!systemContainersMap.has(parentElement.id)) { | ||
| systemContainersMap.set(parentElement.id, []) | ||
| } | ||
| systemContainersMap.get(parentElement.id)!.push(container) | ||
| } | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
The code performs an O(n²) operation by iterating over containers and for each container, calling Array.from(idMap.entries()).find() which creates a new array and searches through all entries. For diagrams with many elements, this could be inefficient.
Consider creating a reverse lookup map (DSL ID -> Cell ID) once before the loop, which would reduce the complexity from O(n²) to O(n).
| const handleCopy = async () => { | ||
| try { | ||
| await navigator.clipboard.writeText(dsl) | ||
| setCopied(true) | ||
| setTimeout(() => setCopied(false), 2000) |
There was a problem hiding this comment.
The timeout for resetting the copied state is not cleaned up when the component unmounts or when the dialog is closed. If a user clicks copy and then closes the dialog before the 2-second timeout completes, the timeout will still fire and attempt to update the state of an unmounted component, which can lead to memory leaks and React warnings.
Store the timeout ID in a ref and clear it in a cleanup function or when the dialog closes.
| lines.push(` ${person.id} = person "${person.name}"`) | ||
| }) | ||
| lines.push("") | ||
| } | ||
|
|
||
| // Systems with their containers | ||
| if (systems.length > 0) { | ||
| lines.push(" # Software Systems") | ||
| systems.forEach((system) => { | ||
| lines.push( | ||
| ` ${system.id} = softwareSystem "${system.name}" {`, | ||
| ) | ||
|
|
||
| const systemContainers = systemContainersMap.get(system.id) || [] | ||
| systemContainers.forEach((container) => { | ||
| const tech = | ||
| container.type === "database" ? "Database" : "Application" | ||
| lines.push( | ||
| ` ${container.id} = container "${container.name}" "" "${tech}"`, | ||
| ) | ||
| }) | ||
|
|
||
| lines.push(" }") | ||
| }) | ||
| lines.push("") | ||
| } | ||
|
|
||
| // If we have orphan containers but no systems, create a default system | ||
| if (systems.length === 0 && orphanContainers.length > 0) { | ||
| // Try to infer system name from containers | ||
| const systemName = orphanContainers.some((c) => | ||
| c.name.toLowerCase().includes("commerce"), | ||
| ) | ||
| ? "E-Commerce System" | ||
| : "System" | ||
|
|
||
| lines.push(" # Software System") | ||
| lines.push(` system = softwareSystem "${systemName}" {`) | ||
| orphanContainers.forEach((container) => { | ||
| const tech = | ||
| container.type === "database" ? "Database" : "Application" | ||
| lines.push( | ||
| ` ${container.id} = container "${container.name}" "" "${tech}"`, | ||
| ) | ||
| }) | ||
| lines.push(" }") | ||
| lines.push("") | ||
| } | ||
|
|
||
| // Relationships | ||
| if (relationships.length > 0) { | ||
| lines.push(" # Relationships") | ||
| relationships.forEach((rel) => { | ||
| const sourceId = idMap.get(rel.source) | ||
| const targetId = idMap.get(rel.target) | ||
|
|
||
| if (sourceId && targetId) { | ||
| // Clean up description | ||
| const desc = rel.description.trim() || "Uses" | ||
| lines.push(` ${sourceId} -> ${targetId} "${desc}"`) |
There was a problem hiding this comment.
Element names and relationship descriptions are directly interpolated into DSL strings without escaping double quotes. If an element name or relationship description contains a double quote character (e.g., 'User "Admin"' or 'Uses "special API"'), it will produce invalid DSL syntax and cause parsing errors in Structurizr.
Escape double quotes in names and descriptions by replacing them with backslash-escaped quotes or by removing/replacing them with an alternative character.
| const textarea = document.createElement("textarea") | ||
| textarea.innerHTML = value | ||
| return textarea.value | ||
| .replace(/<br\s*\/?>/gi, " ") | ||
| .replace(/<[^>]+>/g, "") | ||
| .trim() |
There was a problem hiding this comment.
The use of textarea.innerHTML for HTML entity decoding is susceptible to XSS attacks if the input contains malicious scripts. While the subsequent regex removes HTML tags, an attacker could potentially inject script content before the cleanup occurs.
Consider using a safer approach like the browser's built-in DOMParser with text/html or a dedicated HTML entity decoding library. Alternatively, create a text node and use textContent instead of innerHTML to avoid any potential script execution.
| const textarea = document.createElement("textarea") | |
| textarea.innerHTML = value | |
| return textarea.value | |
| .replace(/<br\s*\/?>/gi, " ") | |
| .replace(/<[^>]+>/g, "") | |
| .trim() | |
| if (!value) { | |
| return "" | |
| } | |
| // Use DOMParser with text/html to safely decode HTML entities | |
| const parser = new DOMParser() | |
| const doc = parser.parseFromString(value, "text/html") | |
| let text = doc.body ? doc.body.textContent || "" : "" | |
| // Normalize line breaks (e.g., from <br>) to spaces and trim | |
| text = text.replace(/\r?\n/g, " ").trim() | |
| return text |
| document.body.appendChild(a) | ||
| a.click() | ||
| document.body.removeChild(a) | ||
| URL.revokeObjectURL(url) |
There was a problem hiding this comment.
The URL created by URL.createObjectURL should be revoked after the download completes to prevent memory leaks. Looking at the similar implementation in contexts/diagram-context.tsx (lines 306-316), there's a pattern of revoking the URL with a timeout. The current implementation creates the blob URL but never revokes it, which can lead to memory leaks if users export DSL multiple times during a session.
| URL.revokeObjectURL(url) | |
| setTimeout(() => { | |
| URL.revokeObjectURL(url) | |
| }, 1000) |
| function sanitizeId(name: string): string { | ||
| return ( | ||
| name | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]+/g, "_") | ||
| .replace(/^[0-9]/, "n$&") | ||
| .replace(/^_+|_+$/g, "") || "element" | ||
| ) |
There was a problem hiding this comment.
The sanitizeId function can return an empty string when the input name contains only special characters (e.g., "!!!"). After all regex replacements, if the result is empty, it falls back to "element", but the trailing underscore removal regex can produce an empty string before the fallback is evaluated. This could potentially cause duplicate "element" IDs if multiple elements have non-alphanumeric names.
Consider generating unique IDs by appending a counter or hash when falling back to "element" to ensure uniqueness.
| function sanitizeId(name: string): string { | |
| return ( | |
| name | |
| .toLowerCase() | |
| .replace(/[^a-z0-9]+/g, "_") | |
| .replace(/^[0-9]/, "n$&") | |
| .replace(/^_+|_+$/g, "") || "element" | |
| ) | |
| let elementIdCounter = 0 | |
| function sanitizeId(name: string): string { | |
| const baseId = name | |
| .toLowerCase() | |
| .replace(/[^a-z0-9]+/g, "_") | |
| .replace(/^[0-9]/, "n$&") | |
| .replace(/^_+|_+$/g, "") | |
| if (baseId) { | |
| // Normal case: we have a non-empty sanitized identifier | |
| return baseId | |
| } | |
| // Fallback for names that sanitize to an empty string. | |
| // Ensure these "element" IDs are unique across calls. | |
| elementIdCounter += 1 | |
| return elementIdCounter === 1 ? "element" : `element_${elementIdCounter}` |
| if (!systemContainersMap.has(parentElement.id)) { | ||
| systemContainersMap.set(parentElement.id, []) | ||
| } | ||
| systemContainersMap.get(parentElement.id)!.push(container) |
There was a problem hiding this comment.
The non-null assertion operator (!) is used without verification that the array will contain the container. While the code checks that the systemContainersMap has the parent ID, if there's a race condition or unexpected state, this could throw a runtime error. This pattern appears risky given the complex mapping logic.
Consider using optional chaining and providing a fallback, or adding an explicit null check before the push operation.
| if (!systemContainersMap.has(parentElement.id)) { | |
| systemContainersMap.set(parentElement.id, []) | |
| } | |
| systemContainersMap.get(parentElement.id)!.push(container) | |
| let containersForSystem = systemContainersMap.get(parentElement.id) | |
| if (!containersForSystem) { | |
| containersForSystem = [] | |
| systemContainersMap.set(parentElement.id, containersForSystem) | |
| } | |
| containersForSystem.push(container) |
|
Hey @godhaniripal, thanks for the work on this! After thinking it through, I don't think this fits what the app should be doing. Export format conversion is really draw.io's responsibility - we're just wrapping their editor with AI capabilities. Appreciate the effort though! |
2 participants
Resolves #264