typescript-eslint

[tbo47]

No description provided.

[tbo47]

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]

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.

[aloisklink]

The npm lint action still has a bunch of errors.

Ideally, I'd like to make it pass, so we can add this back to the https://github.com/tbo47/dagre-es/blob/main/.github/workflows/test.yml file.

We could make any rules that are currently having an error change into a warning instead if it's too much work to fix them.

Also, is there any chance you can make any automatic changes (e.g. when you run npm run lint -- --fix) in a separate commit? It makes PRs much easier to review, since instead of having to review 1000s of lines of code, I can then know I can just ignore most of them, since they're automatically generated!

[aloisklink]

Instead of changing the file completely, like what you've done here, I'd prefer just adding the plugin:@typescript-eslint/recommended. I've also disabled the 'import/no-unresolved' rule since it doesn't seem to work, and TypeScript should check this for us anyway:

module.exports = {
  env: {
    browser: true,
    es2021: true,
    node: true,
  },
  extends: ['eslint:recommended', 'plugin:import/recommended', 'plugin:@typescript-eslint/recommended'],
  parser: '@typescript-eslint/parser',
  overrides: [
    {
      files: ['**/*.test.js', 'test/**/*.js'],
      env: {
        // technically, we are using vitest, but that's pretty similar to jest
        jest: true,
      },
      settings: {
        'import/ignore': [
          // for some reason, `import {it} from "vitest";` throws an error
          /node_modules\/vitest\/dist\/index\.js$/.source,
        ],
      },
    },
  ],
  parserOptions: {
    sourceType: 'module',
  },
  rules: {
    'import/no-cycle': 'error',
    'import/no-unresolved': 'off', // TypeScript's Node16 rule will check this for us
    // make sure that all files have an extension (required by ESM)
    'import/extensions': [
      'error',
      'always',
      {
        js: 'always',
        jsx: 'never',
        mjs: 'always',
      },
    ],
  },
};

[aloisklink]

I can understand removing .vscode, if you want to share your editor's config, but we probably want to still ignore *.d.ts and *.tgz files.