eslint-visitor-keys
eslint-visitor-keys copied to clipboard
feat: make types more specific
When looking a bit into https://github.com/eslint-community/eslint-utils/issues/150 I felt that the types of this module, which is used by that module, was quite generic – so I stepped into that rabbit hole and looked into if it could be made more tightly typed, and this is the result.
Is it strictly needed for https://github.com/eslint-community/eslint-utils/issues/150 ? Not really.
Would it be a waste to throw it away just because of that? Yes, I think so.
Exports
KEYSis now typed with exactly the structure that it has, thanks to using@satisfyand@type {const}. So if I accessKEYS.ArrayPatternit has the type ofreadonly ["elements"]getKeys()is now typed so that it returns a filtered union of string literals, based on the keys of the object sent in, rather than always justreadonly string[]. It still returnsreadonly string[]when given an input that's egRecord<string, any>unionWith()now uses the extendedVisitorKeysto returnVisitorKeyswith the keys set to union of the keys inadditionalKeysandVisitorKeyTypes(the keys ofKEYS). It still returns plainVisitorKeyswhen given a generic input likeVisitorKeys
Types
VisitorKeysis now a generic that can be used to create a more narrow type, likeVisitorKeys<VisitorKeyTypes, import("estree").Node | import("estree-jsx").Node>would roughly be the same as whatKEYShas (but less precise since keys gets all property types). Without any parameters given it works likeVisitorKeysdid previously.- Two new types:
VisitorKeyTypes, that represents all keys fromKEYSFilteredKeysOf<T>, the type equivalent to the filtering thatgetKeys()does
Additional notes
- I replaced the
readonlykeyword in all places where it causes linting troubles (https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/164). It got replaced with eitherReadonly<>orReadonlyArray<>. - Two new challenges with the jsdoc-type-pratt-parser arose though, so there are still some
eslint-disable jsdoc/valid-types- It can't parse template literal types
- It can't parse conditional types
- I had to update the
typescript/tsdandrollupmodules to get it to eg. recognize thelib: ['es2022']in thetsconfig.json
Closing notes
I realize that some of the changes proposed here might be seen as a bit extreme when it comes to juggling TypeScript types in JSDoc definitions – and if some are considered too extreme I can try to pull them out. But I think it's nothing too crazy in there.
Interesting. This seems like overkill for such a simple package. I'm not sure how valuable these changes are based on the additional code that, honestly, makes it harder for me to read. Leaving open to see what the rest of the team thinks.
I'm happy to go through and provide alternatives, justification and/or try to drop parts wherever you are interested.
I do agree this PR may have gone on the overkill-side part of things, so we can for sure scale back.
I'm also not sure how useful is this. KEYS and other exports are meant to be used in generic traversals, so generic types like string[] seem right for this purpose.
Closing as we decided not to move forward with this.