Repository navigation
refactor!: make the codebase smaller and gate on duplication and test-only code - #112
Merged
Merged
Conversation
…cript and a dead test gate
…e one parent-slot helper
…talogue through functions
…e methods only they reached BREAKING CHANGE: mutationWarnings, opWarnings, renderAround, renderContext, renderMetrics, decisionLinkOf, scopeView, assertLossless, kindsSectionJson, runFormatOps, viewDoc and renderShown are no longer exported from @miragon/bpmn-cli, and layoutDocToFile no longer from @miragon/bpmn-cli/node. None of them was documented in docs/library.md.
BREAKING CHANGE: flow and DI ids that bpmn-cli 0.6 cut at the 64-character limit in its own way are no longer recognised as generated, so they keep their id when their ends change. Ids 0.7 and later wrote, and uncut ids of any version, are not affected.
…p production mode)
emaarco
marked this pull request as ready for review
October 27, 2026 16:57
…h mandatory ports Every port of EditDocumentService, RelayoutDiagramService, CreateDocumentService, ValidateDocumentService and ReadDocumentService is required now, so no `!` reaches into an optional one. The in-memory API hands them NoDocumentFiles (adapters/out/memory), the counterpart of NoProjectSettings: nothing to read, nothing written. The public method of each service reads as open, call the domain on a line of its own, write. The check of `bpmn validate` moves out of ValidateDocumentService into the domain object DocumentCheck, which owns CheckOptions and CheckResult; the port re-exports them. The jscpd budget goes from 0.2 % to 0.25 %: the small open / target / write steps repeat in three services, as the architecture keeps helpers out of application/.
…fore it writes the image
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the codebase measurably smaller without moving complexity around. Candidates came from a baseline of knip, jscpd, madge and change hotspots. Each was kept only if it removes net lines or concepts. Code lines in
src/(cloc) drop from 42,538 to 42,212 (−326; −397 before the last commit, which spends 71 lines on clearer services). Test code shrinks by 171 lines. Duplication insrc/falls from 21 clones (0.38 %) to 13 (0.24 %).The baseline already showed a lean codebase: knip found nothing unused and duplication was 0.38 %. The gains are consolidations, and the size that remains is in the features.
What changes, one commit each:
playwrightdevDependency (its script went in chore: remove the demo GIF script in favour of the product-shots skill #90),tools/render.sh(tools/bench/render.shrenders any file), and theit.todogate inops-mutate.test.tsfor packages that are implemented by now.Cli.guardedis gone, becauseCli.runalready reports every error thrown by a command. TheloadPort()/savePort()guards in five services are dropped too, since no entry calls a portless service with a file.harness.ts),routing.ts,geometry.tsandNotePlacerreusegeom.tsinstead of their own copies (segmentHits,overlaps,containsPoint,segmentsCross,toSegments,crossings). Newtranslate*helpers replace the hand-written move loops inFrameEditor,SpaceTool,NodePlacer,Drawingandscope.ts.PlacerStateis now aPick<NodePlacer, …>.CommandParser.parseBranchesinstead of a copy inBranchSplitService.detachFromParent(el, replacement?)replaces the copy inElementTypeChangeService, and the identity mapCASE_WORDis gone.givenStrings/givenFlagsreplace 35 repeated...(options['x'] ? { x: String(options['x']) } : {})lines.CommandCataloguebecomes the functionscommandDocandcommonOption.allLanesmoves todocument/utils/lanes.tsand is shared byDoc,LaneChecksandTemporaryLaneMembers, which also usesindexById.!reaches into an optional one; the in-memory API passesNoDocumentFiles(adapters/out/memory), the counterpart ofNoProjectSettings. Each public method reads as open, call the domain on a line of its own, write. The check ofbpmn validatemoves into the domain objectDocumentCheck, which ownsCheckOptionsandCheckResult(the port re-exports them).npm run gatenow also runsknip --production --exclude dependencies, which fails on code insrc/that only tests reach unless the export is tagged@internal; the eight existing test hooks are tagged. It also runsjscpdwith a budget of 0.25 % forsrc/(the small open / target / write steps repeat in three services, since the architecture keeps helpers out ofapplication/) (.jscpd.json, jscpd 5.4.1 as a new devDependency). knip 6 dropped its class-member check, so unused public methods are not caught automatically.Breaking changes
mutationWarnings,opWarnings,renderAround,renderContext,renderMetrics,decisionLinkOf,scopeView,assertLossless,kindsSectionJson,runFormatOps,viewDocandrenderShownare no longer exported from@miragon/bpmn-cli, andlayoutDocToFileno longer from@miragon/bpmn-cli/node. None of them was documented indocs/library.md, and none is used intest/,tools/, design-iq, ask-miragon, elvis or process-engine-blueprints. The use-case methods that only they reached are removed with them.Looked at and kept:
--engine autotogether with bpmn-auto-layout. On the 115 scenarios, a full redraw with each engine and the same layout metrics gives a total problem score of 410 for clean against 554 for auto. Clean is better on 39 files, auto on 22 and they tie on 54. Auto draws some diagrams cleanly where clean does not (e.g.controlflow__s23: 7 for clean, 0 for auto), so it stays as the alternative.Checklist
npm run typecheckpassesnpm run gatepassessrc/report/guide.tsand the matching page indocs/are updated (only if a command, option or output changed)Every step was checked on its own with
npm run typecheckandnpm run gate(2,395 tests, 246 skipped live-engine tests). The layout-regression score stayed at 370 of a 370 budget throughout. For the id change,tools/speaking-ids.mjs --baselineagainst the build before it gives the same new ids on all 570 probes and no renamed flow. No command, option or CLI output changed. The docs that described removed code (docs/architecture.md,docs/testing.md,CONTRIBUTING.md) are updated. Not run: the live-engine tests (no Camunda engine running) and the full bench (tools/bench/run.mjs).Linked issue
None.