feat(cli,playground,docs,generators): Export LikeC4 views to Draw.io#14
feat(cli,playground,docs,generators): Export LikeC4 views to Draw.io#14
Conversation
…ks (symlink guard, comments, arcSize) Co-authored-by: Cursor <[email protected]>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR refines Draw.io export and related tooling: stabilizes playground export listeners, tightens layout request typing, improves Draw.io parsing/round-trip comment emission, deduplicates filesystem traversal, validates PNG export server startup, and updates test/docs comments. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
…-24.04-arm; chore: JSDoc in drawio/json/png handlers and generators Co-authored-by: Cursor <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/log/src/formatters.ts (1)
20-30:⚠️ Potential issue | 🟡 MinorUse
switch (true)for this conditional chain.
The current if/else chain violates the style rule.♻️ Suggested refactor
.flatMap(([k, err]) => { - if (err instanceof Error) { - const mergedErr = mergeErrorCause(err) - if (mergedErr.stack) { - mergedErr.stack = parseStack(mergedErr.stack).join('\n') - } - return [mergedErr] - } - if (k === 'error' || k === 'err') { - return [new Error(loggable(err))] - } - return [] + switch (true) { + case err instanceof Error: { + const mergedErr = mergeErrorCause(err) + if (mergedErr.stack) { + mergedErr.stack = parseStack(mergedErr.stack).join('\n') + } + return [mergedErr] + } + case k === 'error' || k === 'err': + return [new Error(loggable(err))] + default: + return [] + } })As per coding guidelines, Favor switch(true) over if-else chains.
🤖 Fix all issues with AI agents
In `@packages/generators/src/drawio/parse-drawio.ts`:
- Around line 96-99: The type guard isEdgeWithEndpoints currently allows empty
strings for source/target, which contradicts the "non-empty" intent and can leak
into FQN lookups; update the guard to ensure both c.source and c.target are
non-empty strings (e.g., check typeof === 'string' && c.source.trim().length > 0
and same for c.target) and keep the doc comment in sync to reflect that
endpoints must be non-empty, referring to the isEdgeWithEndpoints function and
the DrawioCell/DrawioEdgeWithEndpoints types.
🧹 Nitpick comments (1)
.github/workflows/checks.yaml (1)
16-16: Switching to ARM runners—verify e2e compatibility.This change moves all Linux jobs from x64 to ARM64 runners (
ubuntu-24.04-arm). While ARM runners are typically cost-effective and Node.js/pnpm work fine on ARM, please verify:
- Playwright e2e tests (line 148): Confirm that Chromium installation and Playwright tests pass on ARM. ARM Linux support exists but has historically been less battle-tested.
- Native dependencies: Ensure any native Node modules used in the build compile correctly on ARM64.
Also, this infrastructure change appears unrelated to the PR's stated objective (Draw.io export). Consider documenting this in the PR description or splitting into a separate PR for clearer change tracking.
,
Also applies to: 38-38, 86-86, 104-104, 126-126, 148-148, 215-215, 265-265
| /** Type guard: true when cell is an edge with non-empty source and target. */ | ||
| function isEdgeWithEndpoints(c: DrawioCell): c is DrawioEdgeWithEndpoints { | ||
| return c.edge === true && typeof c.source === 'string' && typeof c.target === 'string' | ||
| } |
There was a problem hiding this comment.
Type guard allows empty endpoints despite “non-empty” intent.
This accepts '' for source/target, which can leak through to FQN lookups. Tighten the guard or adjust the doc comment.
Suggested fix
function isEdgeWithEndpoints(c: DrawioCell): c is DrawioEdgeWithEndpoints {
- return c.edge === true && typeof c.source === 'string' && typeof c.target === 'string'
+ return c.edge === true &&
+ typeof c.source === 'string' && c.source.trim() !== '' &&
+ typeof c.target === 'string' && c.target.trim() !== ''
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Type guard: true when cell is an edge with non-empty source and target. */ | |
| function isEdgeWithEndpoints(c: DrawioCell): c is DrawioEdgeWithEndpoints { | |
| return c.edge === true && typeof c.source === 'string' && typeof c.target === 'string' | |
| } | |
| /** Type guard: true when cell is an edge with non-empty source and target. */ | |
| function isEdgeWithEndpoints(c: DrawioCell): c is DrawioEdgeWithEndpoints { | |
| return c.edge === true && | |
| typeof c.source === 'string' && c.source.trim() !== '' && | |
| typeof c.target === 'string' && c.target.trim() !== '' | |
| } |
🤖 Prompt for AI Agents
In `@packages/generators/src/drawio/parse-drawio.ts` around lines 96 - 99, The
type guard isEdgeWithEndpoints currently allows empty strings for source/target,
which contradicts the "non-empty" intent and can leak into FQN lookups; update
the guard to ensure both c.source and c.target are non-empty strings (e.g.,
check typeof === 'string' && c.source.trim().length > 0 and same for c.target)
and keep the doc comment in sync to reflect that endpoints must be non-empty,
referring to the isEdgeWithEndpoints function and the
DrawioCell/DrawioEdgeWithEndpoints types.
feat(cli,playground,docs,generators): Export LikeC4 views to Draw.io
Summary
This PR adds export of LikeC4 views to Draw.io (
.drawio) format from the CLI (likec4 export drawio) and the Playground (right-click on diagram → DrawIO → Export view / Export all). It does not include import; import will be proposed in a separate PR.Context for maintainers: This work was first submitted in PR #2614, which received substantial review feedback focused on clean code, structure, and maintainability. We took that feedback seriously and ran a deliberate refactor pass (Uncle Bob / Clean Code) over the DrawIO-related code. This PR re-submits the same feature set with a cleaner, refactored codebase that we believe is easier to review and maintain. We’re grateful for the earlier review and have aimed to address those concerns in this iteration.
What’s in this PR
1. Generators (
@likec4/generators)computeDiagramLayoutsplit into smaller helpers;getViewDescriptionStringextracted;buildNodeCellXml/buildEdgeCellXmland constants (SOLID/DRY/KISS); exported typeDrawioViewModelLike.parseDrawioToLikeC4Multisplit intomergeDiagramStatesIntoMaps,buildRootsFromFqnToCell,emitMultiDiagramModel(orchestrator ~50–60 lines);buildViewBlockLines/escapeLikec4Quotes; O(n²) deduplication replaced withparsedIdsSet; UserObjectfullTagusesinnerXmlso sibling<data>is preserved.generate-drawio.spec.ts,parse-drawio.spec.ts; snapshots in__snapshots__/. Decompress error assertion accepts (base64 decode|inflate|URI decode) for Node/env behavior; snapshots updated for CI.2. CLI (
@likec4/likec4)likec4 export drawiowith--outdir/-o,--all-in-one,--roundtrip,--uncompressed,--project,--use-dot. UsesDEFAULT_DRAWIO_ALL_FILENAMEfrom@likec4/generators(DRY). Phase comments and thin handler pattern aligned with other export commands.PngExportArgs,runExportPng(args, logger); PNG export supports--outdir/-o(docs updated).likec4 import drawioin this PR (nopackages/likec4/src/cli/import/).3. Playground
generateDrawio/generateDrawioMultiandparseDrawioRoundtripComments. No Import menu item or file input.4. Documentation
--outdir; no Import section.5. E2E & tests
playwright.playground.config.ts.drawio-demo-export-import.spec.ts,drawio-tutorial-export-import.spec.ts— export tests; import/round-trip tests skipped in this PR.What’s not in this PR
likec4 import drawiocommand.Refactor summary (response to PR likec4#2614 feedback)
After the initial submission (PR likec4#2614), we applied a structured clean-code pass:
DrawioViewModelLike); constants; phase comments; DRY (e.g.DEFAULT_DRAWIO_ALL_FILENAME, shared helpers).runExport*(args, logger)pattern for drawio/PNG/JSON; consistent error handling and JSDoc.e2e/helpers/.We believe this version is in better shape for review and long-term maintenance.
Correções após review (PR #11)
Incorporadas as sugestões do review anterior:
apimemoizado comuseMemo(..., [actions.openMenu])para evitar re-renders desnecessários.LayoutView.reqemsendRequest(type-safe; removido cast manual).exitCode === 1no erro rejeitado.basenameremove barras finais (/e\) e trata resultado vazio com fallback.arcSize=12(inteiro percentagem) em vez de0.12para cantos ligeiramente arredondados no Draw.io.buildCommonDiagramStateFromCells;buildSingleDiagramStateebuildDiagramStatereutilizam (DRY).useDotlido dos args e passado arunExportDrawio; graphvizbinaryvswasmconforme flag--use-dot.useDotnos args e emrunExportJson; guard quandoprojectsModels.length === 0(warn + throw).Alterações adicionais (nitpicks e robustez)
requestLayoutViewpassa a usarLayoutView.reqemsendRequest(type-safe; params e resposta inferidos; cast removido).try;finallygaranteserver.close()mesmo quandoresolveServerUrlou o export lançam (evita leak do ViteDevServer).readWorkspaceSourceContentcom proteção a ciclos de symlink:realpath+visitedDirspara não seguir o mesmo diretório duas vezes.Checklist
main(merge/rebase as appropriate).feat:,refactor:,test:).pnpm ci:test(Vitest) passes.Verification
pnpm build(filter!./apps/*),pnpm typecheck,pnpm test(orpnpm ci:test) — pass.Notes for reviewers
DrawioViewModelLikeis the public type for view models passed togenerateDrawio/generateDrawioMulti.runExport*+ thin handler).Reference
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Documentation