-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor: 마이페이지 카테고리 SSR + RSC 적용 #317
base: develop
Are you sure you want to change the base?
Conversation
const userId = session.user.id; | ||
|
||
try { | ||
const categories = await db.select().from(categoriesTable).where(eq(categoriesTable.userId, userId)).all(); |
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.
db.select()
내부에서 아래와 같은 형태로 원하는 column만 가져올 수 있습니다.
db.select({
id: category.id,
title: category.title,
})
그러면 아래쪽에 나오는 필요한 필드만 선택해서 반환하는 코드를 따로 구현할 필요가 없을 것 같습니다~
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.
아하 그렇군요 !! 감사합니다!
|
||
return filteredCategories; | ||
} catch (error) { | ||
throw new Error('Failed to fetch categories'); |
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.
여기서 error를 catch해서 뭔가 다른 로직을 넣을 것이 아니라면,
catch 이후에 곧바로 throw 하는 것 보다 그냥 이 함수를 사용하는 바깥쪽에서 예외 처리를 해도 상관없지 않을까요?
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.
바깥에서 외부 처리를 하는 것이 코드를 추가적으로 작성해야하는 거라 상대적으로 더 품이 드는 행위일 것이라 생각하는데 혹시 바깥쪽에서 예외처리를 해주었을 때 좋은 점이 무엇일까요?
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.
@godhyzzang 이 코드의 경우에는 catch
문 내에서 아무런 처리를 하고 있지 않기 때문에 드린 말이었습니다.
만약 공통적으로 에러를 핸들링 하거나 로깅하는 코드가 있어서 그걸 적용하고 싶다면 여기에서 처리를 하는게 맞을텐데,
지금과 같은 경우라면 catch
문의 역할이 새로운 에러를 throw
하는 것 말고는 딱히 없기 때문이에요~
오히려 공통된 에러 메시지로 throw
하기 때문에, 이 함수를 호출하는 쪽에서는 catch
에 실제로 잡힌 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.
아하 그런 의미였군요! 이해했습니다 감사합니다~
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.
공통적으로 쓰이지 않는다면, 차라리 호출부에서 처리하는 것이 명확하게 보일 수 있을 것 같습니다! 의견 감사합니다!
if (categories && 'error' in categories) { | ||
content = ( | ||
<TableRow> | ||
<TableCell colSpan={4}>에러가 났습니다잇..</TableCell> | ||
</TableRow> | ||
); | ||
} else if (categories === null) { | ||
content = ( | ||
<TableRow> | ||
<TableCell colSpan={4}> | ||
<Skeleton variant="rectangular" width="100%" height="100%" /> | ||
</TableCell> | ||
</TableRow> | ||
); | ||
} else if (Array.isArray(categories)) { | ||
content = <CategoryTableInfo categoryList={categories} />; | ||
} | ||
|
||
return <TableBody>{content}</TableBody>; |
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.
if-else문을 사용하는 것보다 아래와 같이 early return 하는 형태도 고려해보시면 좋을 것 같습니다~
if (conditiion1) {
return ...
}
if (conditiion2) {
return ...
}
return ...
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
서버 컴포넌트를 잘 적용하신 것 같고, 기능 상에 오류가 없는지만 잘 체크하시고 merge하면 되지 않을까 싶습니다.
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.
고생하셨습니다!
if (categories === null) { | ||
return ( | ||
<TableBody> |
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.
p4: category가 null일 때 Skeleton이 보이지 않습니다. 확인해보니 height이 0이 되어있는데, 나중에 수정이 필요할 것 같습니다.
What is this PR? 🔍
Changes 📝
CategoryTableBody
는 서버 컴포넌트로 서버의 데이터를 가져오고 상황에 맞는 적절한 컴포넌트를 보여주는 역할을 합니다.CategoryTableBody
역할인 카테고리 데이터를 보여주는 컴포넌트는CategoryTableInfo
가 합니다.const { categoryList: data } = useGetCategoryList(categoryList);
인자로 initialData를 넘깁니다.ScreenShot 📷
기존
수정후
Precaution
RSC를 최대한 이해하고 사용하려 노력하였습니다. 그러나 적절하게 사용하지 못한 부분도 있을 것같습니다.
다양한 피드백 주시면 더욱 공부하고 적용해보겠습니다!!