[Step5] hippo - API 요청과 비동기 처리 - #7
Open
meteorqz6 wants to merge 13 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough로컬 상수 기반의 레스토랑 데이터를 json-server REST API로 교체했습니다. ChangesREST API 연동 및 컴포넌트 리팩토링
문서 및 설정 업데이트
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over App: 초기 렌더링
App->>useRestaurants: useRestaurants() 호출
useRestaurants->>api.js: getRestaurants()
api.js->>json-server: GET /restaurants
json-server-->>api.js: 레스토랑 목록 반환
api.js-->>useRestaurants: restaurants 배열
useRestaurants-->>App: { restaurants, addRestaurant }
end
rect rgba(60, 179, 113, 0.5)
Note over App: 레스토랑 추가
App->>useRestaurants: addRestaurant(newRestaurant) await
useRestaurants->>api.js: createRestaurant(newRestaurant)
api.js->>json-server: POST /restaurants
json-server-->>api.js: 201 응답
api.js-->>useRestaurants: 완료
useRestaurants->>api.js: getRestaurants() 재호출
api.js->>json-server: GET /restaurants
json-server-->>api.js: 갱신된 목록
api.js-->>useRestaurants: 갱신된 restaurants
useRestaurants-->>App: 모달 닫기
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
meteorqz6
marked this pull request as ready for review
June 14, 2026 09:48
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
05-effects/README.md (1)
59-63:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win코드 펜스에 언어를 지정하세요.
Line 59-63의 코드 블록이 언어 없이 선언되어 있습니다. 마크다운 린터 경고를 해결하고 구문 강조를 활성화하려면 언어를 지정해야 합니다.
🔧 제안 수정
### `async/await`를 어디에 붙여야 하는가 `await`는 Promise를 반환하는 함수 앞에 붙인다. `await`를 쓰는 함수 자신은 반드시 `async`여야 한다. 이 규칙이 호출 체인을 따라 전파된다. -``` +```text fetch() → Promise 반환 getRestaurants() → 내부에서 await fetch() → async 필요 handleRestaurantSubmit() → 내부에서 await getRestaurants() → async 필요🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@05-effects/README.md` around lines 59 - 63, The code block on lines 59-63 in the README.md file is missing a language identifier in the markdown code fence. Add the language identifier "text" immediately after the opening triple backticks to enable syntax highlighting and resolve markdown linter warnings. Change the opening fence from triple backticks with no language to triple backticks followed by "text" before the code block containing the fetch, getRestaurants, and handleRestaurantSubmit function descriptions.Source: Linters/SAST tools
🧹 Nitpick comments (4)
src/api.js (2)
15-27: ⚡ Quick win생성된 레스토랑 데이터 반환을 고려하세요.
createRestaurant가 생성된 데이터를 반환하지 않습니다. 서버가id나 타임스탬프를 할당하는 경우, 반환값을 활용하면 전체 목록을 다시 조회하지 않고도 UI를 즉시 업데이트할 수 있어 네트워크 효율성이 향상됩니다.♻️ 제안하는 개선 방안
export async function createRestaurant(restaurant) { try { const response = await fetch(`${BASE_URL}/restaurants`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(restaurant), }); if (!response.ok) throw new Error(`서버 오류: ${response.status}`); + const createdRestaurant = await response.json(); + return createdRestaurant; } catch (error) { console.error("음식점 추가 실패:", error); throw error; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api.js` around lines 15 - 27, The createRestaurant function does not return the restaurant data created by the server. To improve network efficiency and enable immediate UI updates, parse the response body as JSON in the createRestaurant function and return the created restaurant object (which will include server-assigned data like id and timestamp) so the caller can use it without refetching the entire list. This change should occur after the response validation in the try block, before the catch block.
3-27: 에러 핸들링 접근 방식에 대한 피드백PR 설명에서 try/catch 에러 핸들링 접근 방식에 대한 피드백을 요청하셨습니다. 현재 구현은 올바른 패턴입니다:
✅ 잘된 점:
- API 모듈에서 에러를 잡아 로깅한 후 다시 던지는 것은 라이브러리 코드의 표준 패턴입니다
- 호출하는 쪽(useRestaurants, App)에서 에러를 처리할 수 있도록 제어권을 전달합니다
response.ok체크로 HTTP 에러를 명시적으로 처리합니다💡 선택적 개선 사항:
- 에러에 더 많은 컨텍스트를 추가할 수 있습니다 (예:
error.message에 URL 포함)- 특정 HTTP 상태 코드별로 다른 에러 타입을 던질 수 있습니다 (401, 404, 500 등)
현재 패턴이 학습 목적과 이 프로젝트 규모에 적합합니다. 더 정교한 에러 핸들링이나 커스텀 에러 클래스 생성을 원하시면 도움을 드릴 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api.js` around lines 3 - 27, To implement the optional error handling improvements, enhance both getRestaurants() and createRestaurant() functions with more detailed error context and status code-specific handling. For error context, include the request URL in the error message passed to console.error and the thrown Error object. For status-specific handling, check the response.status before throwing to create different error types or messages for common HTTP errors like 401 (Unauthorized), 404 (Not Found), and 500 (Server Error), allowing calling code to handle different failure scenarios appropriately. This maintains the current pattern of logging and re-throwing while providing richer debugging information and more granular error handling capabilities.src/components/RestaurantList/RestaurantList.module.css (1)
13-24: 💤 Low value스타일 린터 경고를 해결하세요.
Stylelint에서 선언 전 빈 줄에 대한 경고를 발생시키고 있습니다 (lines 17, 20). 이는 기능에 영향을 주지 않지만 코드 스타일 일관성을 위해 수정하는 것이 좋습니다.
🎨 제안하는 수정 방안
.restaurant__button { display: flex; align-items: flex-start; - width: 100%; padding: 16px 8px; - background: none; border: none; cursor: pointer; text-align: left; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/RestaurantList/RestaurantList.module.css` around lines 13 - 24, The `.restaurant__button` CSS class has empty lines before property declarations that are triggering Stylelint warnings. Remove the blank lines that appear before the `width` property and before the `background` property to maintain consistent formatting without unnecessary blank lines between CSS property declarations within the rule.Source: Linters/SAST tools
src/hooks/useRestaurants.js (1)
4-22: 커스텀 훅 생성에 대한 피드백PR 설명에서 커스텀 훅을 언제, 어떻게 만들어야 하는지에 대한 피드백을 요청하셨습니다. 현재
useRestaurants훅은 매우 잘 설계되었습니다!✅ 커스텀 훅을 만들기 좋은 경우 (현재 케이스가 해당):
- 관련된 상태와 로직을 함께 캡슐화 (restaurants 상태 + fetch/add 로직)
- 여러 컴포넌트에서 재사용 가능한 로직
- 비즈니스 로직을 UI 로직과 분리
- 복잡한 effect나 여러 hook의 조합을 단순화
✅ 잘된 점:
useCallback으로fetchRestaurants를 최적화useEffect에서 초기 데이터 로딩- 명확한 인터페이스 (
restaurants,addRestaurant)💡 개선 제안:
- 로딩 상태 추가:
const [loading, setLoading] = useState(false)- 에러 상태 추가 (위 코멘트 참조)
- 반환값 확장:
{ restaurants, addRestaurant, loading, error }이러한 개선사항을 구현하는 데 도움이 필요하시면 말씀해 주세요!
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useRestaurants.js` around lines 4 - 22, The useRestaurants hook needs to track loading and error states to provide more complete state management. Add two new state variables using useState for loading and error states. Update the fetchRestaurants function to set loading to true before the async call, false after completion, and capture any errors that occur during the getRestaurants call. Apply the same loading and error handling pattern to the addRestaurant function. Finally, expand the return object to include the new loading and error states alongside restaurants and addRestaurant so consuming components can handle pending and failed states appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api.js`:
- Line 1: The BASE_URL constant is hardcoded with a local development value,
which limits flexibility across different deployment environments. Replace the
hardcoded string value in BASE_URL with a reference to the environment variable
using import.meta.env.VITE_API_URL, which will allow the API URL to be
configured differently for development, staging, and production environments
through environment configuration files.
In `@src/App.jsx`:
- Around line 41-44: The handleRestaurantSubmit function lacks error handling
for the addRestaurant call, causing silent failures where the modal closes
regardless of success or failure, leaving users unaware of issues. Wrap the
await addRestaurant(restaurant) call in a try-catch block, only calling
setIsAddRestaurantModalOpen(false) in the success path (try block), and handle
the catch block by either displaying an error message to the user via a toast
notification or managing an error state so users receive feedback about what
went wrong.
In `@src/hooks/useRestaurants.js`:
- Around line 16-19: The addRestaurant function in the useRestaurants hook lacks
error handling, which means when createRestaurant or fetchRestaurants fails, the
error propagates silently without user notification. Add a try/catch block
within the addRestaurant function to catch any errors from createRestaurant or
fetchRestaurants, then re-throw the error so that the calling component (App)
can handle it with proper error handling and user notification. This ensures
users are informed when restaurant addition fails.
- Around line 7-14: The fetchRestaurants callback function lacks error handling
when calling getRestaurants(), which means any API errors propagate silently
without user notification or state tracking. Wrap the getRestaurants() call in a
try-catch block, capture any errors that occur, and store them in an error state
variable using a new useState hook for error management. Return the error state
along with restaurants from the useRestaurants hook so that calling components
can properly handle and display errors to users. This prevents failures from
going undetected while leaving restaurants as an empty array.
---
Outside diff comments:
In `@05-effects/README.md`:
- Around line 59-63: The code block on lines 59-63 in the README.md file is
missing a language identifier in the markdown code fence. Add the language
identifier "text" immediately after the opening triple backticks to enable
syntax highlighting and resolve markdown linter warnings. Change the opening
fence from triple backticks with no language to triple backticks followed by
"text" before the code block containing the fetch, getRestaurants, and
handleRestaurantSubmit function descriptions.
---
Nitpick comments:
In `@src/api.js`:
- Around line 15-27: The createRestaurant function does not return the
restaurant data created by the server. To improve network efficiency and enable
immediate UI updates, parse the response body as JSON in the createRestaurant
function and return the created restaurant object (which will include
server-assigned data like id and timestamp) so the caller can use it without
refetching the entire list. This change should occur after the response
validation in the try block, before the catch block.
- Around line 3-27: To implement the optional error handling improvements,
enhance both getRestaurants() and createRestaurant() functions with more
detailed error context and status code-specific handling. For error context,
include the request URL in the error message passed to console.error and the
thrown Error object. For status-specific handling, check the response.status
before throwing to create different error types or messages for common HTTP
errors like 401 (Unauthorized), 404 (Not Found), and 500 (Server Error),
allowing calling code to handle different failure scenarios appropriately. This
maintains the current pattern of logging and re-throwing while providing richer
debugging information and more granular error handling capabilities.
In `@src/components/RestaurantList/RestaurantList.module.css`:
- Around line 13-24: The `.restaurant__button` CSS class has empty lines before
property declarations that are triggering Stylelint warnings. Remove the blank
lines that appear before the `width` property and before the `background`
property to maintain consistent formatting without unnecessary blank lines
between CSS property declarations within the rule.
In `@src/hooks/useRestaurants.js`:
- Around line 4-22: The useRestaurants hook needs to track loading and error
states to provide more complete state management. Add two new state variables
using useState for loading and error states. Update the fetchRestaurants
function to set loading to true before the async call, false after completion,
and capture any errors that occur during the getRestaurants call. Apply the same
loading and error handling pattern to the addRestaurant function. Finally,
expand the return object to include the new loading and error states alongside
restaurants and addRestaurant so consuming components can handle pending and
failed states appropriately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 18342010-de44-499c-a510-16820c1f967c
📒 Files selected for processing (12)
.coderabbit.yaml05-effects/README.mdREADME.mdsrc/App.jsxsrc/api.jssrc/components/AddRestaurantModal/AddRestaurantModal.jsxsrc/components/CategoryFilter/CategoryFilter.jsxsrc/components/RestaurantList/RestaurantList.jsxsrc/components/RestaurantList/RestaurantList.module.csssrc/constants/categories.jssrc/constants/restaurants.jssrc/hooks/useRestaurants.js
💤 Files with no reviewable changes (1)
- src/constants/restaurants.js
ehlung
reviewed
Jun 14, 2026
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(restaurant), | ||
| }); | ||
| if (!response.ok) throw new Error(`서버 오류: ${response.status}`); |
Contributor
There was a problem hiding this comment.
[배움]
리드미에서 에러 처리를 고민하신 걸 봤는데, 최종적으로 api는 어차피 호출부에 throw를 해줘야 하니까 catch를 제거해서 불필요한 코드를 삭제하신 부분 좋은 것 같아요! '에러를 처리할 수 있는 곳에서만 잡는다'는 원칙도 좋은 기준인 것 같습니다!
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| void fetchRestaurants(); |
Contributor
There was a problem hiding this comment.
[배움]
fetchRestaurants()를 void로 호출하신 부분 좋은 것 같아요. async 함수를 useEffect 안에서 호출하면 Promise가 반환되는데, React는 cleanup 함수(또는 undefined)만 기대하기 때문에 void로 반환값을 명시적으로 버리는 패턴이군요! 저도 적용해보겠습니다.
Comment on lines
+16
to
+18
| } finally { | ||
| setIsLoading(false); | ||
| } |
Contributor
There was a problem hiding this comment.
[배움]
요구사항에 없는 로딩 상태까지 구현하신 게 인상적이에요! setIsLoading(false)를 try/catch 양쪽에 중복으로 쓰는 대신 finally로 한 번만 쓴 것도 깔끔하고 좋은 것 같아요. 성공/실패와 무관하게 반드시 실행돼야 하는 코드는 finally에 두는 패턴 좋은 기준인 것 같아요.
| @@ -1 +1,2 @@ | |||
| export const ALL_CATEGORY = "전체"; | |||
Contributor
There was a problem hiding this comment.
[배움]
저번 미션에서 매직 스트링을 리팩토링했으면서도 "전체" 문자열이 매직 스트링이라고 인지를 못하고 있었어요. 카테고리 상수에서 관리하는 것 좋은 것 같아요! 저도 적용해보겠습니다!
Comment on lines
+5
to
+12
| useEffect(() => { | ||
| function handleKeyDown(e) { | ||
| if (e.key === "Escape") onClose(); | ||
| } | ||
| document.addEventListener("keydown", handleKeyDown); | ||
| return () => document.removeEventListener("keydown", handleKeyDown); | ||
| }, [onClose]); | ||
|
|
Contributor
There was a problem hiding this comment.
[배움]
키보드 사용자도 모달을 닫을 수 있도록 접근성까지 고려한 게 진짜 꼼꼼하신 것 같아요! cleanup 함수로 이벤트 리스너를 제거하지 않으면 모달을 열고 닫을 때마다 리스너가 누적된다는 건 미처 생각 못했는데, 컴포넌트가 사라져도 자동으로 정리되지 않는 것들은 반드시 cleanup이 필요하다는 것 덕분에 알게 됐어요.
| onClick={onAddButtonClick} | ||
| > | ||
| <img src={addButton} alt="음식점 추가" /> | ||
| <img src={addButton} /> |
Contributor
There was a problem hiding this comment.
[배움]
그러고 보니 버튼에 aria-label이 있는데 이미지에도 alt가 있으면 스크린 리더가 중복해서 읽게 되겠네요. 템플릿을 옮겨오는 과정에서 별 생각 없이 적용한 코드였는데, 접근성까지 챙겨서 리팩토링하신 부분이 인상깊어요. 저도 제거해볼게요!
ehlung
reviewed
Jun 14, 2026
| @@ -0,0 +1,16 @@ | |||
| const BASE_URL = "http://localhost:3000"; | |||
Contributor
There was a problem hiding this comment.
[배움]
http://localhost:3000이 두 곳에 반복되는 걸 상수로 추출하신 부분 실무에서도 자주 쓰이는 패턴인 것 같아요. 저도 적용해볼게요!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
개인 목표 달성 여부
리뷰어에게
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
문서