Skip to content

Добро пожаловать, или посторонним вход воспрещён (часть 2)#13

Open
echenko wants to merge 8 commits into
htmlacademy-react:masterfrom
echenko:module7-task3
Open

Добро пожаловать, или посторонним вход воспрещён (часть 2)#13
echenko wants to merge 8 commits into
htmlacademy-react:masterfrom
echenko:module7-task3

Conversation

@echenko
Copy link
Copy Markdown

@echenko echenko commented May 30, 2026

@keksobot keksobot changed the title Module7 task3 Добро пожаловать, или посторонним вход воспрещён (часть 2) May 30, 2026
@github-actions
Copy link
Copy Markdown

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions Bot pushed a commit that referenced this pull request May 30, 2026
Comment thread src/components/reviews/reviews-form.tsx Outdated
// TODO: Доработать!
try {
dispatch(postReviewAction({
offerId: offerId,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Такие штуки можно просто писать как { offerId, ...}, оно прокинется в объект

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исправил


useEffect(() => {
dispatch(fetchCommentsOfferAction(offerId));
}, [dispatch, offerId]);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Диспатч в зависимости можно не добавлять, это и так стабильная ссылка

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если его убрать то получаем...
React Hook useEffect has a missing dependency: 'dispatch'. Either include it or remove the dependency array.

Comment thread src/pages/offer-page.tsx Outdated
}, [dispatch, offerId]);

if (!checkOfferId(offers, offerId)) {
if (!selectedOffer) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Условный рендеринг добавлять в конец всего перед основным рендером, так как не должно быть такого, что в какой-то момент у тебя рендерится jsx элемент, а в какой-то инициализация каких-то сущностей происходит

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исправил

Comment thread src/pages/offer-page.tsx Outdated
const offer: OfferType = OFFER;

const dispatch = useAppDispatch();
const offerId: string = useParams().offerId || '';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот так лучше не делать. useParams это хук, поэтому вызови и вытащи то, что нужно через деструктуризацию

const { offerId } = useParams()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

как оптимизировать?
const { offerId } = useParams() as { offerId: string };

Comment thread src/store/api-actions.ts Outdated
const {data} = await api.get<OfferType>(`${APIRoute.Offer}/${id}`);
dispatch(selectOffer(data));
} catch {
dispatch(unselectOffer());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лучше, конечно, не просто что-то сбрасывать при ошибки, а как-то обрабатывать ошибку. Но это просто на будущее подумай

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

после устранения замечаний хочу заняться пробрасыванием ошибок

Comment thread src/store/reducer.ts Outdated
.addCase(selectOffer, (state, action) => {
state.selectedOffer = action.payload;
})
.addCase(unselectOffer, (state) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В принципе, в будущем для простоты и для уменьшения кодовой базы, можно не делать такие сбросы, а если тебе надо сбросить, то просто в момент вызова будешь передавать пустые какие-то значения

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

убрал

Comment thread src/types/comments.ts Outdated
@@ -0,0 +1,11 @@
export type CommentElementType = {
'id': string;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ковычки здесь не нужны

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исправил

Comment thread src/types/comments.ts Outdated
Comment on lines +4 to +5
'user': {
'name': string;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Такую вложенность лучше не делать, вынеси user как отдельный тип и прокинь сюда

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

чуть позднее буду делать

Comment thread src/types/offer.ts
title: string;
type: string;
price: number;
city: {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут такая же история с городом и location, подобную вложенность выноси в отдельный тип

Comment thread src/types/offer.ts
@@ -0,0 +1,34 @@
// Create Types

export type OfferType = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это по большей части вкусовщина, но если нет необходимости что-то наследовать, то лучше делай как interface, вместо type. Можно для себя выработать правило, что если interface, это простая типизация, если type - это либо константы, либо с наследованием от кого-то, просто для читабельности

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ок, чуть позднее буду перебирать типы

@github-actions
Copy link
Copy Markdown

Ваш пулреквест опубликован. Посмотреть можно здесь

github-actions Bot pushed a commit that referenced this pull request May 31, 2026
Comment thread src/pages/offer-page.tsx
const dispatch = useAppDispatch();
const offerId: string = useParams().offerId || '';
// TODO: Как оптимизировать offerId
const { offerId } = useParams() as { offerId: string };
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь ты можешь использовать дженерик как useParams<{ offerId: string }>()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants