Sprint 3 #9
Sprint 3 #9
Conversation
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
К сожалению, работу придется отклонить без потери итерации
- Не отображаются мои чаты https://disk.yandex.ru/i/3SIb9LYbxxumeQ
- Не могу добавить новый чат. Скрин https://disk.yandex.ru/i/89BlFQVZaE1EGA . Валидация пишет что-то про логин
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
Посмотрите на гитхабе все комментарии к коду (нужно прокрутить вниз страницу там)
Работа проделана огромная:
Readmeхорошо оформлен- Проект задеплоен и отлично работает
- Отлично, что нет
EOFошибок в гите - Хорошая структура папок и файлов
- Отлично, что указываете понятный текст ошибки под инпутом
- Отлично, что базовый урл вынесен отдельно, чтобы не дублировать его
- Отлично, что почти нет типов
any - Проект отлично выглядит
- Отлично, что отображаете картинку в чате и ее можно менять
- Хороший роутинг
- Хорошая логика сокетов
но есть некоторые недочеты:
- При добавлении пользователя в чат через нажатие на
Enterстраница перезагружается. Это нужно предотвращать во время сабмита формы. И при попытке добавить новый чат это происходит - Нужно отображать историю сообщений при входе в чат
- По заданию эндпоинты в проекте должны быть определённые. Скрин из задания https://disk.yandex.ru/i/5EcJYattAj8NJA
JSON.parse16 раз дублируется под каждый запрос. Нужно было 1 раз это сделать внутриHTTPTransportJSON.parseможет выкинуть исключение, если данные невалидные. Стоит добавить блокtry/catchдля отлова возможной ошибки
Можно лучше
- Обработчик сабмита нужно навешивать только на тег
formс событиемsubmit, а не на кнопку сабмита с событиемclick, так как сабмит формы происходит ещё при нажатииEnter, и он не будет работать, если навесить обработчик клика на кнопку только. Это нужно исправить везде, где есть инпуты и форма - Для любых запросов на сервер в самом конце цепочки запроса нужно осуществлять проверку на возможные ошибки с помощью
catch.
| @@ -0,0 +1,27 @@ | |||
| import HTTPTransport from "../core/http.ts"; | |||
| import {IAuthData, IUser} from "../models/IUser.ts"; | |||
|
|
|||
There was a problem hiding this comment.
- При добавлении пользователя в чат через нажатие на
Enterстраница перезагружается. Это нужно предотвращать во время сабмита формы. И при попытке добавить новый чат это происходит
server.js
Outdated
| @@ -1,5 +1,6 @@ | |||
| // server.js | |||
| const express = require('express'); | |||
| const path = require('path'); | |||
| const PORT = 3000; | |||
There was a problem hiding this comment.
Можно лучше
Лучше дать возможность поменять порт из консоли, если понадобится: const PORT = process.env.PORT || 3000; . Если не задан порт, тогда будет 3000 по умолчанию.
| @@ -0,0 +1,27 @@ | |||
| import HTTPTransport from "../core/http.ts"; | |||
| import {IAuthData, IUser} from "../models/IUser.ts"; | |||
|
|
|||
There was a problem hiding this comment.
- Нужно отображать историю сообщений при входе в чат
| @@ -0,0 +1,27 @@ | |||
| import HTTPTransport from "../core/http.ts"; | |||
| import {IAuthData, IUser} from "../models/IUser.ts"; | |||
|
|
|||
There was a problem hiding this comment.
- По заданию эндпоинты в проекте должны быть определённые. Скрин из задания https://disk.yandex.ru/i/5EcJYattAj8NJA
| password, | ||
| phone | ||
| } as IUser; | ||
| await signUp(data); |
There was a problem hiding this comment.
Для любых запросов на сервер в самом конце цепочки запроса нужно осуществлять проверку на возможные ошибки с помощью catch.
Нужно исправить это везде по коду в конце запросов.
src/services/auth.ts
Outdated
| const result = await authApi.signUp(data); | ||
| const error=responseHasError(result); | ||
| if(!error) { | ||
| setStateUser(JSON.parse(result.responseText)); |
There was a problem hiding this comment.
JSON.parse 16 раз дублируется под каждый запрос. Нужно было 1 раз это сделать внутри HTTPTransport
| method: METHODS.GET | ||
| }, options.timeout); | ||
| }, options.timeout)as Promise<XMLHttpRequest> ; | ||
| }; | ||
|
|
||
| put: HTTPMethod = (url, options = {}) => { |
There was a problem hiding this comment.
Хорошая типизация методов с HTTPMethod
| @@ -34,8 +34,6 @@ export class Block { | |||
|
|
|||
There was a problem hiding this comment.
export class Block {
Можно лучше
Лучше всего передавать тип пропсов в виде дженерика в Block, чтобы можно было конкретный тип пропсов всегда иметь под каждый отдельный экземпляр класса Block. Будет очень универсально
abstract class Block<Props extends Record<string, any> = unknown> {Обратите внимание на abstract. Это не дает создать экземпляр напрямую из класса Block. Только для наследования подходит
There was a problem hiding this comment.
Ок, да я так делала, но что-то не получилось тогда. Давайте я вместе с тестами в 4ом это исправлю.
src/services/send-message.ts
Outdated
| }) | ||
| socket.message((event: MessageEvent) => { | ||
| //console.log('Message!', event.data); | ||
| const message = JSON.parse(event.data); |
There was a problem hiding this comment.
JSON.parse может выкинуть исключение, если данные невалидные. Стоит добавить блок try/catch для отлова возможной ошибки
No description provided.