@sanchos86
Frontend developer

Прокомментируете тестовое на react?

Коллеги, привет!

Нахожусь в поиске работы на react стэке, а так как коммерческого опыта на react нет, то делаю для тренировки тестовые. Последнее от aviasales. Понимаю, что в нем нет ничего особенно сложного, но все равно хотелось бы услышать ваше мнение относительно кода и в целом относительно организации папок и файлов, узнать, что бы вы улучшили или изменили(не считая добавления state-менеджера и тестов).
Код
Демо

Заранее спасибо, буду благодарен за любой фидбэк.
  • Вопрос задан
  • 2238 просмотров
Решения вопроса 1
rockon404
@rockon404 Куратор тега React
Frontend Developer
1. Хотелось бы видеть в проекте использование redux. react+redux - это самый распространенный и востребованный стек в React разработке.

2. Почему все хандлеры и состояния находятся в App, а не в Main? Как вы потом эту кашу собираетесь масштабировать? Переносите все, что связанно только с Main в Main. По-хорошему смотрите пункт 1.

3. Слишком много функциональных компонентов. Подумайте их где можно заменить на классы с реализованным shouldComponentUpdate или на PureComponent, чтобы убрать лишние вызовы render этих компонентов.

4. import Logo from 'images/Logo.png';
называть пути к ресурсам с заглавной буквы неправильно.

5. Вместо:
const StyledLogo = styled.img.attrs({
  src: Logo,
  alt: 'Aviasales'
})`
  width: 60px;
  height: 61px;
`;

Удобней в использовании:
const StyledLogo = styled.img`
  width: 60px;
  height: 61px;
`;

и:
<StyledLogo src={logo} alt="Aviasales" />

6.
const Error = ({ text }) => (
  <StyledError dangerouslySetInnerHTML={{__html: text}} />
);

зачем тут html?
Для сохранения переносов строки есть css правило:
white-space: pre-line;

7. Вместо:
let element;

if (error && !isLoading) {
  element = <Error text={error} />;
}
if (!error && isLoading) {
  element = <Loader />;
}
if (!error && !isLoading) {
  element = (
    <>
    <Heading />
    <Main
    isCurrencyExchanging={isCurrencyExchanging}
    activeCurrency={activeCurrency}
    handleCurrencyChange={this.handleCurrencyChange}
    ticketsFilteredByStops={ticketsFilteredByStops}
    stops={stops}
    handleStopsChange={this.handleStopsChange}
    handleUncheckOther={this.handleUncheckOther}
    />
    </>
  );
}
return element;

Лучше:
if (isLoading) return <Loader />;

if (error) return <Error text={error} />;

return (
  <>
    <Heading />
    <Main
      isCurrencyExchanging={isCurrencyExchanging}
      activeCurrency={activeCurrency}
      handleCurrencyChange={this.handleCurrencyChange}
      ticketsFilteredByStops={ticketsFilteredByStops}
      stops={stops}
      handleStopsChange={this.handleStopsChange}
      handleUncheckOther={this.handleUncheckOther}
    />
  </>
);


8. Вместо:
filterTickets = (tickets, stops) => {
  return tickets.filter((ticket) => {
    return values(stops).indexOf(ticket.stops) !== -1;
  });
};

Лучше:
filterTickets = (tickets, stops) => tickets.filter(
  ticket => values(stops).includes(ticket.stops),
);


9. Не пропускайте отступы между методами и между вложенными свойствами css.

10. Вместо:
componentsDidMount() {
  // много кода
}


Лучше:
componentsDidMount() {
  this.fetchSomeData();
}


11. Директории и индексные файлы для каждого компонента, имхо, лишнее. Лучше компоненты определять в одноименном файле и только когда возникнет необходимость в его декомпозиции, заменять на директорию и index.

12. Loader и Error самое место в директории components/core или что-то вроде того. Там же, по-хорошему, должны находиться базовые компоненты: кнопки, инпуты, табы, чекбоксы.

13. Styled компоненты, имхо, лучше писать в файле с компонентом, где они применяются. Так анализ кода происходит гораздо быстрей и легче поддерживать. Исключение - переиспользуемые компоненты.
Даже если вам больше нравится выносить, называть файл style неправильно, вы там описываете компоненты, а не просто стили.
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы