-
Notifications
You must be signed in to change notification settings - Fork 8
1주차 과제 제출 - 강유진 #4
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| declare namespace JSX { | ||
| interface IntrinsicElements { | ||
| [elemName: string]: any; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,138 @@ | ||
| const root = document.getElementById('root'); | ||
| import { createElement } from "./utils/jsx"; | ||
|
|
||
| interface Ticket { | ||
| id: number; | ||
| title: string; | ||
| description: string; | ||
| status: "open" | "closed"; | ||
| toggle(): void; | ||
| } | ||
|
|
||
| function Header() { | ||
| return ( | ||
| <header> | ||
| <h1>Tickets</h1> | ||
| </header> | ||
| ); | ||
| } | ||
|
|
||
| function Main({ | ||
| tickets, | ||
| addTicket, | ||
| }: { | ||
| tickets: Ticket[]; | ||
| addTicket: ({ title, description }: { title: string; description: string }) => void; | ||
| }) { | ||
| return ( | ||
| <main> | ||
| <TicketList tickets={tickets} /> | ||
| <TicketForm addTicket={addTicket} /> | ||
| </main> | ||
| ); | ||
| } | ||
|
|
||
| function TicketList({ tickets }: { tickets: Ticket[] }) { | ||
| return ( | ||
| <ul id="ticket-list"> | ||
| {tickets.map((ticket) => ( | ||
| <TicketItem ticket={ticket} /> | ||
| ))} | ||
| </ul> | ||
| ); | ||
| } | ||
|
|
||
| function TicketItem({ ticket }: { ticket: Ticket }) { | ||
| const handleClick = () => { | ||
| ticket.toggle(); | ||
| }; | ||
|
|
||
| return ( | ||
| <li> | ||
| <div className="title">{ticket.title}</div> | ||
| <div className="description">{ticket.description}</div> | ||
| <button className="status" onClick={handleClick}> | ||
| {ticket.status === "open" ? "Open" : "Closed"} | ||
| </button> | ||
| </li> | ||
| ); | ||
| } | ||
|
|
||
| function TicketForm({ | ||
| addTicket, | ||
| }: { | ||
| addTicket: ({ title, description }: { title: string; description: string }) => void; | ||
| }) { | ||
| const handleSubmit = (event: Event) => { | ||
| event.preventDefault(); | ||
|
|
||
| const form = event.target as HTMLFormElement; | ||
| const formData = new FormData(form); | ||
| const title = formData.get("title") as string; | ||
| const description = formData.get("description") as string; | ||
|
Comment on lines
+69
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding type assertions or validation to ensure that the form data is of the expected type. This can help prevent runtime errors and improve the overall robustness of the application. const title = formData.get("title") as string;
const description = formData.get("description") as string;
if (!title || !description) {
console.error("Title or description is missing");
return;
} |
||
|
|
||
| addTicket({ title, description }); | ||
| }; | ||
|
|
||
| return ( | ||
| <form id="add-ticket-form" onSubmit={handleSubmit}> | ||
| <div> | ||
| <label for="ticket-title">Title</label> | ||
| <input type="text" name="title" id="ticket-title" placeholder="Title" /> | ||
| </div> | ||
| <div> | ||
| <label for="ticket-description">Description</label> | ||
| <textarea name="description" id="ticket-description" placeholder="Description"></textarea> | ||
| </div> | ||
| <button type="submit" id="add-ticket"> | ||
| Add Ticket | ||
| </button> | ||
| </form> | ||
| ); | ||
| } | ||
|
|
||
| function render({ | ||
| root, | ||
| tickets, | ||
| addTicket, | ||
| }: { | ||
| root: HTMLElement; | ||
| tickets: Ticket[]; | ||
| addTicket: ({ title, description }: { title: string; description: string }) => void; | ||
| }) { | ||
| root.replaceChildren( | ||
| <div> | ||
| <Header /> | ||
| <Main tickets={tickets} addTicket={addTicket} /> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| const root = document.getElementById("root"); | ||
|
|
||
| if (root) { | ||
| root.innerHTML = '<p>Hello, world!</p>'; | ||
| const tickets: Ticket[] = []; | ||
|
|
||
| const update = () => { | ||
| render({ root, tickets, addTicket }); | ||
| }; | ||
|
|
||
| const addTicket = ({ title, description }: { title: string; description: string }) => { | ||
| const id = Math.max(...tickets.map((ticket) => ticket.id), 0) + 1; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation for generating ticket IDs is not ideal. It relies on finding the maximum existing ID and incrementing it, which is not thread-safe and can lead to collisions in concurrent environments. Consider using a more robust ID generation strategy, such as a UUID generator or a server-side auto-incrementing ID. const id = Date.now(); // Using timestamp for simplicity, consider UUID for production |
||
| const ticket: Ticket = { | ||
| id, | ||
| title, | ||
| description, | ||
| status: "open", | ||
| toggle() { | ||
| this.status = this.status === "open" ? "closed" : "open"; | ||
| update(); | ||
| }, | ||
| }; | ||
|
|
||
| tickets.push(ticket); | ||
|
|
||
| update(); | ||
| }; | ||
|
|
||
| update(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||||||||||||||||||||||||||
| export function createElement(type: string | Function, props: any, ...children: any[]) { | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While the custom export function createElement(type: string | Function, props: Record<string, any> | null, ...children: any[]) {There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 함수 타입 선언 개선이 필요합니다. 'Function' 타입은 너무 광범위하며, 타입 안전성 문제를 일으킬 수 있습니다. 정확한 함수 형태를 명시적으로 정의하는 것이 좋습니다. 다음과 같이 개선해 보세요: - export function createElement(type: string | Function, props: any, ...children: any[]) {
+ export function createElement(
+ type: string | ((props: Record<string, any>) => HTMLElement),
+ props: Record<string, any> | null,
+ ...children: any[]
+ ) {📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 1-1: Don't use 'Function' as a type. Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs. (lint/complexity/noBannedTypes) |
||||||||||||||||||||||||||||||
| if (typeof type === "function") { | ||||||||||||||||||||||||||||||
| return type({ ...props, children }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const element = document.createElement(type); | ||||||||||||||||||||||||||||||
| Object.assign(element, props); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 속성 할당 방법 개선이 필요합니다.
다음과 같이 개선해 보세요: const element = document.createElement(type);
- Object.assign(element, props);
+ if (props) {
+ Object.entries(props).forEach(([key, value]) => {
+ if (key === 'className' && typeof value === 'string') {
+ element.className = value;
+ } else if (key === 'style' && typeof value === 'object') {
+ Object.assign(element.style, value);
+ } else if (!key.startsWith('on') && typeof value !== 'function' && typeof value !== 'object') {
+ element.setAttribute(key, value);
+ }
+ });
+ }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
| if (props) { | ||||||||||||||||||||||||||||||
| Object.entries(props).forEach(([key, value]) => { | ||||||||||||||||||||||||||||||
| if (key.startsWith("on") && typeof value === "function") { | ||||||||||||||||||||||||||||||
| const event = key.slice(2).toLocaleLowerCase(); | ||||||||||||||||||||||||||||||
| element.addEventListener(event, value as EventListener); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| children.forEach((child) => { | ||||||||||||||||||||||||||||||
| if (Array.isArray(child)) { | ||||||||||||||||||||||||||||||
| child.forEach((childItem) => element.append(childItem)); | ||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| element.append(child); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return element; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import { defineConfig } from "vite"; | ||
|
|
||
| export default defineConfig({ | ||
| esbuild: { | ||
| jsxFactory: "createElement", | ||
| }, | ||
| }); |
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.
TicketList의 반복 렌더링
tickets 배열을 map으로 순회하고 있습니다. React 구조에서는 key 속성을 생략하면 성능 최적화와 경고 메시지 측면에서 문제가 있을 수 있습니다.
아래와 같이 key를 추가해 주세요:
📝 Committable suggestion
🧰 Tools
🪛 Biome (1.9.4)
[error] 38-38: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)