Forbid native TypeScript collection types#5598
Conversation
|
Following you can find the validation changes against the target branch for the APIs. No changes detected. You can validate these APIs yourself by using the |
| `type MyReq = Required<string>`, | ||
| `type MyPick Pick<integer,"something">`, | ||
| `type MyOmit = Omit<Record, "something">`, | ||
| `type MyDict = Dictionary<string, object>`, |
There was a problem hiding this comment.
The original test file had a bug...it included native types like Record, Partial, etc. in BOTH the valid and invalid arrays. This was contradictory and causing test failures.
I corrected the tests to properly reflect the rule's intent:
valid: Now uses spec-defined aliases (Dictionary) and allowed types (arrays, custom classes)invalid: Contains native utility types (Record,Partial, etc.) and collection types (Map,Set, etc.)
also added test coverage for the new collection types (Map, Set, WeakMap, WeakSet).
margaretjgu
requested review from
Anaethelion,
flobernd,
l-trotta,
picandocodigo and
pquentin
pquentin
left a comment
pquentin
left a comment
There was a problem hiding this comment.
Sorry, I realize I approved, but I think the code could be made a bit more maintainable. The Array suggestion can wait, but we should create an issue for it if we don't do it today.
validator/rules/no-native-types.js
Outdated
| const TYPES_TO_AVOID = ['Record', 'Partial', 'Required', 'Pick', 'Omit']; | ||
| const UTILITY_TYPES = ['Record', 'Partial', 'Required', 'Pick', 'Omit']; | ||
|
|
||
| const COLLECTION_TYPES = ['Map', 'Set', 'WeakMap', 'WeakSet']; |
There was a problem hiding this comment.
Can we please add Array to the list? It could also be a follow-up, as it generates 106 errors.
There was a problem hiding this comment.
I will do this in a follow up pr
validator/rules/no-native-types.js
Outdated
| const COLLECTION_TYPES = ['Map', 'Set', 'WeakMap', 'WeakSet']; | ||
|
|
||
| const TYPES_TO_AVOID = [...UTILITY_TYPES, ...COLLECTION_TYPES]; | ||
|
|
||
| const TYPE_SUGGESTIONS = { | ||
| 'Record': 'Use Dictionary instead', | ||
| 'Map': 'Use Dictionary instead', | ||
| 'Set': 'Use an array type instead (e.g., string[])', | ||
| 'WeakMap': 'Use Dictionary instead', | ||
| 'WeakSet': 'Use an array type instead', | ||
| }; |
There was a problem hiding this comment.
When trying to add Array to the list, I added it to TYPE_SUGGESTIONS and realized it had no effect. Can we please stop using TYPES_TO_AVOID and TYPE_SUGGESTIONS and only rely on TYPE_SUGGESTIONS? Perhaps that will enable us to provide more helpful suggestions than "Use spec-defined aliases instead". (There's no equivalent to Omit or Pick, as far as I know.)
margaretjgu
force-pushed
the
remove_native_ts_collection_types
branch
2 times, most recently
from
141fb95 to
1b0da8f
Compare
* remove native collection types * fix rule * remove array type --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co> (cherry picked from commit 29fdfe0)
* remove native collection types * fix rule * remove array type --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co> (cherry picked from commit 29fdfe0)
* remove native collection types * fix rule * remove array type --------- (cherry picked from commit 29fdfe0) Co-authored-by: margaretjgu <136839162+margaretjgu@users.noreply.github.com> Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
* remove native collection types * fix rule * remove array type --------- (cherry picked from commit 29fdfe0) Co-authored-by: margaretjgu <136839162+margaretjgu@users.noreply.github.com> Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
2 participants
Closes #4538