-
Notifications
You must be signed in to change notification settings - Fork 3
[Feat/#9] 학생회비 납부자 조회 페이지 구현 #13
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
Conversation
| interface Invoice { | ||
| name: string; | ||
| student_id: string; | ||
| admin: boolean; | ||
| } |
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.
객체에 대한 타입 지정할때 interface 사용한 거 좋습니다 👍🏻
| <TableRow key={index}> | ||
| <TableCell className="w-30 text-center">{item.name}</TableCell> | ||
| <TableCell className="w-30 text-center"> | ||
| {item.student_id} | ||
| </TableCell> | ||
| <TableCell className="w-30 text-center"> | ||
| {item.admin ? '⭕' : '❌'} | ||
| </TableCell> | ||
| </TableRow> |
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.
혹시 Array.map을 이용하면서 key 값을 지정해줄 때, index가 아닌 다른 식별자 값을 사용할 수 있을까요?
물론 해당 배열이 변경될 여지가 없거나 추가로 사용할 식별자 값이 없다면 index를 사용해도 괜찮습니다.
[Kim, Lee, Park]
<ul>
<li>Kim</li>
<li>Lee</li>
<li>Park</li>
</ul>
[Choi, Kim, Lee, Park]
<ul>
<li>Choi<li> // <- 새로운 요소
<li>Kim</li>
<li>Lee</li>
<li>Park</li>
</ul>
위와 같은 경우를 예로 들면 배열의 앞에 새로운 요소가 추가됨에 따라 하나씩 key가 밀리면서 리액트는 key 0, 1, 2에 해당하는 값이 변경되고 key 3은 새로 추가되었다고 생각하게 되어 4개 모두 업데이트가 이루어지게 됩니다.
코드상으로는 student_id를 식별자로 사용해도 괜찮아보이는데 확인 한 번만 부탁 드리겠습니다 :)
| @@ -0,0 +1,25 @@ | |||
| import * as React from 'react'; | |||
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.
import {
forwardRef,
ComponentProps
} from "react";
로 Named Import 방식을 사용하도록 교체해볼 수 있을 것 같습니다. Wildcard import를 사용할 경우, 사용하지 않는 코드까지 번들에 포함될 가능성이 있어 보이기 때문입니다. Tree shaking과 관련된 내용이어서 같이 찾아보면 좋을 거 같아요.
| import * as SheetPrimitive from '@radix-ui/react-dialog'; | ||
| import { cva, type VariantProps } from 'class-variance-authority'; | ||
| import { X } from 'lucide-react'; | ||
|
|
||
| import { cn } from '@/lib/utils'; | ||
|
|
||
| const Sheet = SheetPrimitive.Root; | ||
|
|
||
| const SheetTrigger = SheetPrimitive.Trigger; | ||
|
|
||
| const SheetClose = SheetPrimitive.Close; | ||
|
|
||
| const SheetPortal = SheetPrimitive.Portal; |
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.
이 부분도 위 코멘트와 비슷한 이유로 아래와 같이 변경해보는 건 어떨까요? Named Import와 Alias 를 함께 사용하는 방법입니다.
import {
...
Portal as SheetPortal,
...
} from '@radix-ui/react-dialog';
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
Check List
✅ Key Changes
📢 To Reviewers
📸 스크린샷
🔗 참고 자료
https://velog.io/@xoxojw/Next.js-15%EC%97%90%EC%84%9C-shadcnui-%EC%84%A4%EC%B9%98-%EC%8B%9C-%EC%97%90%EB%9F%AC-%ED%95%B4%EA%B2%B0