Типы в defaultProps

[HelenaIsh]

fix IF-284

Решение

Было принято решение разделять пропы на пропы у которых есть значение по умолчанию (DefaultProps) и все остальные.

Экспортировать будем 'частичные пропы':

export type ComponentNameProps = {
   ...
} & Partial<DefaultProps>;

Мы будем пользоваться полноценными пропами:

type ComponentNameComponentProps = ComponentNameProps & DefaultProps;

class ComponentName extends React.Component<ComponentNameComponentProps> {
  ...
  public static defaultProps: DefaultProps = {
    ...
  }
  ...
}

Дополнительно это позволило избавиться от createPropsGetter

[JackUait]

В компонентах Tab и ZIndex нет getDefaultProps.

Давай ещё переименуем getDefaultProps в getDefaultPropsTypes и добавим описание этой функции, чтобы было понятно, что это костыль чисто для классовых компонентов.

[lossir]

По поводу smoke тестов (Lint/Test)

Кратко

Вот "фикс" #2799. Надо будет просто подлить мастер потом.

Подробно

Там возникает ошибка в типах Select'а.

ERROR in src/App.tsx:125:7
TS2589: Type instantiation is excessively deep and possibly infinite.
    123 |       </RadioGroup>
    124 |       <ScrollContainer>scroll container</ScrollContainer>
  > 125 |       <Select onValueChange={() => ({})} />
        |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ERROR in src/App.tsx:125:30
TS2589: Type instantiation is excessively deep and possibly infinite.
    123 |       </RadioGroup>
    124 |       <ScrollContainer>scroll container</ScrollContainer>
  > 125 |       <Select onValueChange={() => ({})} />
        |                              ^^^^^^^^^^

Она также появляется при использовании Partial<Props>, о котором я говорил выше #2760 (comment).
А возникает она, когда TypeScript решает, что типы становятся слишком сложными для вычисления.

microsoft/TypeScript#30188 (comment)

It happens when TS decides that types become too complex to compute (ie).

Способ решения - исследовать каждый случай самостоятельно, шаг за шагов вычисляя проблемный тип...
Но нам повезло =)

У нас в смок тестах устарел TS - ~3.7.4. А в версии 3.9 заметно повысили производительность.
Например, скорость компиляции material-ui сократилась на 40%

TypeScript 3.9 > Speed Improvements

In total, we believe we’ve achieved around a 40% reduction in material-ui’s compile time!

Я проверил, и, действительно, после обновления до 3.9 и выше ошибка пропадает. Пока можно не вникать что же там такого сложно было в вычислениях =)

[JackUait]

Хмм, странно, TypeScript благодаря своей статической натуре может "поглощать" очень глубокие типы. В отличие от например Swift, у которого типизация не статическая. Скорее всего где-то в типах Select закралась рекурсия, которую TypeScript не воспринимает как ошибку.

[JackUait]

@HelenaIsh Обсудили эту проблему с Максимом @lossir голосом, решили остановиться на Partial<Props> внутри компонента.

class SomeComponent {
	public static defaultProps: Partial<Props> = {
		...
	}
}

Недопонимание произошло по причине того, что я думал, что React мутирует типы пропов, чтобы они совпадали с defaultProps.

Как я это понимал:

1. В компонент передаются Props;
2. Мы задаём defaultProps тип Partial<Props>;
3. React работает с Partial<Props>, вместо Props (то как это работает в функциональных компонентах).

Как это работает на самом деле:

1. В компонент передаются Props;
2. Мы задаём defaultProps тип Partial<Props;
3. Тип defaultProps распространяется только на defaultProps и React не подхватывает его для типизации Props переданных в компонент;
4. React работает с Props.

[lossir]

@HelenaIsh Фикс для смок тестов в мастере.

[JackUait]

Столкнулись с проблемой, что Partial<Props> не работает с createPropsGetter.
Решили так:

// Снаружи компонента.

// Разделяем пропы на пропы у которых есть значение по умолчанию и все остальные.
type DefaultProps = {
  width: number | string;
  maxHeight: number | string;
  hasShadow: boolean;
  ...
};

// Пользователи будут получать Partial пропы.
type MenuProps = {
  children?: React.ReactNode;
  ...
} & Partial<DefaultProps>;

// Мы будем пользоваться полноценными пропами.
type MenuComponentProps = MenuProps & DefaultProps;

// Внутри компонента.
class Menu extends React.Component<MenuComponentProps> {
  ...
  public static defaultProps: DefaultProps = {
    ...
  }
  ...
}

Пока что не нашёл изъянов у этого решения.
Это решение покрывает все проблемы, о которых мы говорили выше, а также:

1. Позволяет избавиться от createPropsGetter (что предлагаю сделать прямо в этом PR);
2. Мы получаем более строгую типизацию в том плане, что мы наверняка знаем какие типы будет иметь defaultProps

[JackUait]

Вот такие вот конструкции выглядят очень устращающе:

// ComponentTable.tsx

interface DefaultProps<C, P, S> {
  presetProps: DefaultizeProps<C, P>;
  presetState: Partial<S>;
}

public static defaultProps: DefaultProps<any, any, any> = {
  presetProps: {},
  presetState: {},
};

Давай там, где используются дженерики добавим значений по умолчанию для DefaultProps:

// ComponentTable.tsx

interface DefaultProps<C = any, P = any, S = any> {
  presetProps: DefaultizeProps<C, P>;
  presetState: Partial<S>;
}

public static defaultProps: DefaultProps = {
  presetProps: {},
  presetState: {},
};

Таким образом мы во всех компонентах приведём поглощение типа DefaultProps к общему виду.

[JackUait]

В InputLikeText нет типа DefaultProps.

[JackUait]

И ещё давай вынесем интерфейс компонента в отдельный тип:

type AutocompleteInterface = {
  /** Функция отрисовки элемента меню */
  renderItem: (item: string) => React.ReactNode;
  /** Промис, резолвящий элементы меню */
  source?: string[] | ((patter: string) => Promise<string[]>);
  /** Отключает использование портала */
  disablePortal: boolean;
  /** Отрисовка тени у выпадающего меню */
  hasShadow: boolean;
  /** Выравнивание выпадающего меню */
  menuAlign: 'left' | 'right';
  /** Максимальная высота меню */
  menuMaxHeight: number | string;
  /** Ширина меню */
  menuWidth?: number | string;
  /** Отключить скролл окна, когда меню открыто */
  preventWindowScroll: boolean;
  /** Вызывается при изменении `value` */
  onValueChange: (value: string) => void;
  /** onBlur */
  onBlur?: () => void;
  /** Размер инпута */
  size: InputProps['size'];
  /** value */
  value: string;
};

export type AutocompleteProps = Override<InputProps, AutocompleteInterface> & CommonProps & Partial<DefaultProps>;

В текущей реализации очень сложно воспринимать что происходит.

[JackUait]

У нас получилась какое-то месиво из ключевых слов type и interface. Давай в тех местах, где ты внесла правки приведём всё к единому стилю. Так как type это по сути продвинутый interface, то давай использовать его.

[JackUait]

Нам не нужно дублировать типы из ComponentNameProps в DefaultProps.

План такой:

1. Удаляем пропы с дефолтными значениями из ComponentNameProps;
2. Закидываем их в DefaultProps и удаляем у этих пропов вопросики;
3. Создаём два типа: пользовательский и внутренний, в пользовательском должны быть Partial<Props>, во внутреннем Props.

То о чём я говорю на примере компонента Button:

// Изначальное состояние
export type ButtonProps = {
  corners?: number;
  disabled?: boolean;
  ...
  size?: ButtonSize;
  type?: ButtonType;
  use?: ButtonUse;
} & CommonProps;

// С типом DefaultProps
// Пользовательский тип ButtonProps
export type ButtonProps = {
  corners?: number;
  disabled?: boolean;
  ...
} & CommonProps & Partial<DefaultProps>;

type DefaultProps = {
  use: ButtonUse;
  size: ButtonSize;
  type: ButtonType;
}

// Внутренний тип ButtonComponentProps
type ButtonComponentProps = ButtonProps & DefaultProps;

[JackUait]

Создал пулл-реквест в парковку удаляющий createPropsGetter оттуда.

[zhzz]

Я вижу, что это решение одновременно двух проблем, IF-284 и #2553.

Оно работает, но слишком радикально. Разделение интерфейса пропов на много частей уменьшает читаемость в данном случае. Придется в голове собирать все по кусочкам. Плюс, поддерживать код и писать новый по этим правилам будет непросто. Уже сейчас в ValidationContainer объявление пропа disableSmoothScroll дублируется в двух местах. Нет шансов что в этом всегда будет порядок.

Решаемые проблемы того не стоят. Они все исходят из особенностей defaultProps, от использования которых мы сможем вовсе отказаться с переходом на функциональные компоненты. Предлагаемые изменения тогда станут не нужны.

Я бы обе проблемы решил просто в лоб, локально. Для #2553 cделал бы все пропы опциональными, порешав проблемы с линтом дополнительными проверками в нашем коде. А для IF-284 указал бы везде корректный тип для defaultProps руками, например.

От createPropsGetter можно отказаться.

[JackUait]

Для #2553 cделал бы все пропы опциональными

С этим не соглашусь по нескольким причинам (которые мы даже кажется уже обсуждали):

1. Сделать пропы опциональными - просто. Вернуть их в прежнее состояние, когда будет готов полноценный фикс - не просто
   1. С точки зрения разработки: нужно будет сверять все места из пулл-реквеста, где поменялась обязательность пропов и добавились лишние проверки и ручками всё это возвращать в адекватное состояние. Плюс, из этого вытекает ещё одна проблема - придётся выполнить тройную работу: сначала сделать временное решение, удалить временное решение, запилить полноценное решение
   2. С точки зрения пользователей: чтобы вернуть обязательные пропы придётся выпускать мажорную версию библиотеки, так как для пользователей, которые будут использовать компоненты с дефолтными значениями вместо дефолтов придётся вводить настоящие данные, что будет для них ломающим изменением
2. Пользователям полезно видеть обязательные пропы. Нам полезно видеть, что пользователи видят обязательные пропы (пункт 2, подпункт 2). Не думаю, что много пользователей наизусть помнят API компонентов и какие пропы нужно передавать "чтобы работало", поэтому предполагаю, что многие завязываются именно на обязательных пропах, чтобы заставить компоненты работать
   Если мы сделаем все пропы необязательными мы столкнёмся с:
   i. Ухудшением DX - пользователям придётся лезть в доку или смотреть на типы, чтобы понять что им ещё нужно передать "чтобы заработало"
   ii. "HTML-ностью". В HTML все "пропы" - необязательные. Я это вижу как образец плохого проектирования. Рассмотрим, что я имею ввиду на примере тега <img />: у этого тега на самом деле есть два обязательных атрибута - src и alt. Атрибут src необходим для отображения изображения для зрячих пользователей, атрибут alt необходим для отображения изображения при чтении документа скрин-ридерами, поисковыми работами и т.д. (с какой-то точки зрения атрибут alt даже более необходимый чем атрибут src). Что мы имеем на самом деле: на большинстве сайтов не прописаны alt-атрибуты картинкам (статистики у меня нет, но достаточно зайти даже на какой-нибудь Озон (кажется, что интернет-магазин идеальное место, чтобы прописывать alt-атрибуты картинкам, так как у тебя всегда есть необходимый текст), чтобы увидеть, что там в большинстве разделов сайта отсутствуют alt-атрибуты у изображений), такая же история будет и у нас: пользователи будут неправильно пользоваться компонентами, потому что им никто не запрещает это делать
3. Грязный код. Это изменение затруднит внесение других изменений: например, нам нужно добавить условие, что, если какой-то проп передан сделать то-то, теперь нам придётся выполнять эту логику внутри проверки на undefined, что в зависимости от типа данных либо почти не повлияет на читаемость кода, либо значительно затруднит его чтение

Я вижу два вероятных решения этой проблемы:

1. Решить задачу от начала и до конца, сев и просмотрев все компоненты на обязательность пропов (по опыту функциональных компонентов могу сказать, что это не всегда очевидно и порой приходится покопаться в коде, чтобы понять что проп на самом деле необязательный) - оперативно закроем задачу, потратим много человеко-часов на её закрытие
2. Решить эту задачу в рамках "функциональных компонентов" - крайне не оперативно закроем проблему, почти не потратим человеческих ресурсов, так как опять же по опыту могу сказать, что проблема решается "сама собой", по ходу рефактора

Я голосую за второй вариант, так как оперативно закрыть задачу у нас уже не получилось

[zhzz]

1. Сделать пропы опциональными - просто. Вернуть их в прежнее состояние (...) - не просто.

Зачем их возвращать? У нас проблема с типом пропов, у которых есть дефолтные значения (#2553). Их нужно перманентно порешать, сделав по-настоящему опциональными.

2. Пользователям полезно видеть обязательные пропы.

Согласен. Со "всеми" пропами я погорячился. Нужно сделать опциональными только те, которые сейчас в типах указаны обязательными, но имеют дефолтные значения (по сути - необязательные). Это корень проблемы #2553.

P.S: на счет обязательности alt не согласен. Это выбор каждого делать a11y или нет. Нельзя к такому форсить, можно лишь рекомендовать (через линтеры, например).

3. Грязный код

Это цена решения проблемы на нашей стороне. Не такая уж и большая. До этого мы решили предоставить пользователям мириться с ней, что и привело к #2553.

---

С тем, что можно оставить как есть до перехода на функциональные - согласен.

[JackUait]

Со "всеми" пропами я погорячился. Нужно сделать опциональными только те, которые сейчас в типах указаны обязательными, но имеют дефолтные значения (по сути - необязательные)...

В таком случае, мы кстати можем убить двух зайцев сразу: сейчас сделать все пропы, которые есть в defaultProps опциональными, это не должно занять много времени и мы быстро закроем задачу, а в функциональных компонентах углубиться в задачу и решить, какие пропы на самом деле необязательны

...на счет обязательности alt не согласен. Это выбор каждого делать a11y или нет. Нельзя к такому форсить...

В документации W3C к HTML атрибут alt обозначен как обязательный, это можно проверить в валидаторе. А причины можно узнать в туториале об изображениях, если коротко, то любое, даже декоративное изображение должно иметь атрибут alt, но для декоративных изображений он должен быть пустым, также alt используется не только для обеспечения a11y, но в том числе и для скраппинга веб-страниц (распространённый пример - поисковые роботы)

[zhzz]

Тогда, сходимся на том, что, что сейчас делаем все пропы, которые есть в defaultProps опциональными, прописывая для defaultProps корректный тип, например, через <Partial<P>>. Но проще, думаю, уже в отдельном ПРе. Этот закроем.

углубиться в задачу и решить, какие пропы на самом деле необязательны

Это какая-то отдельная задача, раньше она не звучала. Можно сделать в функциональных, если она действительно стоит.

В документации W3C к HTML атрибут alt обозначен как обязательный

Да, по факту так. Но с моей точки зрения, поддержка a11y, скраппинга и др. - это все дополнительный функционал, который мне может быть сознательно не нужен. В этом случае, меня насильно будут заставлять передавать пустую строку. И даже в остальных случаях, думаю, многие будут это делать. Тоже не очень хорошее API, плохой DX. Я бы сделал такой проп опциональным, снабдив соответствующим правилом для eslint.