Repository navigation
feat(layout-elk): ELK layout package, with audit fixes and API alignment - #3545
Merged
Merged
Conversation
Geliogabalus
approved these changes
Oct 12, 2026
kumilingus
force-pushed
the
fix/layout-elk-audit
branch
from
October 12, 2026 12:01
3aa1fb9 to
7c8ca67
Compare
…r-port ELK options - Elements embedded in another element are laid out as a container - the parent is resized (and positioned) by ELK to fit its content, at any nesting depth. Cross-container links are filed under their lowest common ancestor per ELK's hierarchical-edge convention. - Ports route edges to/from the position JointJS itself already computes for them (`elk.portConstraints: FIXED_POS`); the new `positionPorts` option lets ELK reposition/reorder them instead. - Add a `portOptions` callback for per-port ELK layout options, alongside the existing `nodeOptions`/`edgeOptions`. - Default to `elk.hierarchyHandling: INCLUDE_CHILDREN` so edges crossing a container's boundary are accounted for during layout. - Migrate the `layout-elk-ts` example to the package's `layout()` entry point instead of hand-rolled ELK export/import.
A fixed (non-random) system diagram exercising the new @joint/layout-elk capabilities: three containers, eight services connected through ports (three of which opt into `positionPorts` so ELK orders them to minimize crossings), and `elk.aspectRatio`/`elk.layered.wrapping.strategy` tuned to keep the drawing within a soft 1000-unit width budget.
The package itself (README, build config, initial layout()) isn't covered by an earlier changeset - this PR is its first appearance on master, so the changelog should read as an introduction, not a bump.
It named the first rotated element it happened to meet, which reads as if that one element were the problem - there may be twenty, and fixing the named one just moves the warning to the next. It also described the mechanism at length without saying what to do about it.
- A cell's id may be a number, so `if (!parentId)` read an element embedded in a container with `id: 0` as having no parent at all: the child was laid out as a top-level node and the container as a leaf, with the embedding silently gone. - The rotation warning ran before `exportElement` could drop the element, so it fired on every layout even once the user had excluded the rotated elements it told them to exclude. It now reports only an element that is actually laid out. - `buildEnd()` wrote an element-origin anchor onto any end ELK had anchored on the node. If the end gained a port while the layout was running, `topLeft` with `useModelGeometry` resolves against that port's bbox instead of the element's, so the delta landed the end somewhere else entirely. Such an end is skipped again - which it could not be before links on a dropped port were left out of the layout, since that was the case the skip used to break.
A container went to ELK as `0` x `0`, on the grounds that ELK sizes it from its content anyway. ELK does - but it does so whatever size the node is given, so the placeholder bought nothing, and it had to be undone again whenever `exportElement` dropped every element inside a container, turning it into a leaf. That restore only fired when both dimensions were still `0`, so a container with one dimension set in `exportElement` and all of its embeds dropped was laid out as `height: 0`: ELK reserved no vertical room for it and the next element sat 20px inside it, while the element still drew at its real height. Sending the element's own size removes the placeholder, the restore and the bug together. Verified against elkjs that a node with children comes back the same size whether it was given `0` x `0`, its real size, or no size at all.
…mputed The anchor's delta came from `getRelativePointFromAbsolute()`, which measures from the element's current position. That is the laid out one only once `setElementAttributes` has written it, and a callback is free not to - the README suggests redirecting the result into a `transition()`, and the rectpacking example does. Until the transition settles the element is still where it was, so every end ELK anchored on an element came out short by exactly how far the element then moved: an element going from (400,400) to (12,12) left its link ends 388px adrift. `importNode` already computes the graph-absolute rect for every node, and it recurses into a node's children before importing the edges filed under it, so both ends of an edge are known by the time it is read back. The delta is measured from that rect instead, which no longer depends on when - or whether - the model catches up. The rotation is done around the same rect's centre, as `getRelativePointFromAbsolute()` does around the model's. Adds nine tests for this and for the rest of the audit: links to links, a port the element no longer has, a label's other position settings, the size `exportElement` set on a container whose embeds it dropped, a cell listed twice, a container in no graph, the batch over the graph its cells are in, and two ids differing only in type.
…own class
`WorkerElk` is exported, its `layout()` takes a `{ signal }`, and its doc promises
the worker is terminated when the layout is aborted. The signal only ever reached it
behind `elk instanceof ElkWorkerClient`, so an implementation of the interface that
is not literally our class - a caller's own, or `createWorkerElk()`'s from a second
copy of the package in one bundle - fell through to `abortable()`: the abort
discarded the result while the worker kept computing.
Checked for `terminate()` instead, which `elkjs` does not have. That also lets the
class be imported as a type only, rather than pulled into the bundle for a type
check, and narrows the other branch to `ELK` without an assertion.
…sizes its node A port's imported position is the center ELK gave it, moved onto the border of the node ELK laid out - which is the node's own size, not the element's, whenever `exportElement` handed ELK a size of its own. That is the frame the caller asked for, and a port rendering outside a smaller element follows from the size they declared rather than from the import getting it wrong. Worth a test because the alternative looks defensible until it is measured: clamping to the element's size binds on the along-the-side axis too, so every port on that side collapses onto one corner.
`ElkNodeDraft.labels` is there so ELK can size a node around labels the element does not itself carry, but the drafts went to ELK untouched - and ELK skips a label with no text, which is why `buildPorts()` and `buildEdge()` both stamp one on. So the documented use did nothing, and `ElkLabelDraft` has no `text` for a caller to supply either. A node label now gets the same placeholder: the caller sizes the label, the package makes ELK count it. The doc comment pointed at `elk.nodeSize.constraints: 'NODE_LABELS'` on its own, which collapses the node to 0x0 whether or not it has labels. It now says what else that option needs to work.
…ing one ELK id A port id may be a number, and `5` and `'5'` are two different ports - `ElementView` tags a port element with `port-id-type` precisely so the two survive a round trip through the DOM. The export stringified the id for ELK and handed the same string back to `portProp()`, which looks a port up with `===`: the export half resolved through the loose `portsMap` and succeeded, the import half threw, and the layout was left half-applied. The id now goes back exactly as it came, and the two `hasPort()` calls ask with it rather than with a string. Two such ports on one element also produced two ELK ports with the same id, with the second overwriting the first in the lookup, which is the case elements and links already reject. Ports do too now. See clientIO#3544 for the `PortData` index that collapses them in the first place.
…ring of it
`elementIndicesById` and `childElementsByParentId` keyed on `` `${id}` ``, so an
element whose `parent` is the number 5 was nested inside a container whose id is the
string `'5'` - a different cell, which `CellCollection` keeps apart in a `Map`. The
element ended up laid out inside a container it is not in, silently and with no
error.
Both maps are keyed by the id itself now. What is still stringified is the ELK id,
which is a string by definition.
…uild when core widens Nothing in the package was waiting on clientIO#3544 - a port id is passed through as it came, which works against both the loose index of today and the strict one proposed there. That left no signal: if the core fix never lands, or lands and nobody revisits this package, there is nothing to notice. `ElkGraphPort.portId`, `SetPortAttributesCallbackParameters.portId` and `getElkPortId()` now say `dia.Cell.ID`, which is what a port id really is. That meets `@joint/core`'s `portProp(portId: string)` in exactly one place, and the `@ts-expect-error` there stops being needed - and so becomes an error - the moment those typings are widened. Verified by widening `portProp()` locally: the build fails with TS2578 on that line. The other three hand-offs need no directive: `getPortRelativeRect()` and both `hasPort()` calls are given values the typings already consider strings, so there is nothing to suppress.
Every port is written back with the position ELK returned, which under the default `elk.portConstraints: 'FIXED_POS'` is the one it was given - the layout then records as fixed args what the port's group layout had computed. That is the right default here, since ELK owns the link geometry and any change after a layout means laying out again, but a caller who wants to treat the two cases differently had no way to tell them apart. `hasMoved` says whether the position that came back differs from the one that was sent. `ElkGraphPort` keeps the sent top-left to compare against.
`portsById` is built when the graph is exported and holds the element directly, so a port removed during the layout still resolves when the result comes back - and `portProp()` throws for a port its element no longer has. The import stopped part-way: the elements it had already reached kept their new positions, the rest did not, and `layout()` rejected with `Element: unable to find port with id ...`. The element and link paths both tolerate the graph changing under them - `buildEnd()` re-reads the end, `importEdges()` checks the label is still there. Ports do now too. The check is in the loop rather than in the default callback, so a custom `setPortAttributes` is not handed a port that no longer exists either.
The import clamped every port's center into the node's bounds. That is two operations written as one: ELK gives a port's box top-left and JointJS wants its center, which is `+ size / 2`; and ELK draws that box beyond the border it sits on, which has to be undone. Written as a clamp, the second applied to every port - so a port a group puts outside the element on purpose (`args.dx`, `ellipseLayout`'s `dr`) was dragged onto the border, 110 to 100 for a 10px offset. The geometry alone cannot tell the two apart: a port ELK placed on the east border and a port pushed 10px out both come back with their box starting at the border. What separates them is whether ELK moved the port at all, so `toPortCenter()` undoes the convention only then, and a port handed back untouched keeps its center. That also leaves `elk.port.borderOffset` intact for anyone setting it through `exportPort`: the shift is half the port, not everything down to the border.
The import applied a size only to a container, on the grounds that a leaf keeps its own. But ELK resizes leaves too - `rectpacking`'s white space elimination stretched five of six tiles in a six tile board, and `elk.nodeSize.constraints` does the same - and all of it was discarded unless the caller wrote a `setElementAttributes` of their own to read it back off `elkNode`. The size now comes through whenever what ELK returned differs from what it was given, which `ElkGraphElement` records alongside the element. That covers the container case as a consequence rather than as a special case, and leaves alone a node whose size was raised through `exportElement` purely to shape the layout: ELK hands that back untouched, so the element keeps its own size as before.
`setElementAttributes` ran every element's position and size through `element.transition()`, which walks the model frame by frame and fires a change on each one. The layout already has the answer, so the model can take it in one write and the browser can do the animating: an element's position is a `transform` on its group and a tile's size reaches its `rect` as `width`/`height`, both of which CSS interpolates. The callback itself is no longer needed either - `attributes.size` now arrives whenever ELK resized a tile, which is what it was there to recover.
… against the sent one `1a3f2a1dd` applied a size when ELK returned something other than what it was given. That leaves a third value out of the comparison: what the element itself currently has, which is different again whenever `exportElement` sends a size of its own. The rectpacking example does exactly that - it resets every tile to `baseSize` before export - so turning white space elimination off sent the base size, got the base size back, and the tiles stayed stretched from the run before. The size ELK laid the node out at is the one the element should end up with. Compared against the element's own size, so a node that already has it is not written again. The two overriding cases cannot both be served: one sends a size and wants the result applied, the other sends a size only to shape the layout and wants its element left alone. ELK echoes both, so nothing tells them apart. The second writes a `setElementAttributes` that ignores `attributes.size`.
One test packs four tiles with `rectpacking`, resetting each to its base size in `exportElement` the way the example does, turns white space elimination on and off again, and asserts the tiles stretch and then come back. That is the case the sent size comparison got wrong: `exportElement` sends the base size either way, so ELK returning it unchanged is not the same as the element already having it. The other asserts that `attributes` carries no size for a node ELK laid out at the size it already has. It started out asserting that no `change:size` fired, which passes either way - `mvc.Model.set` compares with `isEqual`, so an identical size emits nothing whether it is written or not.
`33935fad0` gave every node the element's own size, on the grounds that ELK computes a container's size from its content whatever it is given. That holds for `layered`, which is all it was checked against. `rectpacking` keeps the size it is given: a container sent its own size stays at whatever it grew to on an earlier layout and never shrinks back, so shuffling the rectpacking example left every folder with the dead space its tiles no longer needed. A container goes in at `0` again. What that commit was reaching for - the leaf case, where `exportElement` drops every embed and the node needs its real size back - is kept, and fixed per dimension rather than all or nothing, which was the bug it set out to solve. The comment it removed explained this, bar one detail: the size is `0` rather than absent because `ElkNodeDraft` types it as required, not because elkjs demands a number. It says so now, and a test holds it: an oversized container, laid out with `rectpacking`, has to come back fitted to its tiles.
`getBBox()` took the union of the top-level nodes, which leaves out edge routes and edge labels - ELK reserves room for both, so a layout with a label wider than its nodes reported a box the layout does not fit in. ELK already sizes the root node around all of it, plus the root's own padding. The box is now that size with the padding taken off, the way `@joint/layout-directed-graph` and `@joint/layout-msagl` take off the margins they were given. `elk.padding` names the sides it sets and the rest keep ELK's default of `12`.
A tall link label is placed beside the nodes it runs between, so the layout extends past their union - which is what the bounding box used to be.
`portsById` kept the top-left read from `getPortRelativeRect()`, which is taken before `exportPort` runs - while the port handed to ELK carries whatever that callback set. For a port the callback moved, the two disagree. `importLayout` compares them to tell a port ELK placed itself from one that was put somewhere on purpose and handed back untouched: the first has its center moved onto the border, the second is left alone. The stale value made every moved port look placed by ELK, so it was pulled half a port inwards on each layout - a port stood off the element by `exportPort` under `FIXED_POS` drifts back into it.
…Cell.ID` `exportPort`/`exportPortLabel` typed `portId` as a string, while the value passed is whatever `@joint/core` holds - a port id may be a number, and `5` and `'5'` are two different ports. `setPortAttributes` and `ElkGraphPort` already say so. Passing it back to `@joint/core` needs the same `@ts-expect-error` the package carries elsewhere, so the containers-and-ports example grows two, keyed to clientIO#3544 like the rest. Its `exportPortLabel` also destructured `label/size` unguarded, which a port with no label text never has - it now goes to ELK without a label instead.
Every `-js` demo and the newer TypeScript ones already run on vite - 53 of the 72
examples - while these four were still on webpack. Per demo that was a 45-line
`webpack.config.js` and eight devDependencies; it is now no config file and three.
Nothing in those configs was specific to the demos: `resolve.fullySpecified: false`
is there for webpack's handling of extensionless `.mjs` imports, and
`publicPath: 'auto'` for the ELK worker, which vite's
`new Worker(new URL(..., import.meta.url), { type: 'module' })` form resolves on its
own. TypeScript goes through esbuild, with `tsc` kept in the build script for the
type checking, and `.scss` needs only `sass`.
The README described a `setElementAttributes` callback the demo no longer has: the package applies every size ELK computed, so a stretched tile is resized without one.
…arams` `Params` is what `@joint/react` calls these objects, and it does not read like TS's own `Parameters<T>`, which means something else.
`layout-elk-ts` and `layout-elk-default-ts` were both generic, and neither said which one to open first: the plain-looking one lays out a package dependency graph and is the largest of the four, while the "default" one is the no-callbacks starting point. They are now `layout-elk-dependency-graph-ts` and `layout-elk-defaults-ts`. `layout-elk-containers-ports-ts` becomes `layout-elk-containers-and-ports-ts`, the way the other examples join two subjects (`element-tags-and-badges-js`).
`@joint/core` now types a port id as `dia.Cell.ID`, so `hasPort()` and `portProp()` take a number as they always did at runtime.
kumilingus
force-pushed
the
fix/layout-elk-audit
branch
from
October 12, 2026 19:00
38f716f to
96f3406
Compare
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
2 participants
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.
Supersedes #3497 - it is that branch plus 33 commits of fixes and API changes from an audit of the package. The test suite grows from 11 to 76 tests.
API
layout()takes a graph or a list of cells, like@joint/layout-directed-graphand@joint/layout-msagl, instead of{ graph, elements, links }.batchNameoption - a layout always batches aslayout.exportPortdropped is left out of the layout instead of being routed to the node.setPortAttributesis told whether the port moved.exportElementcan size a node around its labels.Fixes
sources/targets; an end on another link no longer resolves to the far element; an anchor is measured from the position ELK computed, including on a rotated element.parent()rather than from the graph, and keyed by the cell id instead of a string of it - so it also works for a list of cells, and for ids that are numbers.5and'5'are two different ports), a port removed mid-layout is skipped, a port the layout did not move is left where it is, and the top-left recorded for a port is the oneexportPortleft.@joint/coretypes a port id as a string, so every call handing one back to it carries a@ts-expect-errorkeyed to dia.Element: ports whose ids differ only in type are not kept apart by PortData #3544 - those fail, and are to be deleted, once that issue widens the typings.bboxcovers everything ELK laid out - edge routes and labels included - rather than the union of the top-level nodes.AbortSignalis handed to anyWorkerElk, not only to the one this package creates.The rectpacking example animates with CSS transitions instead of
transition().