Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[김세동] week20 #1077
[김세동] week20 #1077
Changes from all commits
1ce593d
ed8e055
a2c0105
a8c090e
845cb8c
8116f98
b96f2c1
57abd97
beec6c8
61092b0
49526ec
e914c20
3267fff
3f6061a
9dba7a1
143d9a3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
@Rhajiit
불필요한 상태들이 있고, 한 콤포넌트에서 너무 많은 역할을 가져가고 있습니다
조금 더 구조화를 해보고 콤포넌트를 작성해야 할 것 같아요
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.
@Rhajiit
nextjs useRouter에 shallow가 있던가요?
https://nextjs.org/docs/app/api-reference/functions/use-router#userouter
또한, router push로 인해 렌더될 페이지가 변경되게 되는데
이 이후에 비동기 로직이 실행된다면 실행을 보장하기 어렵지 않을까 생각이 들어요. 어떻게 생각하시나요?
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.
folder 페이지를 [id]로 관리하기위해 전환하는 과정에서 불필요한 로직은 뺏다고 생각을 했는데, 아직 남아있었네요..? 확인해보고 수정하겠습니다!
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.
@Rhajiit
folder id가 있는경우와 없는경우가 달라져야 하는 상황인거죠?
이에대한 핸들링은 오늘 이야기 했던것처럼 service레이어가 알아서 관리하도록 하는게 더 좋지 않을까 생각이 들어요.
한번 리팩토링 해볼까요?
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.
@Rhajiit
필터 값에 따라 데이터를 활용하는 경우, 이렇게 필터된 데이터를 상태로 관리하기보단, 메모화된 상수, 또는 그냥 상수로 관리하는게 더 좋습니다!
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.
@Rhajiit
observer 등록 cleanup이 필요할 것 같아요
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.
@Rhajiit
무적의 FolderUI 콤포넌트가 탄생했군요
콤포넌트 하나에서 모든걸 다 처리하도록 하기보다, 관심사에 맞춰서 기능단, ui단으로 분리해주고
이렇게 분리된 콤포넌트를 조립하는 형태로 개편하는게 좋을 것 같아요
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.
기존에 작성된 코드를 뜯어고치는게 쉽지는 않네요... 알겠습니다!