refactor(CollapseGroup): get rid of additional generics in component types #1377
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/consta/consta-uikit/AhvbmVzCxgmfTjbY1u4DQRidFmpm [Deployment for bbe62f4 failed] |
| @@ -163,7 +163,7 @@ | |||
| "ts-essentials": "^3.0.0", | |||
| "ts-jest": "^25.4.0", | |||
| "ts-node": "^8.8.2", | |||
| "typescript": "3.9.9", | |||
| "typescript": "4.3.5", | |||
There was a problem hiding this comment.
Чтобы заработала сборка, придётся обновить react-scripts и всё, что с ними связано
| getItemRightSide?: never; | ||
| } | ||
| ), | ||
| HTMLDivElement | ||
| > & | ||
| (ITEM extends { label: DefaultItem['label'] } | ||
| ? {} | ||
| ? { getItemLabel?: never } |
There was a problem hiding this comment.
Чтобы TS мог правильно определять, по какой ветке условия мы движемся, приходится добавлять явно исключать пропы из другой ветки с помощью never
There was a problem hiding this comment.
В этом месте типы отличаются от того, что было раньше. Теперь если в item есть label, то getItemLabel вообще нельзя будет указывать.
Сделать так, чтобы getItemLabel был опциональным к сожалению не вышло.
There was a problem hiding this comment.
Возможно, стоит подумать всё-таки отказаться от подхода с геттерами и требовать на входе данные в определённом формате. А подготовку этих данных производить снаружи мапперами.
Нее, маперами не удобно((
Лучше попытаться оставить и с геттерами и без них. Пусть даже если потребуется приведениями типов заниматься, кажется убирать функционал для того чтобы угодить TS - не стоит.
There was a problem hiding this comment.
В этом месте типы отличаются от того, что было раньше. Теперь если в item есть label, то getItemLabel вообще нельзя будет указывать.
Сделать так, чтобы getItemLabel был опциональным к сожалению не вышло.
Это плохо(( обьект может соделжать label, и при этом имя можно взять из какогонибуть name
| ( | ||
| | { | ||
| iconPosition?: 'left'; | ||
| getItemRightSide?: CollapseGroupPropGetItemRightSide<ITEM>; | ||
| } | ||
| | { | ||
| iconPosition?: 'right'; |
There was a problem hiding this comment.
тут TS путался из-за того, что в обоих ветках iconPosition был опциональным и поэтому он не мог понять, по какой ветке мы идём, когда ничего не указано
| return useChoiceGroupIndexed( | ||
| props.isAccordion | ||
| ? { | ||
| value: (openedKeys as typeof props.opened) ?? null, |
There was a problem hiding this comment.
после проверки props.isAccrodion у props.opened выводится правильный тип
| closeIcon?: never; | ||
| directionIcon?: CollapseIconPropDirection; | ||
| closeDirectionIcon?: CollapseIconPropDirection; | ||
| isAccordion?: false; |
There was a problem hiding this comment.
Обновление до TS 4 нужно, чтобы TS правильно определял ветку типов с опциональным дискриминантом (в нашем случае это isAccordion?). В TS 3 придётся делать все дискриминанты обязательными полями: isAccordion, iconPosition и возможно даже closeIcon, getItemLabel и getItemContent, а это усложнит использование компонента снаружи.
Кажется, стоит потратить сейчас время на обновление, чтобы впредь было проще работать с такими кейсами.
There was a problem hiding this comment.
Вообще в TS 4.3 появилось выведение типов для дженериков, так что, возможно, там получится подтюнить код компонента, избавившись от ручных тайпгардов и не меняя сами типы.
|
Обсудили и решили, что стоит в ближайшем будущем перейти на TS 4.3 Плюсы из этого пр-а не перевешивают накладываемые ограничения, так что закрываю |
Описание изменений
Чек-лист
Опционально