search

tbo47 / dagre-es

The dagrejs library exported as ES modules.
Other
32 stars 14 forks source link

typescript-eslint #29

Closed tbo47 closed 1 month ago

tbo47 commented 11 months ago

I tested my changes with my small project https://github.com/tbo47/dagre-d3-es-demo but it's probably not enough.

How can we test more? @aloisklink

By releasing a rc version? Or just testing the main branch for a while?

Can mermaidjs run tests before I release the recent changes?

aloisklink commented 11 months ago

How can we test more? @aloisklink

I've tested this by doing:

git clone https://github.com/mermaid-js/mermaid
cd mermaid
pnpm install # works fine
pnpm add --filter mermaid https://github.com/tbo47/dagre-es.git #FAILS

There are hundreds of errors like packages/mermaid/node_modules/dagre-d3-es/src/dagre-js/intersect/intersect-line.ts(7,24): error TS7006: Parameter 'p1' implicitly has an 'any' type. which I don't understand, since the dagre-d3-es types are pretty similar to the existing ones.

Maybe it's something wrong with mermaid's configuration? I'm not really super experienced with TypeScript, so I can't say for sure.

One thing I did notice is that the new types seems to be a bit less defined than the old types. For example, the src/dagre-js/intersect/intersect-line.d.ts file (back when we were generating types from the .js files) looks like:

export function intersectLine(
  p1: any,
  p2: any,
  q1: any,
  q2: any
): {
  x: number;
  y: number;
};

But the new file at dist/dagre-js/intersect/intersect-line.d.ts instead has x and y as any instead of number:

export { intersectLine };
declare function intersectLine(p1: any, p2: any, q1: any, q2: any): {
    x: any;
    y: any;
};

Which is weird, because you never actually changed anything in the https://github.com/tbo47/dagre-es/blame/main/src/dagre-js/intersect/intersect-line.ts file, all you did was rename it.