Sptint 2 #38
Sptint 2 #38
Conversation
There was a problem hiding this comment.
Поздравляю! Ваша работа принята.
Вы отлично потрудились.
Readmeхорошо оформлен- Проект задеплоен и отлично работает
- Отлично, что не забыли про
.gitignore - Отлично, что ограничиваете версию
node - Отлично, что нет
EOFошибок в гите - Хорошая структура папок и файлов
- Добавлен
ESLintиStylelint - Отлично, что цвета вынесены в
css-константы - Отлично, что не забываете про
altв тегахimg - Отлично, что не забываете про тег
mainдля главной части страницы - Отлично, что почти нет типов
any - Отлично, что указываете понятный текст ошибки под инпутом
Удачного дальнейшего обучения.
Можно лучше
- Если типизация повторяется в каждом методе (функции), то нужно типизировать сам метод (функцию), а не дублировать типизацию аргументов. Код станет чище
- Лучше всего передавать тип пропсов в виде дженерика в
Block, чтобы можно было конкретный тип пропсов всегда иметь под каждый отдельный экземпляр классаBlock. Будет очень универсально - Обработчик сабмита нужно навешивать только на тег
formс событиемsubmit, а не на кнопку сабмита с событиемclick, так как сабмит формы происходит ещё при нажатииEnter, и он не будет работать, если навесить обработчик клика на кнопку только. Это нужно исправить везде, где есть инпуты и форма - В
scriptsнужно добавить скрипты для запускаESLintиStylelint
| method?: string, | ||
| timeout?: number | ||
| }; | ||
| type HTTPMethod = (url: string, options?: TOptions) => Promise<unknown>; |
| xhr.open( | ||
| method, | ||
| isGet && !!data | ||
| ? `${url}${queryStringify(data)}` |
There was a problem hiding this comment.
Можно лучше
queryStringify лучше сразу в методе get вызывать, чтобы не проверять METHODS.GET при каждом запросе
| }; | ||
|
|
||
| // Нельзя создавать экземпляр данного класса | ||
| abstract class Block { |
There was a problem hiding this comment.
Можно лучше
Лучше всего передавать тип пропсов в виде дженерика в Block, чтобы можно было конкретный тип пропсов всегда иметь под каждый отдельный экземпляр класса Block. Будет очень универсально
abstract class Block<Props extends Record<string, any> = unknown> {| FLOW_CDM: 'flow:component-did-mount', | ||
| FLOW_CDU: 'flow:component-did-update', | ||
| FLOW_RENDER: 'flow:render', | ||
| }; |
There was a problem hiding this comment.
тут нужно добавить } as const, чтобы зафиксировать литеральный тип строк внутри объекта
Подробнее https://igorfonin.gitbook.io/typescript-book-ru/typescript-type-system/literals
| }); | ||
| } | ||
|
|
||
| // Not used anywhere yet |
There was a problem hiding this comment.
_removeEvents нужно использовать на строке 171 перед тем, как заменить элемент другим, чтобы удалить обработчики предыдущего элемента
| constructor(props: Props) { | ||
| super({ | ||
| ...props, | ||
| onBlur: () => this.validate(), |
There was a problem hiding this comment.
Отлично, что обработчик валидации находится внутри InputField, а не дублируется под каждый инпут отдельно
| "engines": { | ||
| "node": "16.15.0" | ||
| }, | ||
| "scripts": { |
There was a problem hiding this comment.
В scripts нужно добавить скрипты для запуска ESLint и Stylelint
No description provided.