eclipse-thingweb / node-wot

Components for building WoT devices or for interacting with them over various IoT protocols
https://thingweb.io
Other
161 stars 78 forks source link

Track of ESLint Ignored Rules #534

Open fatadel opened 2 years ago

fatadel commented 2 years ago

This issue is created to keep track of ignored eslint rules with respective justifications.

  1. File/Line: https://github.com/eclipse/thingweb.node-wot/blob/master/packages/td-tools/src/thing-description.ts Ignored Rule: no-use-before-define Reason: Many circular dependencies where interface/type/class A uses interface/type/class B in its definition and interface/type/class B uses interface/type/class A in its definition.

  2. File/Line: https://github.com/eclipse/thingweb.node-wot/blob/master/packages/td-tools/src/thing-description.ts Ignored Rule: eslint-disable-next-line @typescript-eslint/no-explicit-any Reason: TD class should be replaced by autogenerated one (see https://github.com/eclipse/thingweb.node-wot/issues/529)

  3. CoAP, see https://github.com/eclipse/thingweb.node-wot/pull/588/commits/9270d48e9ce12a16f5be77dd9025f797bd03765d

  4. Netconf, see disabled warnings in https://github.com/eclipse/thingweb.node-wot/pull/818

relu91 commented 2 years ago

from @danielpeintner in #589

Note: I had to disable rules.. I did not manage to solve them ... for example not using any

cli.ts // eslint-disable-next-line @typescript-eslint/no-var-requires // eslint-disable-next-line @typescript-eslint/no-var-requires

cli-default-servient.ts // eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types

relu91 commented 2 years ago

From the PR linked right above: packages/binding-mbus/src/mbus-connection.ts

// eslint-disable-next-line @typescript-eslint/no-var-requires
const MbusMaster = require("node-mbus");
JKRhb commented 2 years ago

If I see it correctly, we are targeting ES6 with the TS transpiler at the moment, right? Then I think we could actually disable the rule no-use-before-define completely which apparently is only relevant if a JavaScript version prior to ES6 is used at the transpile target (see https://eslint.org/docs/rules/no-use-before-define).

JKRhb commented 2 years ago
3. CoAP, see [9270d48](https://github.com/eclipse/thingweb.node-wot/commit/9270d48e9ce12a16f5be77dd9025f797bd03765d)

This point has actually been resolved in the meantime, by the way :)

danielpeintner commented 2 years ago

This point has actually been resolved in the meantime, by the way :)

I have striked through it

danielpeintner commented 2 years ago

If I see it correctly, we are targeting ES6 with the TS transpiler at the moment, right? Then I think we could actually disable the rule no-use-before-define completely which apparently is only relevant if a JavaScript version prior to ES6 is used at the transpile target (see https://eslint.org/docs/rules/no-use-before-define).

ES6 dates back to ECMAScript 2015. Hence I think this is okay.

@relu91 any opinion?

BTW, I noticed that in .prettierrc.json we still use es5. Shall we change that too? see https://github.com/eclipse/thingweb.node-wot/search?q=ES5

relu91 commented 2 years ago

ES6 dates back to ECMAScript 2015. Hence I think this is okay.

Good point, but I think is still a good practice to define stuff before using them (i.e. classes types etc. ). So I would be ok for leaving the rule as it is, but I would not stop you if you think differently :)

BTW, I noticed that in .prettierrc.json we still use es5. Shall we change that too? see https://github.com/eclipse/thingweb.node-wot/search?q=ES5

Yeah, I think we should.

JKRhb commented 2 years ago

ES6 dates back to ECMAScript 2015. Hence I think this is okay.

Good point, but I think is still a good practice to define stuff before using them (i.e. classes types etc. ). So I would be ok for leaving the rule as it is, but I would not stop you if you think differently :)

Oh, yeah, you are right, I meant only typescript related definitions with the no-use-before-define rule :) I just noticed that there is a typescript-specific rule in @typescript-eslint that can be used instead of the default so we can actually get rid of the false positives (see here). I would open a PR for this shortly :)