cosmjs icon indicating copy to clipboard operation
cosmjs copied to clipboard

enabling lints

Open dynst opened this issue 5 months ago • 2 comments

These lints are recommended by the eslint TypeScript plugin but are currently disabled because they flag so many parts of the codebase.

  • [ ] https://typescript-eslint.io/rules/no-empty-object-type/
  • [ ] https://typescript-eslint.io/rules/no-explicit-any/
  • [ ] https://typescript-eslint.io/rules/no-misused-promises/
  • [ ] https://typescript-eslint.io/rules/no-redundant-type-constituents/
  • [ ] https://typescript-eslint.io/rules/no-unsafe-argument/
  • [ ] https://typescript-eslint.io/rules/no-unsafe-assignment/
  • [ ] https://typescript-eslint.io/rules/no-unsafe-call/
  • [x] https://typescript-eslint.io/rules/no-unsafe-enum-comparison/
  • [ ] https://typescript-eslint.io/rules/no-unsafe-member-access/
  • [ ] https://typescript-eslint.io/rules/no-unsafe-return/
  • [ ] https://typescript-eslint.io/rules/prefer-promise-reject-errors/
  • [ ] https://typescript-eslint.io/rules/restrict-template-expressions/
  • [ ] https://typescript-eslint.io/rules/unbound-method/

Which should be turned on someday, if any?

dynst avatar Jul 29 '25 20:07 dynst

Nice! I like no-unsafe-member-access and no-unsafe-enum-comparison

webmaster128 avatar Jul 30 '25 06:07 webmaster128

Currently 154 errors from running no-unsafe-member-access. 59 errors if you exclude unit tests and the file demo/web.ts.

Like it doesn't like obj[key] here (even though key is from the array of strings from Object.keys(obj):

https://github.com/cosmos/cosmjs/blob/461772f41231fe053fb0480faf6e19a5bcbfe65d/packages/amino/src/signdoc.ts#L47

Then in another spot, due to the design of the type, nobody can even use this value at all:

https://github.com/cosmos/cosmjs/blob/461772f41231fe053fb0480faf6e19a5bcbfe65d/packages/tendermint-rpc/src/rpcclients/rpcclient.ts#L15

grep 'readonly .* any' -rnI packages/ turns up similar types (a lot more than just these 4):

packages/amino/src/signdoc.ts:9:  readonly value: any;
packages/amino/src/pubkeys.ts:6:  readonly value: any;
packages/json-rpc/src/types.ts:15:  readonly result: any;
packages/proto-signing/src/registry.ts:80:  readonly value: any;

Maybe these APIs need to change any to Record if they never store a primitive like a number. But this one can't be changed right now, because it is currently allowed to return a string number or just null:

https://github.com/cosmos/cosmjs/blob/1b984119e37b388705cd1719f9b1fca4ab6bb0f7/packages/json-rpc/src/types.ts#L15

[@cosmjs/json-rpc]: src/parse.ts:144:5 - error TS2322: Type 'JsonCompatibleValue | readonly JsonCompatibleValue[]' is not assignable to type 'Record<string, any>'.
[@cosmjs/json-rpc]:   Type 'null' is not assignable to type 'Record<string, any>'.

So the json-rpc API would need to be changed. Are there any external users that care, though?

How many external facing APIs like this are there that need reform?

dynst avatar Jul 30 '25 07:07 dynst