Repository navigation
feat(layout-elk): accept JointJS-shaped layout options, with a derived native type - #3547
Merged
Geliogabalus merged 28 commits intoOct 15, 2026
Merged
Conversation
…S shapes Every option is declared in its JointJS spelling: a number option as `number`, a boolean one as `boolean`, an `ElkPadding`/`ElkMargin` one as `dia.Sides` and a `KVector` one as `g.PlainPoint`. The string spellings are projected back from these in the next commit, so each option stays declared in one place. This commit does not compile on its own - the index signatures still read `string | undefined` and the `dia`/`g` imports are not in yet.
Each option now takes either spelling, per option, so one object can mix them:
`{ 'elk.spacing.nodeNode': 6, 'elk.padding': '[top=12,left=12]' }`. The string
form stays legal - it is what ELK's own reference pages use, and what options
arriving as data are already shaped like.
`Strict*ElkLayoutOptions` holds an option to the JointJS spelling alone and is
where every option is declared; `Native*ElkLayoutOptions` is projected from it
onto ELK's string spelling. The five existing names keep their meaning and take
both. Opting into the strict form is an annotation in your own code, not a flag
this package switches on.
…or ELK Takes an ELK graph whose layout options are in either spelling and returns one whose every option value is the string ELK takes - the node's own options, its labels', its ports' (and theirs), the edges filed under it (and theirs), and each of its children, as deep as they nest. A new graph: the options may be an object an export callback took straight from a cell's own data, or a shared - even frozen - constant, and neither is this package's to rewrite. `layout()` calls it after `exportGraph()`, so it sees what the export callbacks left behind, and its result goes to ELK with no assertion at the call site. A padding is read back in either spelling for the bounding box, which also fixes `'[top=40]'`: ELK reads a side its padding string does not name as `0` rather than as the option's `12`, where this had kept the default on every unnamed side.
`yarn test-ts` type-checks `test/types/` under the package's own flags, so the assertions are held to what a user gets. It runs as part of `yarn test`, which is what CI runs. What they pin is what fails silently otherwise: the branch order in `ToNativeElkOptions` (testing `dia.Sides` before `number` would degrade every number option to a plain string) and its terminal branch (without it the native variant's index signature is the strict one, and the type is not native at all).
The rectpacking demo no longer casts a range input to `` `${number}` ``,
stringifies a checkbox, or assembles a padding string by hand - `FOLDER_PADDING`
now goes to ELK as it is. The other three drop the quotes from their numbers and
booleans, and with them the comments explaining that a number goes as a string.
Also drops two `@ts-expect-error` directives the containers-and-ports demo no
longer needs: `@joint/core` types a port id as `dia.Cell.ID` now, so `tsc` had
been failing on the unused directives. Unrelated to the option spellings - it is
only that nothing builds the examples in CI, so nobody had seen it.
…lkNode()` Says which options take a `dia.Sides` or a point, that a side a padding does not name is `0` rather than the option's own default, and that the strict spelling is an annotation in your own code rather than a flag on this package.
`@joint/layout-elk` has not shipped, so a bullet describing what changed since a version nobody has is noise in its first changelog - the pending "new package" changeset already covers it. An empty changeset keeps the PR check satisfied.
…ey are imported `NativeElkNode` was a second name for what `elkGraph.mts` already imported as `RawElkNode` - two words for one idea, which is what naming the function `toNativeElkNode()` was meant to settle in the first place. The import alias carries the name now, and the extra declaration is gone. `Raw` no longer appears in `src/`: "native" is ELK's own spelling throughout, for the option types and the graph types alike.
`ElkLayoutOptions` led with `Elk` while `NodeElkLayoutOptions` and its siblings led with the element kind, so the modifier order flipped between the graph-level type and the per-element ones - and `StrictNodeElkLayoutOptions` stacked three. They now read as "the layout options of an ELK node", matching `ElkNode`, `ElkPort` and `ElkExtendedEdge`, which already lead with `Elk`.
kumilingus
requested review from
Geliogabalus
and
a balanced review from Copilot
October 13, 2026 14:47
…e calls The example called `elk.layout()` on an `elk` it never introduced, and `layout()` is this package's own headline function - so it read as if it were calling that. `elk` also already means something else in this README: the instance you hand to `layout()` as an option. It now creates the instance, says that `elk.layout()` is elkjs's own method and that what comes back is an ELK graph rather than a laid out JointJS one, and introduces `exportGraph()`, which the example leans on and the README had never mentioned. Imports `elkjs/lib/elk.bundled.js` rather than `elkjs`, whose entry point is an API shim with no algorithm in it - `new ELK()` on it needs a worker factory and would have failed in a browser bundle. A type test holds the example's import form and call to that.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Full-ID padding produces an incorrect bbox, legacy public type names disappear, and the native return contract can be violated.
3 open findings
What changed in this PR
Adds JointJS-native ELK option values, converts them to ELK’s string format, and fixes padding-based bounding boxes.
Changes:
- Adds strict, native, and mixed option types.
- Adds
toNativeElkNode()with runtime conversion and tests. - Updates examples and fixes partial-padding bbox handling.
| File | Description |
|---|---|
.changeset/elk-options-unreleased-package.md |
Records the unreleased-package change. |
packages/joint-layout-elk/package.json |
Adds TypeScript tests. |
packages/joint-layout-elk/README.md |
Documents option spellings and conversion. |
packages/joint-layout-elk/src/elkOptions.mts |
Implements option conversion and padding parsing. |
packages/joint-layout-elk/src/export.mts |
Uses mixed option types in export drafts. |
packages/joint-layout-elk/src/index.mts |
Exports toNativeElkNode(). |
packages/joint-layout-elk/src/layout.mts |
Converts options before layout and updates bbox handling. |
packages/joint-layout-elk/src/types/elkEdgeOptions.mts |
Adds strict/native/mixed edge options. |
packages/joint-layout-elk/src/types/elkGraph.mts |
Adds native graph typing. |
packages/joint-layout-elk/src/types/elkLabelOptions.mts |
Adds strict/native/mixed label options. |
packages/joint-layout-elk/src/types/elkLayoutOptions.mts |
Adds strict/native/mixed root options. |
packages/joint-layout-elk/src/types/elkNodeOptions.mts |
Adds strict/native/mixed node options. |
packages/joint-layout-elk/src/types/elkPortOptions.mts |
Adds strict/native/mixed port options. |
packages/joint-layout-elk/src/types/index.mts |
Updates type exports. |
packages/joint-layout-elk/src/types/toNativeElkOptions.mts |
Defines native type projection helpers. |
packages/joint-layout-elk/test/index.js |
Tests conversion, bbox behavior, and workers. |
packages/joint-layout-elk/test/types/api.test-d.mts |
Tests public API types. |
packages/joint-layout-elk/test/types/assert.mts |
Adds type-level assertions. |
packages/joint-layout-elk/test/types/options.test-d.mts |
Tests projected option types. |
packages/joint-layout-elk/test/types/tsconfig.json |
Configures type tests. |
examples/layout-elk-containers-and-ports-ts/src/index.ts |
Uses native numeric options and current port IDs. |
examples/layout-elk-containers-and-ports-ts/src/shapes.ts |
Uses object padding. |
examples/layout-elk-defaults-ts/src/index.ts |
Uses numeric spacing values. |
examples/layout-elk-dependency-graph-ts/src/index.ts |
Uses native number and boolean values. |
examples/layout-elk-rectpacking-ts/src/index.ts |
Uses native option values and shared padding. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
It took `(elements, links, options, elkLayoutOptions)` - the arguments `layout()` happened to call it with, which made two things leak: - Splitting cells into elements and links is `splitCells()`'s job, and that is private. A caller with a list of cells had to reimplement it, including the part that is easy to miss: a cell listed twice becomes two ELK nodes sharing an id. It now takes a graph or a list of cells, like `layout()`, so anything `layout()` lays out can be exported - subsets included. - `elkLayoutOptions` is the exported graph's own root options, so it belongs in `ExportGraphOptions` rather than beside it. `LayoutOptions` inherits it from there now instead of declaring it a second time. `layout()` still merges its defaults into them; `exportGraph()` takes them as given, which its JSDoc says. `splitCells()` moves to `cells.mjs`, shared by both.
…ue handle
`exportGraph()` returned its bookkeeping as three `Map`s - `elementsById`,
`linksById`, `portsById` - and `importLayout()` took them back as three separate
arguments, so applying a result read:
importLayout(result, data.elementsById, data.linksById, data.portsById, opt)
The maps are real: ELK ids are built from cell ids, and a port's entry also
carries where it was when it was exported, which the result alone cannot say.
None of that is anyone else's business, though. They are now one opaque `cells`
handle, made in `exportGraph()` and read back in `importLayout()`:
const { elkGraph, cells } = exportGraph(graph, opt);
importLayout(await elk.layout(toNativeElkNode(elkGraph)), cells, opt);
`ElkGraphPort` is no longer public - it was exported only because `portsById`
leaked it - and `ImportLayoutOptions` is optional now, as `exportGraph()`'s is.
The README documents the three as a path of their own, and says what most people
should reach for instead: a custom `elk`, which is any object with `layout()` and
`terminate()`, and leaves `layout()` doing the export and the import.
…raphLayout()` `exportGraph()` and `importLayout()` shared no noun, so they did not read as the pair they are - and next to a function called `layout()`, "import layout" can be read as the layout settings rather than a computed result. Both now say what they carry: a graph's layout, out and back. `ExportGraphOptions` and `ImportLayoutOptions` follow.
`ElkGraphData` was named like the ELK graph types in `types/`, and `cells` reads
in JointJS as a list of models - it is neither. What it holds is the ELK graph
and what each of its ids stands for:
ExportedGraphLayout { elkGraph: ElkNode; index: ExportedCellIndex }
`importGraphLayout()` takes the index alone, which is all it reads - the exported
graph beside it is the pre-layout one, and having it in scope next to the result
only invites the question of which of the two gets applied.
`Elk*` now means one thing across the package: ELK's own shapes, all in `types/`.
…index `ExportedCellIndex` was a phantom - a symbol declared with `declare const`, so no object could ever satisfy it and the only way in or out was a double cast through `unknown`, at both ends. The symbol is a real one now, and the index carries it, so `CellIndex` genuinely is an `ExportedCellIndex`: building one needs no cast at all, and reading it back is an ordinary downcast to what it was built as. The symbol is not exported from the package, so an index still cannot be forged outside it, and it never reaches the worker - only the converted graph is posted.
`CellIndex` is the type now, with its maps in plain sight, declared beside the `exportGraphLayout()` that builds it and the `importGraphLayout()` that reads it. It replaces a branded `ExportedCellIndex` that hid the same three maps behind a second type, a symbol and a cast at each end - machinery to stop people reading fields a doc comment can tell them not to. `cells.mjs` is back to `splitCells()` alone.
- `getBBox()` read `elk.padding` under that key alone, while `toElkOption()` takes ELK's full option id too - so `org.eclipse.elk.padding` laid the graph out with the padding given and measured the box with the default. - `exportGraphLayout()` took `elkLayoutOptions` as given, where `layout()` fills this package's defaults in. One of those is `elk.json.edgeCoords`, which is not a preference: `importGraphLayout()` reads an edge's points as the root's coordinates, so a graph exported by hand and laid out had its links routed off by the container's origin - silently, and only on hierarchical graphs. Both now fill them in, through `withDefaultElkLayoutOptions()`. - `toNativeElkNode()` returned the node, port or label it was given when there was nothing to convert, so a graph whose ports carry no `layoutOptions` was shared with the caller - and ELK writes its coordinates into the graph it is given. It copies everything now, as its "a new graph" contract says. - `parseElkPadding()` read `[TOP=40]`, `[foo=1]` and `[]` as no padding at all and fell back to 12 a side, where ELK reads them as a padding naming no side - 0 a side. It also missed an uppercase exponent (`4E1`, which is what Java's own `toString` emits) and a leading `+`, both of which ELK takes. Each is covered: the hierarchical round trip fails without the second fix.
…c API `withDefaultElkLayoutOptions()` landed in `export.mjs`, which the barrel re-exports whole, so it became public by accident. It sits in `elkOptions.mjs` now, beside `getElkPadding()` and the defaults themselves - the barrel takes only `toNativeElkNode()` from there.
`exportGraphLayout()` and `importGraphLayout()` are no longer public, and nor are the types they passed between them - `ExportedGraphLayout`, `CellIndex`, `ExportedPort`. What the ids of an ELK graph stand for is this package's own bookkeeping: a map of them to cells, with each port's position as it was sent. There is no shape of that worth promising to anyone. Nothing is lost with them. To run ELK elsewhere - a Web Worker, a server, a cache - or to change the graph on its way there, pass an `elk` of your own to `layout()`; it is any object with `layout()` and `terminate()`. And to see the graph this package builds, that same `elk` is handed exactly what ELK is handed, with every option already a string - so it pastes into elk-live, which the pre-conversion graph `exportGraphLayout()` returned did not. The barrel lists what it exports one by one now, so that being public is a decision rather than a side effect of where something is declared.
`elkIds.mts` claimed they are "unique across the whole graph", and put that down to JointJS cell ids being unique across elements and links. That holds within one graph, and `layout()` also takes cells from several - so a node's id and an edge's can be the same string. Nothing reads one as the other: they are looked up apart, which is why the duplicate check is per kind. The comment says so now instead of claiming an invariant the escaping does not provide.
`layout()` split them for the graphs to batch on, and the export split them again for the elements and the links. That was the price of the export taking a graph or a list of cells like `layout()` does, so that a caller would not have to reimplement `splitCells()` - and the export is not public any more, so there is no caller to spare. It hands the graphs back with the rest now, and `layout()` splits nothing.
- The guard said to keep the runtime option lists in step with the types was not one. `new Set<Union>([...])` does not check that the literals cover the union, so a `dia.Sides` or point option added to the types and left out of the list compiled fine - and then went to ELK as an object, which ELK rejects outright. (The check that did exist was the other way round: a listed key that is not sides-valued.) There is a real exhaustiveness assertion now, and it names the option it is missing. - `elk.json.shapeCoords` is the twin of `elk.json.edgeCoords`: `importGraphLayout` adds each container's position as it walks down, so a node's has to be its parent's. It is ELK's own default, but it was neither pinned nor written down, and setting it to `ROOT` counts every container's origin twice - silently, and only on a graph with containers. Pinned beside `edgeCoords`, where the constraint can be read. - The README's recipe for looking at the graph passes an `elk` with a no-op `terminate()`, which is how an `elk` says it can be stopped - so `layout()` hands it the signal instead of ignoring a late result, and an abort then does nothing until ELK settles. Said so. - The per-element option types carried a copy of the graph-level doc example, citing `elk.spacing.nodeNode` on edges, ports and labels, which have no such option, and advertising sides and points on edges, which have neither. Each shows one of its own options now.
… not two The exhaustiveness assertion only looked at the graph's and the node's strict interfaces, so one declared on an edge, a port or a label could still be left out of the runtime list and reach ELK unconverted - which ELK rejects. Ports and labels do declare point options today. Verified in all three: an unlisted point option on a port or a label, and an unlisted sides option on an edge, each now name themselves in the error.
`elk.nodeLabels.placement`, `elk.portLabels.placement`, `elk.contentAlignment`, `elk.nodeSize.constraints` and `elk.nodeSize.options` are sets of enum values in ELK - every `ENUMSET` option elkjs knows. They were typed as a plain `string`, which checks nothing, and ELK silently ignores the whole value if any one of its values is unknown. elkjs takes a set only as one string of its values and throws on an array. Type them as readonly arrays of their enum in the strict spelling (the native one stays `string`), and write such an array out as its values separated by spaces, as padding and point values already are. The list of these options is checked against the strict interfaces at compile time. Their documented defaults were Java expressions (`NodeLabelPlacement.fixed`, `EnumSet.noneOf(SizeConstraint)`, ...); write them as values instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Geliogabalus
approved these changes
Oct 15, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
Closes clientIO/joint-plus#883.
@joint/layout-elkpassed ELK's layout options straight through, so every value had to be written as a string — ELK's wire format, but not what writing them looks like:Each option now takes the JointJS spelling as well, per option, so one object can mix them:
The string spelling stays supported. It is what the ELK reference page linked from every option uses, and what options arriving as data — a saved diagram, a backend's config — are already shaped like.
Types: authored strict, projected native
Each option is declared once, in
StrictElk*LayoutOptions, andNativeElk*LayoutOptionsis projected from it by a mapped type, so the two spellings cannot drift. The unprefixed names stay permissive and take both, which is whatelkLayoutOptionsis typed as.Opting into the strict form is an annotation in your own code, not a flag on the package:
Nativemeans ELK's own spelling throughout, for the option types and the graph types alike;NativeElkNodeis elkjs's node type under this package's name, which is what lets a converted graph reachelk.layout()with no cast at the call site.Rawno longer appears insrc/. Every option type leads withElk(ElkNodeLayoutOptions,StrictElkNodeLayoutOptions, …), where the per-element ones used to lead with the element kind while the graph-level one led withElk.The public surface
plus the option types, the callback types and the ELK graph types.
toNativeElkNode()is the function #883 asked for: it writes a graph's layout options out as the strings ELK takes, throughout — the node's own, its labels', its ports' (and theirs), the edges filed under it (and theirs), and each of its children, as deep as they nest. It returns a new graph; the one passed in is untouched.Building the ELK graph and applying a result back are not public. They were briefly, and the bookkeeping they pass between them — a map of ELK ids to cells, with each port's position as it was sent — is not a shape worth promising anyone. Nothing is lost by withholding them:
elkof your own tolayout(). It is any object withlayout()andterminate().elkis handed exactly what ELK is handed — every option already a string, so it pastes straight into elk-live. The pre-conversion graph the export used to return did not.The barrel lists its exports one by one now, so being public is a decision rather than a side effect of which file something is declared in. That is how a helper leaked into the API mid-branch.
Bugs fixed along the way
Found by reviewing this branch:
getBBox()ignored ELK's full option id.toElkOption()acceptsorg.eclipse.elk.paddingas well aselk.padding, butgetBBox()read the short key only — so the full id laid the graph out with the padding given and measured the box from the 12px default.elk.json.edgeCoords, which is not a preference: applying a result reads an edge's points as the root's coordinates. Without it, links were routed off by the container's origin — silently, and only on hierarchical graphs.toNativeElkNode()did not always copy. It returned the node, port or label it was given when there was nothing to convert, and ELK writes its coordinates into the graph it is handed — so a graph whose ports carry nolayoutOptionswas written through, against the function's own "a new graph" contract.parseElkPadding()disagreed with ELK three ways.[TOP=40],[foo=1]and[]are a padding to ELK that names no side — 0 a side — where this read them as no padding at all and fell back to 12. It also rejected an uppercase exponent (4E1, which is what Java's owntoStringemits) and a leading+.And one that predates the branch:
elk.paddingstring gave the wrong bbox.getBBox()seeded all four sides with12and overrode only the ones the string named, so'[top=40]'gaveRect(12, 40, w-24, h-52)where the correct box isRect(0, 40, w, h-40). This is also why the two spellings mean the same thing:normalizeSides({ top: 40 })is exactly what ELK does with'[top=40]', so no@joint/corechange was needed.Scope notes
elk.paddingandelk.margins(graph + node),elk.spacing.portsSurrounding,elk.nodeLabels.padding.elk.position(on nodes, ports and labels),elk.nodeSize.minimum,elk.port.anchor.elk.bendPointsandelk.junctionPointsare layout output, written by ELK's own JSON exporter.The conversion is key-aware:
'elk.padding': 20must become[top=20,…], because plain'20'is silently ignored by ELK and20is the most natural thing to write. The runtime key set is derived from the types, so adding adia.Sidesoption without listing it fails to compile.Unrelated fix, called out
examples/layout-elk-containers-and-ports-tscarried two@ts-expect-errordirectives for #3544 that went stale whenElement.PortIDlanded in #3546 —tscwas already failing there onmaster. Nothing builds the examples in CI, so it had not been seen. Removed, since otherwise "the examples type-check" is not a claim this PR can make.Verification
tsc,test-ts,eslintcleanyarn test-tstype tests, chained intoyarn test. Mutation-tested: reordering thedia.Sides/numberbranches in the projection trips the number assertions, dropping its terminalstringbranch trips the index-signature one — they are not vacuous. Likewise for the defaults: flippingelk.json.edgeCoordstoCONTAINERfails two tests.tscclean against the builtdist, and all four were converted to the new spelling.getBBoxpath its zeroed padding normally masks.One empty changeset: the package is unreleased, so this is part of its first release and the pending "new package" changeset already covers it. A bullet describing what changed since a version nobody has would be noise.