-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Feat: posting 포스트 발행 기능 #61
Conversation
src/apis/posting/index.ts
Outdated
} catch (error) { | ||
console.error(error); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2;
여기서 console.error로 에러를 잡으면 UI레이어에서는 에러를 핸들링 할 수 없을것 같아요!
또 react-query가 error도 잡아주기 때문에 여기서 에러를 잡아야 하나?라는 생각도 드네요!
아니면 단순히 throw Error해주는 방법도 있을것 같아요~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw Error를 활용하면 error 메시지의 내용을 확인할 수 있으면서, react-query의 isError를 활용한 핸들링도 가능해질 것 같네요! throw Error를 활용하는 식으로 변경하겠습니다!
if (validateContent(contentRef.current.value)) { | ||
const formData = createFormData(contentRef.current.value, channelId); | ||
const handleConfirmButton = () => { | ||
const content: string = contentRef.current.value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단순 질문!
타입추론이 안 돼서 string타입을 지정하신건가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 그것도 있고 바로 밑에서 length 속성을 활용하고 있는데 'content.length가 문자열의 길이다' 라는 것을 조금 더 명시적으로 드러내고 싶어서 쓴 것도 있습니다!
postCreateNewPost(customToken, formData) | ||
.then(() => { | ||
navigate('/posts'); | ||
}) | ||
.catch(() => { | ||
console.error('포스트 생성에 실패했습니다.'); | ||
}); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단순 질문!
postCreateNewPost에서 에러를 console.log로 catch했는데 여기서 catch문으로 진입하는 경우가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음... 가능한 경우는 navigate('/posts')
의 실행이 실패하는 경운데, 지금 해보니까 /posts
로의 라우팅 설정이 되어있지 않아도 이동 자체는 가능하다보니 catch로 넘어가지는 않는 것 같습니다. 수정할게요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
const purifyContent = (content: string): string => { | ||
const tmp = document.createElement('div'); | ||
tmp.innerHTML = content; | ||
return tmp.textContent || tmp.innerText || ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
🪄 변경사항
🖥 결과 화면
title 필드에 담긴 실제 데이터 값은 아래와 같이 JSON.parse() 메서드를 통해 JSON 객체로 변환한 뒤에 사용되어야 합니다.
✏️ PR 포인트
PR 포인트는 생략합니다!
대신 궁금한점:
API 명세 페이지 내 설명을 보고 구현하긴 했는데, 혹시 임의의 데이터 필드를 클라이언트 단에서 만들 수 있는 방법은 없겠...죠? 😅