[우연서]w5_리뷰요청_실시간 새 글 등록 알람 #150
Conversation
| }; | ||
|
|
||
| async function feedAlarmOn() { | ||
| if (feedAlarm > ALARM_LIMIT) { |
There was a problem hiding this comment.
feedAlarm이 0개 이상일 경우에만 해당되는 분기라면,
ALARM_LIMIT보다 클때를 체크하기보다는 feedAlarm 존재여부를 체크한다는 의도를 드러내는 것도 괜찮을 것 같습니다.
ALARM_LIMIT은 특정한 제한이 있고 숫자가 조절가능하다는 의미로 보여졌어요~
There was a problem hiding this comment.
넵 맞습니다. 지금은 친구인 관계가 많지 않아서 하나라도 새로운 글이 뜨면 알림으로 주고 있는데, 친구의 관계가 많아지면 해당 limit을 올릴 예정입니다
| !fetchMoreResult.feeds || | ||
| !fetchMoreResult.feeds.feedItems || | ||
| !prev.feeds || | ||
| !prev.feeds.feedItems |
There was a problem hiding this comment.
어떤 상태를 나타내는 건지 따로 변수로 분리하여 의도를 노출하면 더 좋을 거 같아요.
예를들면 이런식으로..
const hasNoResult = ...
const hasNoPrevFeed = ...
if (hasNoResult || hasNoPrevFeed) {
return prev;
}
| return prev; | ||
| } | ||
|
|
||
| if (!fetchMoreResult.feeds.feedItems.length) { |
There was a problem hiding this comment.
이 부분은 바로 윗 분기와 합칠 수 있을 것 같아 보이는데 맞나요?
| return subscribeToMore({ | ||
| document: FEEDS_SUBSCRIPTION, | ||
| variables: { | ||
| userEmail: myInfo && myInfo.me && myInfo.me.email ? myInfo.me.email : '' |
There was a problem hiding this comment.
[참고] 기존에는 객체 속성에 접근할 때 이런 코드를 불가피하게 많이 사용할 수밖에 없어서
좀 더 간결한 코드를 위해 옵셔널 체이닝이라는 스펙(https://github.com/tc39/proposal-optional-chaining/) 이 논의중이었는데요.
최근에 stage 4로 올라갔다고 하네요 :) (tc39/ecma262#1646)
요런식으로 쓸 수 있게 되겠네요.
{ userEmail: myInfo?.me?.email ? myInfo.me.email : '' }
적용보다는 알고 계시면 좋을 거 같아서 멘트드립니다 :)
There was a problem hiding this comment.
{ userEmail: myInfo?.me?.email ?? '' }There was a problem hiding this comment.
앗 그렇네요! 훨씬 간결해지고 좋은 것 같습니다. 감사합니다
| key={getDate(feed.feed.createdAt).toISOString()} | ||
| content={feed.feed.content} | ||
| feedinfo={feed} | ||
| createdAt={getDate(feed.feed.createdAt).toISOString()} |
There was a problem hiding this comment.
feed.feed가 반복되는 것이 가독성이 떨어져 보이기도 합니다. 비슷한 패턴이 다른 부분에도 종종 보이는데 개선해봐도 좋겠네요~
There was a problem hiding this comment.
넵 반복되는 부분은 구조분해나 변수를 지정해서 받는 방법으로 가독성 개선을 해보겠습니다 감사합니다
| MutationWriteCommentArgs | ||
| } from '../../types'; | ||
|
|
||
| const DEFAUT_MAX_DATE = '9999-12-31T09:29:26.050Z'; |
| const { filename, createReadStream } = file; | ||
| filePromises.push(uploadToObjStorage(createReadStream(), filename)); | ||
| } | ||
| const fileLocations = await Promise.all(filePromises); |
There was a problem hiding this comment.
for-await-of와 Promise.all를 잘 활용하셨네요👍
| return pubsub.asyncIterator(NEW_FEED); | ||
| }, | ||
| async (payload, _, context) => { | ||
| const myEmail = context.email; |
There was a problem hiding this comment.
이 프로젝트에서 UUID가 email로 정해진 건가요?
맞다면 유저 정보로써의 email과 UUID로서의 email이 동일하게 쓰이고 있는건지 궁금하고
만약 프로그램 요구사항이 바뀌어서 다른 형식의 UUID로 대체된다고 할때 유연하게 대응가능한가도 궁금합니다.
There was a problem hiding this comment.
네 사용자 이메일을 고유값으로 사용하기로 했습니다. 말씀하신 것처럼 다른 형식의 key로 변경된다면 어떻게 처리될지에 대한 부분도 고려해보겠습니다.
5주차 리뷰 요청드립니다!
작업한 부분과 관련있는 컴포넌트와, 서버 api 쪽 파일을 같이 묶어서 올렸습니다.
작업의 히스토리는 #140 로 묶어서 정리해 두었습니다.
작업한 부분
(ex : 새 게시글 n 개)
작업이유
실시간 새 글 등록은 pubsub을 이용해서 구현하였습니다.
apollo client의 hook을 사용하다보니 쿼리를 요청한 부분과 새로 알림을 받는 데이터를 병합해야 했습니다. 그래서 저번주에는 쿼리로 요청받은 데이터를 state로 다시 저장해서 사용하는 방식을 사용하였습니다.
하지만 apollo client의 기능 중 publish 된 데이터를 받을 때 state를 합칠 수 있게 제공하는 기능이 있어서(
subscriptomore) 해당 기능을 사용해서 피드 데이터를 합치는 작업을 진행하였습니다.리뷰 요청드리는 부분
2019-17/client/src/composition/Feed/index.tsx19-1662019-17/client/src/composition/Feed/NewFeedAlarm.tsx24-352019-17/server/src/api/feed/feed.resolvers.ts1-203