Sprint 2 #64
Sprint 2 #64
Conversation
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
Посмотрите на гитхабе все комментарии к коду (нужно прокрутить вниз страницу там)
Работа проделана огромная
Что сделано отлично:
Readmeхорошо оформлен- Проект задеплоен и отлично работает
- Отлично, что не забыли про
.gitignore - Отлично, что ограничиваете версию
node - Отлично, что нет
EOFошибок в гите - Хорошая структура папок и файлов
- Добавлен
ESLintиStylelint - Отлично, что цвета вынесены в
css-константы - Отлично, что не забываете про
altв тегахimg - Отлично, что не забываете про тег
mainдля главной части страницы - Отлично, что нет типов
any
Что нужно исправить:
- В теории давался еще метод
_removeEvents, который правильно удаляет обработчики перед размонтированием блока. Нужно его добавить вBlock.
Что можно улучшить:
- Если типизация повторяется в каждом методе, то нужно типизировать сам метод, а не дублировать типизацию аргументов. Код станет чище
Исправьте, пожалуйста, недочеты и работа будет принята. Пожалуйста, проверьте работоспособность проекта и наличие возможных ошибок в консоли браузера (кнопка F12) перед отправкой на ревью.
Напоминаю, что работа может быть принята только после исправления всех критических замечаний Нужно исправить.
Комментарии Можно лучше не обязательны к исправлению прямо сейчас, это рекомендации
Удачного рефакторинга кода.
| }) | ||
| } | ||
|
|
||
| private addEvents() { |
There was a problem hiding this comment.
Нужно исправить
В теории давался еще метод _removeEvents, который правильно удаляет обработчики перед размонтированием блока. Нужно его добавить в Block.
любой обработчик события должен навешиваться только через пропсы экземпляра класса, который наследуется от Block. В теории показано было, что в пропсах передается объект events со всеми нужными событиями, а в родительском классе Block они навешиваются через _addEvents и удаляются через _removeEvents. Посмотрите реализацию Block в теории
src/core/HTTPTransport.ts
Outdated
| } | ||
|
|
||
| export default class HTTPTransport { | ||
| get(url: string, options: Options) { |
There was a problem hiding this comment.
Можно лучше
Если типизация повторяется в каждом методе, то нужно типизировать сам метод, а не дублировать типизацию аргументов. Код станет чище
// создаем тип метода
type HTTPMethod = (url: string, options?: Options) => Promise<unknown>
export class HTTPTransport {
// используем тип и удаляем дублирование в аргументах
get: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.GET}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
put: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.PUT}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
post: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.POST}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
delete: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.DELETE}, options.timeout)
)
No description provided.