Sprint 3 #7
Sprint 3 #7
Conversation
fighting errors
put data in json
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
Работа проделана огромная
- Отлично, что фиксируете версии зависимостей (почти все)
- Хорошая структура папок и файлов
- Хороший роутинг
- Отлично, что базовый урл вынесен в константу
- Отлично, что ловите возможные ошибки в конце каждого запроса к серверу
но есть некоторые недочеты:
- нет ссылки на задеплоенное приложение на Нетлифай в
Readme - По семантике нужно помещать весь главный контент страницы в тег
main. - я могу добавить пустое сообщение в чат. А валидация не должна мне позволять это. Скрин https://disk.yandex.ru/i/-blLZ4Kx_iKtUw . Нужно проверять, что оно не пустое
- не подгружается история сообщений в чатах
- при изменении профиля нужно вставлять данные в инпуты, иначе придется все впечатывать опять, хотя я одну букву хочу изменить в имени https://disk.yandex.ru/i/X4gXqLuz-9yB1w
- нет валидации в инпутах профиля. Она должна появляться при
blurилиfocus - не могу добавить новый чат. Требуется
idпользователя, а я его не знаю. Я хочу создать чат, а потом только добавлять пользователей туда (не одного) - Не могу удалить чат. Нет такого функционала
- меню с тремя точками не работает https://disk.yandex.ru/i/loKgYvPQer_g8w
- В проекте в некоторых файлах есть ошибка
EOF. Дляgitважно наличие пустой строки в конце файла. Подробнее тут: https://stackoverflow.com/questions/5813311/whats-the-significance-of-the-no-newline-at-end-of-file-log Можно в настройки IDE добавитьinsert_final_newline = true JSON.parseможет выкинуть исключение, если данные невалидные. Стоит добавить блокtry/catchдля отлова возможной ошибки
Можно лучше
- При валидации нужно отображать текст ошибки красным цветом под инпутом, чтобы было понятно, что пользователь делает не так и как это исправить.
- Если типизация повторяется в каждом методе, то нужно типизировать сам метод, а не дублировать типизацию аргументов. Код станет чище
- Попробуйте убрать типы
any
Исправьте, пожалуйста, недочеты и работа будет принята. Пожалуйста, проверьте работоспособность проекта и наличие возможных ошибок в консоли браузера (кнопка F12) перед отправкой на ревью.
Напоминаю, что работа может быть принята только после исправления всех критических замечаний Нужно исправить.
Удачного рефакторинга кода.
| .then(() => { | ||
| MyRouter.go(routs.chatsPage); | ||
| }) | ||
| .catch(() => { |
There was a problem hiding this comment.
Отлично, что ловите возможные ошибки в конце каждого запроса к серверу
There was a problem hiding this comment.
Весь обязательный функционал реализован, мелкие ошибки исправлены, код залит на нетлифай. Для исправления ошибок с чатом необходимо полностью переработать шаблоны. Прошу одобрить проект. Начну работу над новыми шаблонами и буду собирать уже на webpack
package.json
Outdated
| "assert": "2.0.0", | ||
| "browserify-zlib": "0.2.0", | ||
| "buffer": "6.0.3", | ||
| "console-browserify": "^1.2.0", |
There was a problem hiding this comment.
Можно лучше
Лучше задавать фиксированные версии зависимостям. Если нечаянно удалить package-lock.json и попытаться повторно установить зависимости с помощью команды npm install, то npm установит последние версии зависимостей, что может привести к ошибкам.
| @@ -0,0 +1,2 @@ | |||
| a(class="button-back") | |||
| p(class="arrow") ← | |||
There was a problem hiding this comment.
В проекте в некоторых файлах есть ошибка EOF. Для git важно наличие пустой строки в конце файла. Подробнее тут: https://stackoverflow.com/questions/5813311/whats-the-significance-of-the-no-newline-at-end-of-file-log Можно в настройки IDE добавить insert_final_newline = true
src/utils/core/HTTPTransport.ts
Outdated
| } | ||
|
|
||
| class HTTPTransport { | ||
| get(url: string, options: OptionsWithoutMethod = {}) { |
There was a problem hiding this comment.
Можно лучше
Если типизация повторяется в каждом методе, то нужно типизировать сам метод, а не дублировать типизацию аргументов. Код станет чище
// создаем тип метода
type HTTPMethod = (url: string, options?: OptionsWithoutMethod) => Promise<XMLHttpRequest>
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)
)| } | ||
|
|
||
| if (method === Methods.GET) { | ||
| url += queryStringify(data); |
There was a problem hiding this comment.
Можно лучше
queryStringify лучше сразу в методе get вызывать, чтобы не проверять METHODS.GET при каждом запросе
src/api/base-api.ts
Outdated
|
|
||
| type HTTPOption = Record<string, any> | ||
|
|
||
| const host = 'https://ya-praktikum.tech/api/v2' |
There was a problem hiding this comment.
Отлично, что базовый урл вынесен в константу
|
|
||
| this.socket.addEventListener("message", async (event) => { | ||
| const messages = store.getState().currentDialog || []; | ||
| const ans = JSON.parse(event.data); |
There was a problem hiding this comment.
JSON.parse может выкинуть исключение, если данные невалидные. Стоит добавить блок try/catch для отлова возможной ошибки
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
К сожалению, работу придется отклонить без потери итерации
- не могу запустить проект локально. Скрин https://disk.yandex.ru/i/WNvE8m8SjIdt9w
- по чек-листу нужно задеплоить приложение на Нетлифай и показать ссылку в
Readme
✅ Deploy Preview for bucolic-paprenjak-cebd36 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
К сожалению, работу придется отклонить без потери итерации, так как не исправлены все замечания:
- Скрин https://disk.yandex.ru/i/nziDpP9Og71DqA я могу добавить пустое сообщение в чат. А валидация не должна мне позволять это.
- не подгружается история сообщений в чатах
- не могу добавить новый чат. Требуется
idпользователя, а я его не знаю. Я хочу создать чат, а потом только добавлять пользователей туда (не одного) - Не могу удалить чат. Нет такого функционала
- меню с тремя точками не работает https://disk.yandex.ru/i/loKgYvPQer_g8w
- при изменении профиля нужно вставлять данные в инпуты, иначе придется все впечатывать опять, хотя я одну букву хочу изменить в имени https://disk.yandex.ru/i/X4gXqLuz-9yB1w
No description provided.