Skip to content

Refactor: Suppressed typescript errors clean up in packages\ketcher-core\src\application\render\renderers\TransientView\AngleSnapView.ts file #10902

Description

@AlexeyGirin

Problem:
Typescript errors suppressed by // eslint-disable-next-line @typescript-eslint/ban-ts-comment and // @ts-ignore directives

Why is this an issue?
// @ts-ignore
This is a TypeScript directive.
It tells the TypeScript compiler:

Don't show TypeScript errors for the following line.

eslint-disable-next-line @typescript-eslint/ban-ts-comment
The ESLint rule is enabled in Ketcher:
@typescript-eslint/ban-ts-comment
It prohibits or limits the use of comments such as:

// @ts-ignore
// @ts-nocheck
// @ts-check
// @ts-expect-error

The result is a rather typical construct:

ESLint: "Don't use @ts-ignore"
Developer: "Ignore this ESLint rule"
TypeScript: "There's an error here"
Developer: "Ignore this TypeScript error"

In other words, this is double suppression of the check.

These are two directives that essentially say, "TypeScript complains about this symbol, but we're consciously ignoring this error, and we ask ESLint not to complain about us ignoring it."

A single @ts-ignore isn't necessarily a disaster. Sometimes it's actually necessary—for example, when working with a poorly typed legacy library.
The problem is that it suppresses any TypeScript error on the next line.

Let's say the original was:

// @ts-ignore
legacyApi.doSomething(foo);

Because legacyApi has incorrect typings.
Six months later, someone changes the code:

// @ts-ignore
legacyApi.doSomething(nonExistingVariable);

TypeScript could have detected a new, real error, but @ts-ignore continues to suppress it.
This is one of the main reasons why @ts-ignore is considered deprecated.

How can it be fixed?
It's generally better to use @ts-expect-error
In modern TypeScript, the following is preferable:

// @ts-expect-error
legacyApi.doSomething(foo);

The difference is very important.

@ts-ignore says:

Whether there's an error or not, I don't care. Don't show it.

@ts-expect-error says:

I know the next line should contain a TypeScript error.

If the error ever goes away, TypeScript will automatically report:

Unused '@ts-expect-error' directive.

For example:

// @ts-expect-error
const x: number = 'hello';

is valid from the perspective of @ts-expect-error – the error does exist.

But after the fix:

// @ts-expect-error
const x: number = 10;

TypeScript will say:

Unused '@ts-expect-error' directive.

This helps remove unnecessary workarounds.

Even better, fix the typings.

For example, if Ketcher encounters something like this:

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
window.ketcher.editor.someMethod();

Then my first option would be to fix the typings, not replace them with @ts-expect-error.

For example:

interface Window {
  ketcher: Ketcher;
}

or a correct interface extension:

declare global {
  interface Window {
    ketcher: Ketcher;
  }
}

Then suppression isn't required at all.

Problem locations:
packages\ketcher-core\src\application\render\renderers\TransientView\AngleSnapView.ts

Metadata

Metadata

Labels

refactorCode refactoring, without changing the functionality

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions