Skip to content

HT5#69

Open
Meranuxapb wants to merge 1 commit intoTa4i:masterfrom
Meranuxapb:HT5
Open

HT5#69
Meranuxapb wants to merge 1 commit intoTa4i:masterfrom
Meranuxapb:HT5

Conversation

@Meranuxapb
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Owner

@Ta4i Ta4i left a comment

Choose a reason for hiding this comment

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

Все выглядит хорошо. У меня есть кооментарии и вопросы

.then(response => {
dispatch({
type: LOAD_ALL_COMMENTS + SUCCESS,
// Не очевидно, в каком месте разбирать запрос - в action или reducer
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

лучше если ты про res => res.json() то точно в санке (тут), а если ты про response: response.records - лучше в редюсере

// ожидался string -> приходит null.
// Но id и tilte описаны аналогичным образом в TypeArticle
// и в нашем ArticleRecord в reducer/articles.
// а warning'a для них нет - почему такое избирательное поведение?
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

потому что наш сервер не возвращает текс статьи по эндпоинту http://localhost:3001/api/article

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.

Вопрос снимается)
Когда поймал ошибку, почему-то решил, что Article в принципе пустой

// - громоздкую запись
return articles
.setIn(['entities', response.id], response)
.setIn(['entities', response.id], merge({ comments: [] }, response))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

А зачем тебе на LOAD_ARTICLE + SUCCESS мерджить объект с пустыми комментариями с респонсом?

Copy link
Copy Markdown
Author

@Meranuxapb Meranuxapb Feb 4, 2019

Choose a reason for hiding this comment

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

Работа с Immutable даётся тяжеловато
Нужно было сделать так?
.setIn(['entities', response.id], new ArticleRecord(response))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Неплохая идея

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