Простые советы по написанию чистого кода React-компонентов

[alexiusp]

неплохие советы за исключением второго. Если пользоваться Typescript или Flow и прописывать JSDoc в особо важных случаях, то никаких проблем с порядком переменных у внимательного разработчика быть не должно. Явная передача параметров делает вызов функции понятнее и чище. К тому же это даёт возможность «каррировать» функцию. А вот подход, описаный тут, может привести к багам в гораздо большем количестве случаев.

Остальные советы вполне полезные, особенно про использование хуков. Хотя про оборачивание часто используемых компонентов в хуки — мне кажется немного бесполезным. Зачем хук если сам компонент может следить за своим состоянием? Зачем плодить сущности и усложнять, если можно просто использовать компонент обёртку с состоянием?

[amakhrov]

Второй совет полезен в случае, когда у функции много параметров. Например, обертка над апи-запросом, в котором вдобавок еще часть параметров необязательная.

[alexiusp]

Ну в отдельных случаях, конечно, имеет смысл так делать. И, в общем-то, если посмотреть на интерфейсы многих библиотек, то можно увидеть в каких. Но я бы не стал этот приём как бест практис рекомендовать и все параметры на объект заменять, как предлагается в этой статье.

[mSnus]

В совете "используйте объекты вместо условных операторов"
const Component = COMPONENT_MAP[user.type]
разве не придётся обвешивать это проверками на наличие этого user.type, дефолтным значением? Тогда как switch или if/else просто по привычке заставят об этом подумать.

[kahi4]

Вообще обычно в этом есть смысл когда возможных компонентов много или заранее они неизвестны. В противном случае проще и чище будет вообще внутри jsx описать.

[amakhrov]

Привычки бывают разные. У одних они одни, у других другие. Так что кого-то заставят, а кого-то и нет — неважно в каком стиле код писать.

[faiwer]

В случае Typescript/Flow вы не сможете забыть про это. В случае JS у вас оно точно так же упадёт в runtime. React не даст вам отрендерить в качестве компоненты undefined. Т.е. ручная проверка в духе:

or else throw new Error('Unknown user type: ${user.type}`);

будет иметь плюс только в большей понятности самой ошибки.

[mSnus]

Мне кажется, всё-таки тут разработчик этого компонента должен предусмотреть, что делать, если данные пришли "странные".

Я бы вывел ошибку, но сбросил тип на дефолтный, чтобы всё приложение из-за этого не падало… интересно, насколько такой подход сейчас считается правильным?

[faiwer]

если данные пришли "странные"

Хех. Так?

const sum = (a: number, b: number): number => {
  if (typeof a !== 'number' || !isFinite(a)) {
     throw new Error(`wrong input: ${a}`);
  }
  if (typeof b !== 'number' || !isFinite(b)) {
     throw new Error(`wrong input: ${b}`);
  }
  return a + b;
}

Я правильно понял вашу мысль? :)

Я бы вывел ошибку, но сбросил тип на дефолтный, чтобы всё приложение из-за этого не падало…

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

Наиболее корректный путь для большинства приложений — падать в случае ошибки. Совершенно не обязательно целиком. Пусть это будет форма, отдельно взятая панелька, может быть модуль, может даже какая-нибудь хитрая система обработки ошибок. Но не молча игнорировать её, надеясь на авось. Особенно если это хоть как-то касается финансов вашего пользователя.

Если же надёжность для вас серьёзный приоритет:

1. Берём язык со статической системой типов (e.g. Typescript)
2. Выкручиваем его на самый строгий режим (strict: true & type linters для eslint)
3. При необходимости пишем свои правила для линтера
4. Весь внешний input (скажем ответы сервера и сторонних сервисов) проводим через что-нибудь вроде io-ts
5. Тесты и QA ;)

[bgnx]

Использование этой простой стратегии делает компоненты более декларативными и упрощает понимание их кода.

Серьезно? Что же, давайте более подробно проанализируем этот кусок кода

import React from 'react'

const Student = ({ name }) => <p>Student name: {name}</p>
const Teacher = ({ name }) => <p>Teacher name: {name}</p>
const Guardian = ({ name }) => <p>Guardian name: {name}</p>

const COMPONENT_MAP = {
    student: Student,
    teacher: Teacher,
    guardian: Guardian
}

export default function SampleComponent({ user }) {
    const Component = COMPONENT_MAP[user.type]

    return (
        <div>
            <Component name={user.name} />
        </div>
    )
}

Сколько раз ревьюеру (который видит этот кусок кода первый раз) нужно будет промотать глазами чтобы понять что происходит в коде?
1) видим строчку COMPONENT_MAP[user.type] — ага, нужно найти взглядом эту переменную из внешнего скоупа и понять что там хранится,
2) видим строчку рендера какого-то компонента <Component name={user.name} /> — опять нужно найти взглядом переменную из внешнего скоупа и понять что записано в переменную Component.
3-6) и наконец по значениям-ссылкам на три компонента которые были записаны в COMPONENT_MAP мы теперь должны найти каждый компонент и понять что он рендерит

А теперь сравним это с такой версией

import React from 'react'

export default function SampleComponent({ user }) {
    return (
        <div>
          {user.type == "student" && 
             <p>Student name: {user.name}</p>
          }
          {user.type == "teacher" && 
             <p>Teacher name: {user.name}</p>
          }
          {user.type == "guardian" && 
             <p>Guardian name: {user.name}</p>
          }
        </div>
    )
}

Сколько раз в этой версии разработчику нужно будет мотать глазами чтобы понять что происходит в коде? Не проще ли становится код когда вообще не нужно мотать глазами? Не является ли вторая версия более "декларативной" ?

[faiwer]

Серьезно?

Да

А теперь сравним это с такой версией

У вас просто использование компонента сводится к:

<p>Guardian name: {user.name}</p>

Более реалистичный пример будет включать в себя 5-7 props. И тогда вы получаете либо груду copy-paste, либо код где props высчитываются в одном месте, а используются в другом через spread operator, что тоже не добавляет читаемости.

Использование hashMap + key настолько избитый и широкоиспользуемый подходы, что я сильно удивлён что есть люди, которых это удивляет.

Вот из недавнего в моей практике. В приложении есть около 15 видов уведомлений. Я подключаю нужный компонент по типу самого уведомления. Все компоненты имеют единый API. Я не пишу 15 IF-ов. Я пишу hashmap (типизированный с TS).

const notificationsMap: Record<NotificationType, React.ComponentType<{ notification: Notification }> = { ... };

const NotificationItem = ({ notification }: { notification: Notification }) => <notificationsMap[notification.type] {...{ notification }}/>;

код получается примерно таким. И да, разумеется, чтобы "понять всё" придётся побегать по файлам. Ну а как иначе то? Сделав тут портянку IF-ов проблема никуда не исчезнет.

В случае TS это особенно удобно ввиду того что без лишних телодвижений вы автоматически получаете exhaustive проверку.

[nikitapaparimskiy]

Вам показали упрощенный пример, с небольшим компонентом и небольшим вариантов(3), увеличив кол-во и того и другого — вы заметите, что подход автора статьи имеет место быть, я бы даже сказал предпочтительнее

[m1khaylov]

Нет, не проще. Ваш код нарушает принцип DRY.

[0xLDev]

Очень полезно, благодарю за такую понятную статью.

[xadd]

За совет #3 нужно увольнять.

[asakasinsky]

Речь идёт о том, что следует избегать использования оператора spread ({...props}) при передаче свойств от родительских компонентов дочерним.

Я бы не был столь категоричен. Есть случаи, когда иначе никак: обёртки (wrappers).

[megahertz]

Бывает удобно запретить spread props линтером для всего проекта и разрешить для каталога shared components

[asakasinsky]

То есть глобально запрещаем, а для определённых директорий разрешаем?

[megahertz]

Все верно

[ravendyg]

В последнем примере кода один export внутри другого. Вы уверены, что так стоит писать?