[dagre] Dagre graphlib types are wrong

[Timmmm]

• I tried using the @types/xxxx package and had problems.
• I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript
• I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).
• Mention the authors (see Definitions by: in index.d.ts) so they can respond.
  • Authors: @qinfchen, @Frankrike, @vilterp, @rustedgrail, @GrahamLea, @danvk, @rebelwolfson

Dagre uses Graphlib, and has a copy of Graphlib's type definitions. If you compare Graphlib's type definitions with Dagre's copy, you can see that they are very different.

As far as I can tell the Dagre copy is completely wrong (or out of date). For example:

        node(id: string | Label): Node<T>;

You can't look up a node by its label (which is an arbitrary Record<string, unknown>). Graphlib's types have this correct:

    node(name: string): any;

(well, ignoring the fact that it returns any rather than Record<string, unknown>)

[GrahamLea]

I'll ask the obvious question: Should Dagre have a copy of Graphlib's types at all? Or should it import them?

[Timmmm]

I suspect they copied them so that they could require extra types on the nodes. As far as I understand it Graphlib is a general purpose graph library so nodes can have any properties, but Dagre is a layout algorithm so it needs some properties to exist (width, heigh, etc.)

I think the thing that most makes sense is to change Graphlib to have a generic type parameter for the labels/nodes (which can default to Record<string, unknown> or Record<string, any> if you like), and then have Graphlib use that. Something like this:

declare module "graphlib" {
...

    export class Graph<Label = Record<string, any>> {
...
        setNode(name: string, label?: Label): Graph;
        node(name: string): Label;

And

import * as graphlib from "graphlib";

export interface GraphLabel {
    width?: number;
    height?: number;
    compound?: boolean;
    rankdir?: string;
    align?: string;
    nodesep?: number;
    edgesep?: number;
    ranksep?: number;
    marginx?: number;
    marginy?: number;
    acyclicer?: string;
    ranker?: string;
}

export type Graph<UserLabel = {}> = graphlib.Graph<UserLabel & GraphLabel>;

Does that make sense?

[FunkMonkey]

There are already some updated type definitions in this PR: dagrejs/graphlib#98

Unfortunately graphlib doesn't seem to have an owner, as such they may not be merged into the graphlib npm package any time soon...