Open
Conversation
📝 WalkthroughWalkthrough세 개의 완전한 풀스택 연습 프로젝트를 추가합니다: TMDB API 통합 영화 검색 앱(mission1)과 두 가지 폼 검증 접근 방식의 인증 프로젝트(mission2, mission3)입니다. React Router, TypeScript, Vite, Tailwind CSS로 구성되어 있습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (6)
Week04/gureum/mission1/src/pages/HomePage.tsx (1)
1-7:HomePage와PopularPage가 동일 구현이라 중복을 줄이는 편이 좋습니다.동일 컴포넌트를 두 파일로 유지하면 이후 제목/동작 변경 시 불일치 위험이 생깁니다.
HomePage는PopularPage를 재사용하도록 단순화하는 것을 권장합니다.리팩터 예시
-import MoviesPage from '../components/MoviesPage'; - -const HomePage = () => { - return <MoviesPage endpoint="popular" title="인기 영화" />; -}; - -export default HomePage; +export { default } from './PopularPage';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission1/src/pages/HomePage.tsx` around lines 1 - 7, HomePage duplicates PopularPage; update HomePage to reuse PopularPage instead of reimplementing the same JSX: import PopularPage (the existing component name) and either return <PopularPage /> from HomePage or simply default-export PopularPage from the PopularPage module and change HomePage to re-export it; ensure you remove duplicated props/title hardcoding in HomePage and keep MoviesPage usage centralized in PopularPage to avoid future divergence.Week04/gureum/mission1/package.json (1)
12-19:@tailwindcss/vite는 devDependencies로 이동하는 것이 좋습니다.
@tailwindcss/vite는 빌드 시에만 사용되는 플러그인이므로devDependencies에 위치하는 것이 적절합니다. 프로덕션 번들에는 포함되지 않아야 합니다.💡 권장 수정 사항
"dependencies": { - "@tailwindcss/vite": "^4.2.2", "axios": "^1.14.0", "react": "^19.2.4", "react-dom": "^19.2.4", "react-router-dom": "^7.13.2", "tailwindcss": "^4.2.2" }, "devDependencies": { + "@tailwindcss/vite": "^4.2.2", "@eslint/js": "^9.39.4",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission1/package.json` around lines 12 - 19, The dependency "@tailwindcss/vite" is listed under "dependencies" but should be a dev-only plugin; move the entry for "@tailwindcss/vite" from the "dependencies" object to "devDependencies" in package.json (keep the same version string "^4.2.2"), then reinstall (npm/yarn) so the lockfile updates; ensure no other build-time plugins are left under "dependencies".Week04/gureum/mission1/README.md (1)
1-73: Vite 템플릿 기본 README입니다.현재 문서는 Vite 템플릿의 기본 내용입니다. 프로젝트의 실제 기능(TMDB API 연동, 영화 검색/상세 조회 등)과 실행 방법을 설명하는 내용으로 업데이트하면 프로젝트 이해도가 높아질 것입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission1/README.md` around lines 1 - 73, Update the README.md to replace the generic Vite template content with project-specific documentation: describe the app purpose (TMDB integration for movie search and detail pages), list key features (search, detail view, pagination, favorites), specify required environment variables (e.g., VITE_TMDB_API_KEY), add exact run/build/test commands (dev, build, preview, lint, test) and example usage, and include a brief project structure (src/components, src/pages, src/api) and troubleshooting notes so contributors can run and understand the codebase (refer to README.md, mention TMDB API usage and related scripts in package.json).Week04/gureum/mission1/src/App.tsx (1)
15-25: LGTM! 라우터 구성이 적절합니다.
createBrowserRouter와 중첩 라우트 구조가 올바르게 설정되어 있습니다.Layout컴포넌트가<Outlet />을 사용하여 자식 라우트를 렌더링하는 것도 확인됩니다.선택적으로, 존재하지 않는 경로에 대한 404 페이지를 추가하면 사용자 경험이 향상될 수 있습니다:
💡 404 라우트 추가 예시
<Route path="movies/:movieId" element={<MovieDetailPage />} /> + <Route path="*" element={<NotFoundPage />} /> </Route>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission1/src/App.tsx` around lines 15 - 25, Add a catch-all 404 route to the router so unknown paths render a NotFound page: update the createBrowserRouter / createRoutesFromElements setup that uses Route and Layout to include a Route with path="*" (or an explicit NotFound route) as a child of the root Route, and create/import a NotFoundPage component to render there; ensure Layout (and its Outlet) remains unchanged so the 404 page is rendered within the existing layout.Week04/gureum/mission1/src/hooks/useMovies.ts (1)
6-27: 카테고리 변경 시 페이지를 1로 초기화하는 것을 권장합니다.현재는
type이 바뀌어도 이전 페이지 번호를 유지해서, 새 카테고리 진입 시 UX가 다소 어색할 수 있습니다.🔧 제안 수정안
-import { useCallback, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; @@ export const useMovies = (type: MovieListType) => { const [currentPage, setCurrentPage] = useState(1); + + useEffect(() => { + setCurrentPage(1); + }, [type]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission1/src/hooks/useMovies.ts` around lines 6 - 27, When the movie category `type` changes, reset pagination by calling `setCurrentPage(1)` so users always start at page 1 for a new category; add a small effect in `useMovies` (useEffect watching `type`) that invokes `setCurrentPage(1)` (and ensure `type` is in the dependency array) so `currentPage` is cleared when `type` changes while keeping the existing `fetcher`, `useFetch` and memoized `movies`/`totalPages` logic intact.Week04/gureum/mission1/src/components/MoviesPage.tsx (1)
56-57: 레이아웃 간격은<br />대신 클래스 기반으로 관리해주세요.UI 간격 제어를 마크업 요소로 하지 않고
mb-*,mt-*등으로 통일하면 구조와 스타일이 더 명확해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission1/src/components/MoviesPage.tsx` around lines 56 - 57, In MoviesPage.tsx remove the literal <br /> used for spacing and instead apply a margin utility class (e.g., mb-4 / mt-4 or the project’s spacing token) to the appropriate JSX element(s) in the MoviesPage component (wrap/div or the element immediately before/after the former <br />) so spacing is controlled via classes like "mb-*" or "mt-*" rather than markup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Week04/gureum/mission1/index.html`:
- Line 2: Update the HTML root element's language attribute from lang="en" to
lang="ko" to match the Korean UI; locate the <html lang="en"> tag in index.html
and change it to <html lang="ko"> so assistive technologies and browsers use the
correct language locale.
In `@Week04/gureum/mission1/src/api/tmdb.ts`:
- Around line 9-23: The code only validates TMDB_TOKEN but not BASE_URL, causing
requests to use a relative path if VITE_TMDB_BASE_URL is missing; update
validation so BASE_URL is checked as well (e.g., extend or replace ensureToken
with ensureConfig that verifies both BASE_URL and TMDB_TOKEN and throws a clear
Error when BASE_URL is falsy) and call that validation before creating
tmdbClient (or delay axios.create until after ensureConfig) so tmdbClient is
always initialized with a valid baseURL.
In `@Week04/gureum/mission1/src/components/movie-detail/CastGrid.tsx`:
- Around line 17-22: The img src fallback uses a likely-invalid path
`/api/placeholder/150/225`; update CastGrid.tsx to use a real placeholder by
either importing a local default avatar (e.g., import defaultAvatar from '...')
and using that when actor.profile_path is falsy, or point the fallback to a
guaranteed external placeholder URL (e.g., a CDN or project assets). Adjust the
conditional around actor.profile_path in the <img> src expression so it uses the
new default (reference: actor.profile_path, CastGrid component) and ensure the
project bundler can resolve the chosen asset.
In `@Week04/gureum/mission1/src/components/movie-detail/MovieHero.tsx`:
- Around line 26-28: In MovieHero's image onError handler, prevent a fallback
loop by removing the error handler before changing the src; locate the onError
handler attached to the img (the arrow function using event.currentTarget.src)
and update it to first set event.currentTarget.onerror = null (or remove the
handler) and then assign the fallback URL (e.g., '/api/placeholder/300/450'),
optionally guarding by checking that the current src is not already the fallback
to avoid redundant work.
In `@Week04/gureum/mission1/src/components/movie-detail/MovieMetaCards.tsx`:
- Around line 47-54: MovieMetaCards currently injects movie.homepage directly
into the anchor href (in the MovieMetaCards component, variable movie.homepage),
leaving a potential XSS vector; add a validation step (e.g., an isSafeUrl helper
that uses the URL constructor and checks url.protocol is "http:" or "https:")
and only set href when the helper returns true; if validation fails, render
either no anchor or a non-clickable fallback (plain text or disabled link) so
untrusted schemes are never placed into the href or opened with target="_blank".
In `@Week04/gureum/mission1/src/components/movies/MovieGrid.tsx`:
- Around line 17-24: The img src currently builds a URL using movie.poster_path
which can be null; update the MovieGrid img src expression to use
movie.poster_path conditionally (e.g., if movie.poster_path is truthy use
`https://image.tmdb.org/t/p/w500${movie.poster_path}` otherwise use the local
placeholder `/api/placeholder/300/450`) so you don't create URLs like
".../null"; keep the existing onError handler as a secondary fallback to handle
broken images.
- Around line 21-23: The onError handler in MovieGrid.tsx (and the similar
handlers in CastGrid.tsx and MovieHero.tsx) points to a non-existent internal
path '/api/placeholder/300/450'; replace that fallback with a valid placeholder
URL or a public static asset: either change event.currentTarget.src to an
external service like 'https://placehold.co/300x450' or add a static image
(e.g., placeholder-300x450.png) to the public folder and set the src to
'/placeholder-300x450.png'; update the onError handlers in MovieGrid, CastGrid,
and MovieHero to use the chosen valid path.
In `@Week04/gureum/mission1/src/components/movies/Pagination.tsx`:
- Around line 16-42: The previous/next buttons currently render only "<" and ">"
which is unclear to assistive tech; update the two button elements that call
onPrev and onNext (and use currentPage/totalPages for disabled logic) to include
descriptive ARIA labels (e.g., aria-label="Previous page" and aria-label="Next
page"); also ensure the disabled state remains set (and optionally add
aria-disabled when currentPage === 1 or currentPage >= totalPages) so screen
readers get both the action and disabled information.
In `@Week04/gureum/mission1/src/components/Navbar.tsx`:
- Around line 20-22: The NavLink for the home route in Navbar.tsx (the NavLink
with to="/" and className={navClassName}) should include the end prop so it only
becomes active on the exact "/" path; update the NavLink component to add end to
its props to prevent prefix matching (so "/popular" or "/upcoming" won't mark
the home link active).
In `@Week04/gureum/mission1/src/hooks/useFetch.ts`:
- Around line 31-46: The hook can suffer from stale responses overwriting newer
state because only isMountedRef is checked; modify the request flow in useFetch
so each fetch call has a unique requestId (or use an AbortController) and only
apply setData/setError/setIsLoading if the current requestId matches the most
recent one (or the signal is not aborted). Concretely, generate/increment a
local requestId (or create and store an AbortController) before calling
fetcher(), capture it in the try/catch/finally, and guard the setData, setError,
and setIsLoading calls with that requestId check (or controller.signal.aborted
check) to ensure only the latest response updates state while retaining existing
isMountedRef checks.
In `@Week04/gureum/mission1/src/types/movie.ts`:
- Around line 8-43: The Movie and MovieDetail interfaces currently declare
poster_path and backdrop_path as plain string which breaks type-safety because
TMDB can return null; update Movie.poster_path and MovieDetail.poster_path and
MovieDetail.backdrop_path to be string | null (keep MovieDetail.homepage as
string per API examples) so callers must handle null values; locate these fields
in the Movie and MovieDetail interfaces and change their types accordingly.
---
Nitpick comments:
In `@Week04/gureum/mission1/package.json`:
- Around line 12-19: The dependency "@tailwindcss/vite" is listed under
"dependencies" but should be a dev-only plugin; move the entry for
"@tailwindcss/vite" from the "dependencies" object to "devDependencies" in
package.json (keep the same version string "^4.2.2"), then reinstall (npm/yarn)
so the lockfile updates; ensure no other build-time plugins are left under
"dependencies".
In `@Week04/gureum/mission1/README.md`:
- Around line 1-73: Update the README.md to replace the generic Vite template
content with project-specific documentation: describe the app purpose (TMDB
integration for movie search and detail pages), list key features (search,
detail view, pagination, favorites), specify required environment variables
(e.g., VITE_TMDB_API_KEY), add exact run/build/test commands (dev, build,
preview, lint, test) and example usage, and include a brief project structure
(src/components, src/pages, src/api) and troubleshooting notes so contributors
can run and understand the codebase (refer to README.md, mention TMDB API usage
and related scripts in package.json).
In `@Week04/gureum/mission1/src/App.tsx`:
- Around line 15-25: Add a catch-all 404 route to the router so unknown paths
render a NotFound page: update the createBrowserRouter /
createRoutesFromElements setup that uses Route and Layout to include a Route
with path="*" (or an explicit NotFound route) as a child of the root Route, and
create/import a NotFoundPage component to render there; ensure Layout (and its
Outlet) remains unchanged so the 404 page is rendered within the existing
layout.
In `@Week04/gureum/mission1/src/components/MoviesPage.tsx`:
- Around line 56-57: In MoviesPage.tsx remove the literal <br /> used for
spacing and instead apply a margin utility class (e.g., mb-4 / mt-4 or the
project’s spacing token) to the appropriate JSX element(s) in the MoviesPage
component (wrap/div or the element immediately before/after the former <br />)
so spacing is controlled via classes like "mb-*" or "mt-*" rather than markup.
In `@Week04/gureum/mission1/src/hooks/useMovies.ts`:
- Around line 6-27: When the movie category `type` changes, reset pagination by
calling `setCurrentPage(1)` so users always start at page 1 for a new category;
add a small effect in `useMovies` (useEffect watching `type`) that invokes
`setCurrentPage(1)` (and ensure `type` is in the dependency array) so
`currentPage` is cleared when `type` changes while keeping the existing
`fetcher`, `useFetch` and memoized `movies`/`totalPages` logic intact.
In `@Week04/gureum/mission1/src/pages/HomePage.tsx`:
- Around line 1-7: HomePage duplicates PopularPage; update HomePage to reuse
PopularPage instead of reimplementing the same JSX: import PopularPage (the
existing component name) and either return <PopularPage /> from HomePage or
simply default-export PopularPage from the PopularPage module and change
HomePage to re-export it; ensure you remove duplicated props/title hardcoding in
HomePage and keep MoviesPage usage centralized in PopularPage to avoid future
divergence.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5434cb2-4bb8-4a20-8c41-73013aa58c90
⛔ Files ignored due to path filters (2)
Week04/gureum/mission1/package-lock.jsonis excluded by!**/package-lock.jsonWeek04/gureum/mission1/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (33)
Week04/gureum/mission1/.gitignoreWeek04/gureum/mission1/README.mdWeek04/gureum/mission1/eslint.config.jsWeek04/gureum/mission1/index.htmlWeek04/gureum/mission1/package.jsonWeek04/gureum/mission1/src/App.cssWeek04/gureum/mission1/src/App.tsxWeek04/gureum/mission1/src/api/tmdb.tsWeek04/gureum/mission1/src/components/Layout.tsxWeek04/gureum/mission1/src/components/MoviesPage.tsxWeek04/gureum/mission1/src/components/Navbar.tsxWeek04/gureum/mission1/src/components/common/ErrorState.tsxWeek04/gureum/mission1/src/components/common/LoadingSpinner.tsxWeek04/gureum/mission1/src/components/movie-detail/CastGrid.tsxWeek04/gureum/mission1/src/components/movie-detail/MovieHero.tsxWeek04/gureum/mission1/src/components/movie-detail/MovieMetaCards.tsxWeek04/gureum/mission1/src/components/movies/MovieGrid.tsxWeek04/gureum/mission1/src/components/movies/Pagination.tsxWeek04/gureum/mission1/src/hooks/useFetch.tsWeek04/gureum/mission1/src/hooks/useMovieDetail.tsWeek04/gureum/mission1/src/hooks/useMovies.tsWeek04/gureum/mission1/src/index.cssWeek04/gureum/mission1/src/main.tsxWeek04/gureum/mission1/src/pages/HomePage.tsxWeek04/gureum/mission1/src/pages/MovieDetailPage.tsxWeek04/gureum/mission1/src/pages/PopularPage.tsxWeek04/gureum/mission1/src/pages/TopRatedPage.tsxWeek04/gureum/mission1/src/pages/UpcomingPage.tsxWeek04/gureum/mission1/src/types/movie.tsWeek04/gureum/mission1/tsconfig.app.jsonWeek04/gureum/mission1/tsconfig.jsonWeek04/gureum/mission1/tsconfig.node.jsonWeek04/gureum/mission1/vite.config.ts
| @@ -0,0 +1,13 @@ | |||
| <!doctype html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
문서 lang 값을 실제 콘텐츠 언어와 맞춰주세요.
현재 UI가 한국어 중심이므로 lang="ko"가 접근성/보조기기 동작에 더 정확합니다.
수정 예시
-<html lang="en">
+<html lang="ko">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <html lang="en"> | |
| <html lang="ko"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/index.html` at line 2, Update the HTML root element's
language attribute from lang="en" to lang="ko" to match the Korean UI; locate
the <html lang="en"> tag in index.html and change it to <html lang="ko"> so
assistive technologies and browsers use the correct language locale.
Comment on lines
+9
to
+23
| const BASE_URL = import.meta.env.VITE_TMDB_BASE_URL; | ||
| const TMDB_TOKEN = import.meta.env.VITE_TMDB_API_KEY; | ||
|
|
||
| const tmdbClient = axios.create({ | ||
| baseURL: BASE_URL, | ||
| headers: { | ||
| Authorization: `Bearer ${TMDB_TOKEN}`, | ||
| }, | ||
| }); | ||
|
|
||
| const ensureToken = () => { | ||
| if (!TMDB_TOKEN) { | ||
| throw new Error('TMDB API 토큰이 설정되지 않았습니다.'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
BASE_URL 미설정 검증이 빠져 잘못된 요청 경로가 발생할 수 있습니다.
현재는 토큰만 확인하므로, VITE_TMDB_BASE_URL이 비어 있으면 상대경로로 요청이 나가 실패 원인 추적이 어려워집니다.
🔧 제안 수정안
-const ensureToken = () => {
+const ensureConfig = () => {
+ if (!BASE_URL) {
+ throw new Error('TMDB BASE URL이 설정되지 않았습니다.');
+ }
if (!TMDB_TOKEN) {
throw new Error('TMDB API 토큰이 설정되지 않았습니다.');
}
};
export const fetchMovies = async (type: MovieListType, page: number) => {
- ensureToken();
+ ensureConfig();
@@
export const fetchMovieDetail = async (movieId: string) => {
- ensureToken();
+ ensureConfig();
@@
export const fetchMovieCredits = async (movieId: string) => {
- ensureToken();
+ ensureConfig();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/api/tmdb.ts` around lines 9 - 23, The code only
validates TMDB_TOKEN but not BASE_URL, causing requests to use a relative path
if VITE_TMDB_BASE_URL is missing; update validation so BASE_URL is checked as
well (e.g., extend or replace ensureToken with ensureConfig that verifies both
BASE_URL and TMDB_TOKEN and throws a clear Error when BASE_URL is falsy) and
call that validation before creating tmdbClient (or delay axios.create until
after ensureConfig) so tmdbClient is always initialized with a valid baseURL.
Comment on lines
+17
to
+22
| <img | ||
| src={ | ||
| actor.profile_path | ||
| ? `https://image.tmdb.org/t/p/w300${actor.profile_path}` | ||
| : '/api/placeholder/150/225' | ||
| } |
There was a problem hiding this comment.
Placeholder 이미지 경로가 유효하지 않을 수 있습니다.
/api/placeholder/150/225 경로는 실제 이미지를 반환하지 않을 가능성이 높습니다. 프로필 이미지가 없는 경우를 위해 실제 placeholder 이미지나 기본 아바타 이미지를 사용하는 것이 좋습니다.
💡 권장 수정 사항
src={
actor.profile_path
? `https://image.tmdb.org/t/p/w300${actor.profile_path}`
- : '/api/placeholder/150/225'
+ : 'https://via.placeholder.com/150x225?text=No+Image'
}또는 로컬 기본 이미지 파일을 사용:
- : '/api/placeholder/150/225'
+ : '/default-profile.png'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <img | |
| src={ | |
| actor.profile_path | |
| ? `https://image.tmdb.org/t/p/w300${actor.profile_path}` | |
| : '/api/placeholder/150/225' | |
| } | |
| <img | |
| src={ | |
| actor.profile_path | |
| ? `https://image.tmdb.org/t/p/w300${actor.profile_path}` | |
| : 'https://via.placeholder.com/150x225?text=No+Image' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/components/movie-detail/CastGrid.tsx` around lines
17 - 22, The img src fallback uses a likely-invalid path
`/api/placeholder/150/225`; update CastGrid.tsx to use a real placeholder by
either importing a local default avatar (e.g., import defaultAvatar from '...')
and using that when actor.profile_path is falsy, or point the fallback to a
guaranteed external placeholder URL (e.g., a CDN or project assets). Adjust the
conditional around actor.profile_path in the <img> src expression so it uses the
new default (reference: actor.profile_path, CastGrid component) and ensure the
project bundler can resolve the chosen asset.
Comment on lines
+26
to
+28
| onError={(event) => { | ||
| event.currentTarget.src = '/api/placeholder/300/450'; | ||
| }} |
There was a problem hiding this comment.
이미지 fallback 처리에서 onError 재호출을 차단해주세요.
fallback 이미지도 실패할 경우를 대비해 핸들러를 해제한 뒤 src를 바꾸는 편이 안전합니다.
🔧 제안 수정안
onError={(event) => {
+ event.currentTarget.onerror = null;
event.currentTarget.src = '/api/placeholder/300/450';
}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/components/movie-detail/MovieHero.tsx` around
lines 26 - 28, In MovieHero's image onError handler, prevent a fallback loop by
removing the error handler before changing the src; locate the onError handler
attached to the img (the arrow function using event.currentTarget.src) and
update it to first set event.currentTarget.onerror = null (or remove the
handler) and then assign the fallback URL (e.g., '/api/placeholder/300/450'),
optionally guarding by checking that the current src is not already the fallback
to avoid redundant work.
Comment on lines
+47
to
+54
| {movie.homepage && ( | ||
| <div className="bg-white p-6 rounded-lg shadow-md"> | ||
| <h3 className="text-lg font-semibold mb-3 text-blue-600">링크</h3> | ||
| <a | ||
| href={movie.homepage} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="inline-flex items-center text-blue-600 hover:text-blue-800 transition-colors" |
There was a problem hiding this comment.
외부 링크 스킴 검증이 없어 잠재적 XSS 벡터가 남아 있습니다.
movie.homepage를 그대로 href에 넣기 전에 http/https 스킴만 허용하는 검증이 필요합니다.
🔧 제안 수정안
+ const isSafeHomepage =
+ typeof movie.homepage === 'string' && /^https?:\/\//i.test(movie.homepage);
- {movie.homepage && (
+ {isSafeHomepage && (
<div className="bg-white p-6 rounded-lg shadow-md">
<h3 className="text-lg font-semibold mb-3 text-blue-600">링크</h3>
<a
href={movie.homepage}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {movie.homepage && ( | |
| <div className="bg-white p-6 rounded-lg shadow-md"> | |
| <h3 className="text-lg font-semibold mb-3 text-blue-600">링크</h3> | |
| <a | |
| href={movie.homepage} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="inline-flex items-center text-blue-600 hover:text-blue-800 transition-colors" | |
| const isSafeHomepage = | |
| typeof movie.homepage === 'string' && /^https?:\/\//i.test(movie.homepage); | |
| {isSafeHomepage && ( | |
| <div className="bg-white p-6 rounded-lg shadow-md"> | |
| <h3 className="text-lg font-semibold mb-3 text-blue-600">링크</h3> | |
| <a | |
| href={movie.homepage} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="inline-flex items-center text-blue-600 hover:text-blue-800 transition-colors" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/components/movie-detail/MovieMetaCards.tsx` around
lines 47 - 54, MovieMetaCards currently injects movie.homepage directly into the
anchor href (in the MovieMetaCards component, variable movie.homepage), leaving
a potential XSS vector; add a validation step (e.g., an isSafeUrl helper that
uses the URL constructor and checks url.protocol is "http:" or "https:") and
only set href when the helper returns true; if validation fails, render either
no anchor or a non-clickable fallback (plain text or disabled link) so untrusted
schemes are never placed into the href or opened with target="_blank".
Comment on lines
+21
to
+23
| onError={(event) => { | ||
| event.currentTarget.src = '/api/placeholder/300/450'; | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# public 폴더에 placeholder 이미지가 있는지 확인
fd -t f "placeholder" Week04/gureum/mission1/
# vite.config.ts에서 proxy 설정 확인
cat Week04/gureum/mission1/vite.config.ts 2>/dev/null || echo "vite.config.ts not found in review context"Repository: HSU-Makeus-Challenge-10th/Web
Length of output: 305
🏁 Script executed:
# Check project structure and look for public/static folders
find Week04/gureum/mission1 -maxdepth 3 -type d | head -20
# Look for public or static folders specifically
fd -t d "^public$|^static$" Week04/gureum/mission1/
# Check if there's a package.json to understand the dev setup
cat Week04/gureum/mission1/package.json 2>/dev/null | head -40
# Search for any API-related files or server setup
fd -t f -e "ts" -e "js" Week04/gureum/mission1/src | grep -i "api\|server\|proxy" | head -10Repository: HSU-Makeus-Challenge-10th/Web
Length of output: 1751
🏁 Script executed:
# Check what's in the public folder
ls -la Week04/gureum/mission1/public/
# Check the src/api folder
fd . Week04/gureum/mission1/src/api -type f
# Check the actual MovieGrid.tsx file to understand the context
cat Week04/gureum/mission1/src/components/movies/MovieGrid.tsx
# Look for any mock server or dev server setup
fd . Week04/gureum/mission1/src -type f -name "*mock*" -o -name "*server*" -o -name "*api*"Repository: HSU-Makeus-Challenge-10th/Web
Length of output: 2283
🏁 Script executed:
# Check src/api folder contents
find Week04/gureum/mission1/src/api -type f
# Look for index.ts, fetch, or any API setup files
cat Week04/gureum/mission1/src/api/* 2>/dev/null
# Check if there's any server setup in the project root
ls -la Week04/gureum/mission1/ | grep -E "server|api|proxy"
# Search for any references to /api/placeholder in the codebase
rg "api/placeholder" Week04/gureum/mission1/Repository: HSU-Makeus-Challenge-10th/Web
Length of output: 1645
🏁 Script executed:
# Check for .env files
find Week04/gureum/mission1 -maxdepth 2 -name ".env*" -o -name "*.example"
# Search for any mock server setup or mock data
rg "mock\|placeholder" Week04/gureum/mission1/src --type-list | head -5
# Check if there's a backend directory or server setup
ls -la Week04/gureum/mission1/ | head -20
# Look at CastGrid.tsx and MovieHero.tsx to see if they have the same issue
cat Week04/gureum/mission1/src/components/movie-detail/CastGrid.tsxRepository: HSU-Makeus-Challenge-10th/Web
Length of output: 2812
🏁 Script executed:
# Check MovieHero.tsx to see the full context
cat Week04/gureum/mission1/src/components/movie-detail/MovieHero.tsx
# Check for any environment file examples
cat Week04/gureum/mission1/.gitignore
# Look for README to understand the project setup
cat Week04/gureum/mission1/README.mdRepository: HSU-Makeus-Challenge-10th/Web
Length of output: 5917
플레이스홀더 이미지 경로가 존재하지 않음 - 수정 필요
/api/placeholder/300/450은 현재 프로젝트에서 구성되지 않은 경로입니다. Vite 설정에 API 프록시가 없고, 백엔드 서버나 모의(mock) 서버도 없으므로 이미지 로드 실패 시 이 fallback도 404 에러가 발생합니다.
https://placehold.co/300x450과 같은 외부 placeholder 서비스를 사용하거나, public 폴더에 정적 placeholder 이미지를 추가하여 사용하는 것이 권장됩니다. CastGrid.tsx와 MovieHero.tsx에서도 동일한 패턴이 사용되고 있으므로 함께 수정하길 바랍니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/components/movies/MovieGrid.tsx` around lines 21 -
23, The onError handler in MovieGrid.tsx (and the similar handlers in
CastGrid.tsx and MovieHero.tsx) points to a non-existent internal path
'/api/placeholder/300/450'; replace that fallback with a valid placeholder URL
or a public static asset: either change event.currentTarget.src to an external
service like 'https://placehold.co/300x450' or add a static image (e.g.,
placeholder-300x450.png) to the public folder and set the src to
'/placeholder-300x450.png'; update the onError handlers in MovieGrid, CastGrid,
and MovieHero to use the chosen valid path.
Comment on lines
+16
to
+42
| <button | ||
| onClick={onPrev} | ||
| disabled={currentPage === 1} | ||
| className={`w-12 h-12 rounded-full font-bold text-xl transition-colors ${ | ||
| currentPage === 1 | ||
| ? 'bg-gray-400 text-gray-600 cursor-not-allowed' | ||
| : 'bg-pink-500 text-white hover:bg-pink-600' | ||
| }`} | ||
| > | ||
| < | ||
| </button> | ||
|
|
||
| <span className="text-gray-800 font-semibold mx-4"> | ||
| {currentPage} / {totalPages} | ||
| </span> | ||
|
|
||
| <button | ||
| onClick={onNext} | ||
| disabled={currentPage >= totalPages} | ||
| className={`w-12 h-12 rounded-full font-bold text-xl transition-colors ${ | ||
| currentPage >= totalPages | ||
| ? 'bg-gray-400 text-gray-600 cursor-not-allowed' | ||
| : 'bg-pink-500 text-white hover:bg-pink-600' | ||
| }`} | ||
| > | ||
| > | ||
| </button> |
There was a problem hiding this comment.
페이지 이동 버튼에 접근성 레이블을 추가해주세요.
<, >만으로는 보조기기에서 의미 전달이 약하므로 aria-label을 명시하는 게 좋습니다.
🔧 제안 수정안
<button
+ type="button"
+ aria-label="이전 페이지"
onClick={onPrev}
disabled={currentPage === 1}
@@
<button
+ type="button"
+ aria-label="다음 페이지"
onClick={onNext}
disabled={currentPage >= totalPages}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| onClick={onPrev} | |
| disabled={currentPage === 1} | |
| className={`w-12 h-12 rounded-full font-bold text-xl transition-colors ${ | |
| currentPage === 1 | |
| ? 'bg-gray-400 text-gray-600 cursor-not-allowed' | |
| : 'bg-pink-500 text-white hover:bg-pink-600' | |
| }`} | |
| > | |
| < | |
| </button> | |
| <span className="text-gray-800 font-semibold mx-4"> | |
| {currentPage} / {totalPages} | |
| </span> | |
| <button | |
| onClick={onNext} | |
| disabled={currentPage >= totalPages} | |
| className={`w-12 h-12 rounded-full font-bold text-xl transition-colors ${ | |
| currentPage >= totalPages | |
| ? 'bg-gray-400 text-gray-600 cursor-not-allowed' | |
| : 'bg-pink-500 text-white hover:bg-pink-600' | |
| }`} | |
| > | |
| > | |
| </button> | |
| <button | |
| type="button" | |
| aria-label="이전 페이지" | |
| onClick={onPrev} | |
| disabled={currentPage === 1} | |
| className={`w-12 h-12 rounded-full font-bold text-xl transition-colors ${ | |
| currentPage === 1 | |
| ? 'bg-gray-400 text-gray-600 cursor-not-allowed' | |
| : 'bg-pink-500 text-white hover:bg-pink-600' | |
| }`} | |
| > | |
| < | |
| </button> | |
| <span className="text-gray-800 font-semibold mx-4"> | |
| {currentPage} / {totalPages} | |
| </span> | |
| <button | |
| type="button" | |
| aria-label="다음 페이지" | |
| onClick={onNext} | |
| disabled={currentPage >= totalPages} | |
| className={`w-12 h-12 rounded-full font-bold text-xl transition-colors ${ | |
| currentPage >= totalPages | |
| ? 'bg-gray-400 text-gray-600 cursor-not-allowed' | |
| : 'bg-pink-500 text-white hover:bg-pink-600' | |
| }`} | |
| > | |
| > | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/components/movies/Pagination.tsx` around lines 16
- 42, The previous/next buttons currently render only "<" and ">" which is
unclear to assistive tech; update the two button elements that call onPrev and
onNext (and use currentPage/totalPages for disabled logic) to include
descriptive ARIA labels (e.g., aria-label="Previous page" and aria-label="Next
page"); also ensure the disabled state remains set (and optionally add
aria-disabled when currentPage === 1 or currentPage >= totalPages) so screen
readers get both the action and disabled information.
Comment on lines
+20
to
+22
| <NavLink to="/" className={navClassName}> | ||
| 홈 | ||
| </NavLink> |
There was a problem hiding this comment.
홈 링크에 end prop 추가가 필요합니다.
NavLink to="/"는 기본적으로 경로 접두사 매칭을 사용합니다. 따라서 /popular, /upcoming 등의 페이지에서도 "홈" 링크가 active 상태로 표시될 수 있습니다. end prop을 추가하면 정확히 / 경로일 때만 active 상태가 됩니다.
🐛 권장 수정 사항
- <NavLink to="/" className={navClassName}>
+ <NavLink to="/" end className={navClassName}>
홈
</NavLink>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <NavLink to="/" className={navClassName}> | |
| 홈 | |
| </NavLink> | |
| <NavLink to="/" end className={navClassName}> | |
| 홈 | |
| </NavLink> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/components/Navbar.tsx` around lines 20 - 22, The
NavLink for the home route in Navbar.tsx (the NavLink with to="/" and
className={navClassName}) should include the end prop so it only becomes active
on the exact "/" path; update the NavLink component to add end to its props to
prevent prefix matching (so "/popular" or "/upcoming" won't mark the home link
active).
Comment on lines
+31
to
+46
| try { | ||
| const nextData = await fetcher(); | ||
|
|
||
| if (isMountedRef.current) { | ||
| setData(nextData); | ||
| } | ||
| } catch (requestError) { | ||
| console.error('데이터 요청 실패:', requestError); | ||
|
|
||
| if (isMountedRef.current) { | ||
| setError(options?.errorMessage ?? '데이터를 불러오지 못했습니다.'); | ||
| } | ||
| } finally { | ||
| if (isMountedRef.current) { | ||
| setIsLoading(false); | ||
| } |
There was a problem hiding this comment.
이전 요청의 늦은 응답이 최신 상태를 덮어쓸 수 있습니다.
현재는 mount 여부만 검사하므로, 연속 요청 시 먼저 보낸 요청이 나중에 도착하면 stale 데이터가 반영될 수 있습니다. 요청 ID(또는 AbortController)로 “마지막 요청만 반영”되게 막아주세요.
수정 예시 (requestId 가드)
export const useFetch = <T>(
fetcher: () => Promise<T>,
dependencies: readonly unknown[],
options?: UseFetchOptions
) => {
const [data, setData] = useState<T | null>(null);
const [isLoading, setIsLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
+ const requestIdRef = useRef(0);
const isMountedRef = useRef(true);
const runFetch = useCallback(async () => {
+ const requestId = ++requestIdRef.current;
setIsLoading(true);
setError(null);
try {
const nextData = await fetcher();
- if (isMountedRef.current) {
+ if (isMountedRef.current && requestId === requestIdRef.current) {
setData(nextData);
}
} catch (requestError) {
console.error('데이터 요청 실패:', requestError);
- if (isMountedRef.current) {
+ if (isMountedRef.current && requestId === requestIdRef.current) {
setError(options?.errorMessage ?? '데이터를 불러오지 못했습니다.');
}
} finally {
- if (isMountedRef.current) {
+ if (isMountedRef.current && requestId === requestIdRef.current) {
setIsLoading(false);
}
}
}, [fetcher, options?.errorMessage]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/hooks/useFetch.ts` around lines 31 - 46, The hook
can suffer from stale responses overwriting newer state because only
isMountedRef is checked; modify the request flow in useFetch so each fetch call
has a unique requestId (or use an AbortController) and only apply
setData/setError/setIsLoading if the current requestId matches the most recent
one (or the signal is not aborted). Concretely, generate/increment a local
requestId (or create and store an AbortController) before calling fetcher(),
capture it in the try/catch/finally, and guard the setData, setError, and
setIsLoading calls with that requestId check (or controller.signal.aborted
check) to ensure only the latest response updates state while retaining existing
isMountedRef checks.
Comment on lines
+8
to
+43
| export interface Movie { | ||
| id: number; | ||
| title: string; | ||
| overview: string; | ||
| poster_path: string; | ||
| release_date: string; | ||
| vote_average: number; | ||
| } | ||
|
|
||
| export interface MovieListResponse { | ||
| page: number; | ||
| results: Movie[]; | ||
| total_pages: number; | ||
| total_results: number; | ||
| } | ||
|
|
||
| export interface MovieDetail { | ||
| id: number; | ||
| title: string; | ||
| overview: string; | ||
| poster_path: string; | ||
| backdrop_path: string; | ||
| release_date: string; | ||
| vote_average: number; | ||
| vote_count: number; | ||
| runtime: number; | ||
| genres: Genre[]; | ||
| budget: number; | ||
| revenue: number; | ||
| tagline: string; | ||
| homepage: string; | ||
| status: string; | ||
| original_language: string; | ||
| original_title: string; | ||
| popularity: number; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TMDB API 공식 문서(또는 예시 응답) 기준으로 /movie/{movie_id}, /movie/{category} 응답에서 poster_path, backdrop_path, homepage 필드가 null 가능(nullable)인지 확인해 주세요.
💡 Result:
TMDB v3 공식 OpenAPI(tmdb-api.json)에 포함된 예시 응답(example) 기준으로 확인하면 아래와 같습니다.
| 엔드포인트 | 필드 | null 가능? | 근거(공식 예시 응답) |
|---|---|---|---|
/movie/{category} (예: /movie/popular) |
poster_path |
가능 | 영화 객체에 "poster_path": null 예시가 존재합니다. [1] |
/movie/{category} |
backdrop_path |
가능 | 영화 객체에 "backdrop_path": null 예시가 존재합니다. [1] |
/movie/{movie_id} (Details) |
poster_path |
가능(실무상) | 동일한 “movie object” 예시에서 poster_path가 null로 나오는 케이스가 확인됩니다. [1] |
/movie/{movie_id} (Details) |
backdrop_path |
가능(실무상) | 동일한 “movie object” 예시에서 backdrop_path가 null로 나오는 케이스가 확인됩니다. [1] |
/movie/{movie_id} (Details) |
homepage |
null 보장 근거는 없음 (대신 빈 문자열 "" 케이스 확인) |
공식 예시에서 "homepage": "" 형태(빈 문자열)가 확인됩니다. [2] |
추가로, OpenAPI 스키마(schema) 타입 표기에서는 poster_path, backdrop_path가 string으로만 표시되어(= null 허용을 명시하지 않아) 스키마만 보면 non-null처럼 보일 수 있는데, 공식 예시 응답에는 null 케이스가 포함되어 있어 클라이언트에서는 nullable로 처리하는 것이 안전합니다. [1], [3]
TMDB null 가능 필드를 string으로 고정하면 타입 안정성이 깨집니다.
공식 API 예시 응답에 따르면 poster_path와 backdrop_path는 null이 가능하므로 string | null로 모델링해야 합니다. 반면 homepage는 공식 예시에서 null보다는 빈 문자열("")로 처리되므로 현재 string 타입으로 유지하는 것이 적절합니다.
🔧 제안 수정안
export interface Movie {
@@
- poster_path: string;
+ poster_path: string | null;
@@
export interface MovieDetail {
@@
- poster_path: string;
- backdrop_path: string;
+ poster_path: string | null;
+ backdrop_path: string | null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export interface Movie { | |
| id: number; | |
| title: string; | |
| overview: string; | |
| poster_path: string; | |
| release_date: string; | |
| vote_average: number; | |
| } | |
| export interface MovieListResponse { | |
| page: number; | |
| results: Movie[]; | |
| total_pages: number; | |
| total_results: number; | |
| } | |
| export interface MovieDetail { | |
| id: number; | |
| title: string; | |
| overview: string; | |
| poster_path: string; | |
| backdrop_path: string; | |
| release_date: string; | |
| vote_average: number; | |
| vote_count: number; | |
| runtime: number; | |
| genres: Genre[]; | |
| budget: number; | |
| revenue: number; | |
| tagline: string; | |
| homepage: string; | |
| status: string; | |
| original_language: string; | |
| original_title: string; | |
| popularity: number; | |
| } | |
| export interface Movie { | |
| id: number; | |
| title: string; | |
| overview: string; | |
| poster_path: string | null; | |
| release_date: string; | |
| vote_average: number; | |
| } | |
| export interface MovieListResponse { | |
| page: number; | |
| results: Movie[]; | |
| total_pages: number; | |
| total_results: number; | |
| } | |
| export interface MovieDetail { | |
| id: number; | |
| title: string; | |
| overview: string; | |
| poster_path: string | null; | |
| backdrop_path: string | null; | |
| release_date: string; | |
| vote_average: number; | |
| vote_count: number; | |
| runtime: number; | |
| genres: Genre[]; | |
| budget: number; | |
| revenue: number; | |
| tagline: string; | |
| homepage: string; | |
| status: string; | |
| original_language: string; | |
| original_title: string; | |
| popularity: number; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission1/src/types/movie.ts` around lines 8 - 43, The Movie and
MovieDetail interfaces currently declare poster_path and backdrop_path as plain
string which breaks type-safety because TMDB can return null; update
Movie.poster_path and MovieDetail.poster_path and MovieDetail.backdrop_path to
be string | null (keep MovieDetail.homepage as string per API examples) so
callers must handle null values; locate these fields in the Movie and
MovieDetail interfaces and change their types accordingly.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
Week04/gureum/mission2/src/components/auth/AuthTopBar.tsx (1)
1-24: 핸들러가 없을 때 버튼 비활성화 처리를 권장합니다.
onLoginClick/onSignupClick가 없는 상태에서도 버튼이 활성으로 보여 클릭 무반응 UX가 생길 수 있습니다.제안 변경안
<button type="button" onClick={onLoginClick} + disabled={!onLoginClick} className="text-white px-3 py-1 border border-gray-600 rounded text-sm hover:bg-gray-800" > 로그인 </button> <button type="button" onClick={onSignupClick} + disabled={!onSignupClick} className="bg-pink-500 text-white px-3 py-1 rounded text-sm hover:bg-pink-600" > 회원가입 </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission2/src/components/auth/AuthTopBar.tsx` around lines 1 - 24, AuthTopBar currently renders actionable buttons even when handlers are undefined, causing confusing non-responsive UX; update the component (AuthTopBar) to disable each button when its prop is missing by setting disabled={!onLoginClick} and disabled={!onSignupClick}, only attach onClick handlers if the prop exists (e.g., onClick={onLoginClick ?? undefined} or conditional handler), and add/adjust className to reflect a disabled state (muted text/bg and remove hover styles) so the buttons visually and functionally indicate they are disabled when handlers are not provided.Week04/gureum/mission2/src/components/auth/AuthField.tsx (1)
1-33: 입력 필드에 라벨/오류 연계 속성을 추가하는 것을 권장합니다.현재는 placeholder 중심이라 보조기기에서 필드 의미 전달이 약합니다.
id + label,aria-invalid,aria-describedby를 연결하면 접근성이 좋아집니다.제안 변경안
interface AuthFieldProps { + id: string; + label: string; + name: string; type: 'email' | 'password'; placeholder: string; value: string; onChange: (event: React.ChangeEvent<HTMLInputElement>) => void; onBlur: () => void; error?: string; } @@ const AuthField = ({ + id, + label, + name, type, placeholder, @@ return ( <div> + <label htmlFor={id} className="sr-only">{label}</label> <input + id={id} + name={name} type={type} placeholder={placeholder} value={value} onChange={onChange} onBlur={onBlur} + autoComplete={type === 'email' ? 'email' : 'current-password'} + aria-invalid={!!error} + aria-describedby={error ? `${id}-error` : undefined} @@ - {error && <p className="mt-1 text-sm text-red-500">{error}</p>} + {error && <p id={`${id}-error`} className="mt-1 text-sm text-red-500">{error}</p>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission2/src/components/auth/AuthField.tsx` around lines 1 - 33, AuthField lacks accessible labeling and error associations; update AuthFieldProps to accept an id (string) and an optional label (string), render a <label htmlFor={id}> using that label before the input inside the AuthField component, give the input the id, set aria-invalid={!!error} and aria-describedby={error ? `${id}-error` : undefined}, and render the error paragraph with that id (e.g., id={`${id}-error`} and role="alert") so screen readers can relate the label and error to the input; reference the AuthField component and AuthFieldProps when making these changes.Week04/gureum/mission2/src/components/Navbar.tsx (1)
13-24: 활성 라우트 스타일을 추가하면 내비게이션 인지가 더 좋아집니다.
NavLink의className콜백을 사용해 현재 페이지를 시각적으로 구분하는 것을 권장합니다.리팩터 예시
<NavLink to="/login" - className="px-3 py-1 border border-gray-300 rounded text-sm text-gray-600 hover:bg-gray-100 transition-colors" + className={({ isActive }) => + `px-3 py-1 border rounded text-sm transition-colors ${ + isActive + ? 'border-pink-500 text-pink-600 bg-pink-50' + : 'border-gray-300 text-gray-600 hover:bg-gray-100' + }` + } > 로그인 </NavLink>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission2/src/components/Navbar.tsx` around lines 13 - 24, Navbar's NavLink instances don't apply active-route styling; update the NavLink `className` prop in this component (the NavLink elements that render 로그인 and 회원가입) to use the callback form (receive { isActive }) and return a string that conditionally includes an "active" style (e.g., different bg, text or border) when isActive is true, otherwise the default classes; ensure both NavLink usages in Navbar.tsx use the same pattern so the current route is visually highlighted.Week04/gureum/mission2/src/components/auth/SocialLoginButton.tsx (1)
1-13: 클릭 핸들러를 받을 수 없어 버튼이 항상 무동작입니다.소셜 로그인 버튼이라면
onClick(또는href)를 props로 받아 동작을 연결할 수 있게 해두는 편이 안전합니다.리팩터 예시
interface SocialLoginButtonProps { label: string; + onClick?: () => void; } -const SocialLoginButton = ({ label }: SocialLoginButtonProps) => { +const SocialLoginButton = ({ label, onClick }: SocialLoginButtonProps) => { return ( <button type="button" + onClick={onClick} className="w-full flex items-center justify-center px-4 py-3 border border-gray-600 rounded-md bg-transparent text-white hover:bg-gray-800 transition-colors" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission2/src/components/auth/SocialLoginButton.tsx` around lines 1 - 13, The SocialLoginButton currently ignores click actions; update SocialLoginButtonProps to accept an onClick callback (e.g., onClick?: (e: React.MouseEvent<HTMLButtonElement>) => void) and pass that prop through to the rendered <button> in SocialLoginButton so consumers can attach behavior; alternatively optionally accept an href string and render an <a> when provided, but at minimum add the onClick prop to SocialLoginButtonProps and wire it into the button element in SocialLoginButton.Week04/gureum/mission2/src/hooks/useForm.ts (1)
24-24:validate함수의 참조 안정성 확인 필요
useMemo의존성 배열에validate가 포함되어 있습니다. 현재 사용처(LoginPage, SignupPage)에서는validate함수가 컴포넌트 외부에 정의되어 있어 안정적이지만, 컴포넌트 내부에서 인라인으로 정의할 경우 매 렌더링마다errors가 재계산됩니다.향후 사용 시
validate함수를useCallback으로 감싸거나 컴포넌트 외부에 정의하도록 JSDoc 주석을 추가하는 것을 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission2/src/hooks/useForm.ts` at line 24, The useMemo in useForm currently depends on a validate function (const errors = useMemo(() => validate(values), [validate, values])) which will cause errors to recompute on every render if callers pass an inline validate; ensure validate is referentially stable by documenting and/or changing callers to wrap validate in useCallback or move validate out of the component (e.g., define it in LoginPage/SignupPage module scope or useCallback in those components); update JSDoc on useForm to warn callers to provide a stable validate function or wrap examples in useCallback so errors only recalc when values actually change.Week04/gureum/mission2/src/pages/SignupPage.tsx (2)
67-71:name필드 자동 생성 로직 검토이메일 로컬 파트에서
name을 추출하는 방식은 실용적이지만, 특수문자가 포함된 이메일(예:john.doe+test@example.com)의 경우 예상치 못한 이름이 생성될 수 있습니다.현재 구현으로도 동작하지만, 향후 사용자가 직접 이름을 입력하는 필드를 추가하는 것을 고려해 보세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission2/src/pages/SignupPage.tsx` around lines 67 - 71, The current automatic name generation uses values.email.split('@')[0] which may include plus-addressing or special characters; update the logic in the signup flow where postSignup is called (the postSignup call and values.email usage) to: extract the local part, strip any plus-addressing (take substring before '+'), remove non-alphanumeric characters (e.g., regex replace), and fallback to 'user' if the cleaned result is empty; also consider leaving a TODO or prepare for adding a user-entered name field later.
16-39: 이메일 유효성 검증 로직 중복
emailRegex와 이메일 유효성 검증 로직이loginValidation.ts에도 동일하게 존재합니다. DRY 원칙에 따라 공통 유틸리티로 추출하는 것을 권장합니다.♻️ 공통 유틸리티 추출 제안
constants/validation.ts에 공통 로직 추출:// constants/validation.ts export const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; export const validateEmail = (email: string): string | undefined => { if (!email.trim()) { return '이메일을 입력해주세요.'; } if (!emailRegex.test(email)) { return '유효하지 않은 이메일 형식입니다.'; } return undefined; }; export const validatePassword = (password: string): string | undefined => { if (!password.trim()) { return '비밀번호를 입력해주세요.'; } if (password.length < 6) { return '비밀번호는 최소 6자 이상이어야 합니다.'; } return undefined; };그 후
loginValidation.ts와SignupPage.tsx에서 공통 함수를 import하여 사용:+import { validateEmail, validatePassword } from '../constants/validation'; const validateSignupForm = (values: SignupValues) => { const errors: Partial<Record<keyof SignupValues, string>> = {}; - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; - if (!values.email.trim()) { - errors.email = '이메일을 입력해주세요.'; - } else if (!emailRegex.test(values.email)) { - errors.email = '유효하지 않은 이메일 형식입니다.'; - } + const emailError = validateEmail(values.email); + if (emailError) errors.email = emailError; - if (!values.password.trim()) { - errors.password = '비밀번호를 입력해주세요.'; - } else if (values.password.length < 6) { - errors.password = '비밀번호는 최소 6자 이상이어야 합니다.'; - } + const passwordError = validatePassword(values.password); + if (passwordError) errors.password = passwordError; // confirmPassword validation remains here (signup-specific) ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission2/src/pages/SignupPage.tsx` around lines 16 - 39, Extract the email validation logic (emailRegex and checks) into a shared utility (e.g., constants/validation.ts) exposing a validateEmail(email: string): string | undefined and keep or add validatePassword if desired, then update validateSignupForm in SignupPage.tsx to call validateEmail(values.email) and use its return to set errors.email (replace inline emailRegex and checks), and update the similar logic in loginValidation.ts to import and reuse validateEmail so the regex and messaging are centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Week04/gureum/mission2/src/api/axios.ts`:
- Around line 9-13: The Authorization header may include extra quotation marks
because accessToken from localStorage can be a JSON string; in axios request
config where you read accessToken (variable accessToken) and set
config.headers.Authorization = `Bearer ${accessToken}`, parse or normalize the
stored value first (e.g., attempt JSON.parse(accessToken) inside a try/catch and
fall back to the raw string, or trim surrounding quotes) before composing the
header so it becomes Bearer tokenWithoutQuotes; update the logic in the module
that reads LOCAL_STORAGE_KEYS.accessToken to ensure consistent token formatting.
In `@Week04/gureum/mission2/src/constants/loginValidation.ts`:
- Around line 11-14: The email validation currently trims only for the empty
check but uses the original values.email for the regex test, causing inputs like
"test@example.com " to fail; fix by computing a trimmed variable (e.g., const
email = values.email.trim()) and use that trimmed email for both the empty check
and emailRegex.test, updating assignments to errors.email accordingly so both
checks operate on the same trimmed value.
In `@Week04/gureum/mission2/src/hooks/useLocalStorage.ts`:
- Around line 2-9: The current setItem/getItem in useLocalStorage always
JSON.stringify values and JSON.parse without error handling which causes tokens
to be stored as quoted strings (e.g., "\"token\"") and JSON.parse to throw on
malformed values. Update setItem in useLocalStorage.ts to store raw strings
as-is when value is a string (use typeof value === 'string'), otherwise
JSON.stringify non-strings; update getItem<T> to safely parse: if stored value
starts with "{" or "[" try JSON.parse inside a try/catch and return parsed
result, otherwise return the raw string; ensure getItem returns null when no
entry and does not throw on parse errors. Reference functions: setItem and
getItem in useLocalStorage.ts.
In `@Week04/gureum/mission2/src/pages/LoginPage.tsx`:
- Around line 32-35: Remove the unused dead function handleSubmit (which
references isFormValid) from the component since the form's onSubmit already
uses handleLogin; delete the handleSubmit declaration and any imports or
references that only support it to keep the file clean and ensure no unused
symbols remain.
---
Nitpick comments:
In `@Week04/gureum/mission2/src/components/auth/AuthField.tsx`:
- Around line 1-33: AuthField lacks accessible labeling and error associations;
update AuthFieldProps to accept an id (string) and an optional label (string),
render a <label htmlFor={id}> using that label before the input inside the
AuthField component, give the input the id, set aria-invalid={!!error} and
aria-describedby={error ? `${id}-error` : undefined}, and render the error
paragraph with that id (e.g., id={`${id}-error`} and role="alert") so screen
readers can relate the label and error to the input; reference the AuthField
component and AuthFieldProps when making these changes.
In `@Week04/gureum/mission2/src/components/auth/AuthTopBar.tsx`:
- Around line 1-24: AuthTopBar currently renders actionable buttons even when
handlers are undefined, causing confusing non-responsive UX; update the
component (AuthTopBar) to disable each button when its prop is missing by
setting disabled={!onLoginClick} and disabled={!onSignupClick}, only attach
onClick handlers if the prop exists (e.g., onClick={onLoginClick ?? undefined}
or conditional handler), and add/adjust className to reflect a disabled state
(muted text/bg and remove hover styles) so the buttons visually and functionally
indicate they are disabled when handlers are not provided.
In `@Week04/gureum/mission2/src/components/auth/SocialLoginButton.tsx`:
- Around line 1-13: The SocialLoginButton currently ignores click actions;
update SocialLoginButtonProps to accept an onClick callback (e.g., onClick?: (e:
React.MouseEvent<HTMLButtonElement>) => void) and pass that prop through to the
rendered <button> in SocialLoginButton so consumers can attach behavior;
alternatively optionally accept an href string and render an <a> when provided,
but at minimum add the onClick prop to SocialLoginButtonProps and wire it into
the button element in SocialLoginButton.
In `@Week04/gureum/mission2/src/components/Navbar.tsx`:
- Around line 13-24: Navbar's NavLink instances don't apply active-route
styling; update the NavLink `className` prop in this component (the NavLink
elements that render 로그인 and 회원가입) to use the callback form (receive { isActive
}) and return a string that conditionally includes an "active" style (e.g.,
different bg, text or border) when isActive is true, otherwise the default
classes; ensure both NavLink usages in Navbar.tsx use the same pattern so the
current route is visually highlighted.
In `@Week04/gureum/mission2/src/hooks/useForm.ts`:
- Line 24: The useMemo in useForm currently depends on a validate function
(const errors = useMemo(() => validate(values), [validate, values])) which will
cause errors to recompute on every render if callers pass an inline validate;
ensure validate is referentially stable by documenting and/or changing callers
to wrap validate in useCallback or move validate out of the component (e.g.,
define it in LoginPage/SignupPage module scope or useCallback in those
components); update JSDoc on useForm to warn callers to provide a stable
validate function or wrap examples in useCallback so errors only recalc when
values actually change.
In `@Week04/gureum/mission2/src/pages/SignupPage.tsx`:
- Around line 67-71: The current automatic name generation uses
values.email.split('@')[0] which may include plus-addressing or special
characters; update the logic in the signup flow where postSignup is called (the
postSignup call and values.email usage) to: extract the local part, strip any
plus-addressing (take substring before '+'), remove non-alphanumeric characters
(e.g., regex replace), and fallback to 'user' if the cleaned result is empty;
also consider leaving a TODO or prepare for adding a user-entered name field
later.
- Around line 16-39: Extract the email validation logic (emailRegex and checks)
into a shared utility (e.g., constants/validation.ts) exposing a
validateEmail(email: string): string | undefined and keep or add
validatePassword if desired, then update validateSignupForm in SignupPage.tsx to
call validateEmail(values.email) and use its return to set errors.email (replace
inline emailRegex and checks), and update the similar logic in
loginValidation.ts to import and reuse validateEmail so the regex and messaging
are centralized.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd8ba14d-97ff-4517-b69c-26f9319aeda3
⛔ Files ignored due to path filters (1)
Week04/gureum/mission2/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
Week04/gureum/mission2/.gitignoreWeek04/gureum/mission2/README.mdWeek04/gureum/mission2/eslint.config.jsWeek04/gureum/mission2/index.htmlWeek04/gureum/mission2/package.jsonWeek04/gureum/mission2/src/App.cssWeek04/gureum/mission2/src/App.tsxWeek04/gureum/mission2/src/api/auth.tsWeek04/gureum/mission2/src/api/axios.tsWeek04/gureum/mission2/src/components/Layout.tsxWeek04/gureum/mission2/src/components/Navbar.tsxWeek04/gureum/mission2/src/components/auth/AuthDivider.tsxWeek04/gureum/mission2/src/components/auth/AuthField.tsxWeek04/gureum/mission2/src/components/auth/AuthPageTitle.tsxWeek04/gureum/mission2/src/components/auth/AuthTopBar.tsxWeek04/gureum/mission2/src/components/auth/SocialLoginButton.tsxWeek04/gureum/mission2/src/constants/key.tsWeek04/gureum/mission2/src/constants/loginValidation.tsWeek04/gureum/mission2/src/hooks/useForm.tsWeek04/gureum/mission2/src/hooks/useLocalStorage.tsWeek04/gureum/mission2/src/index.cssWeek04/gureum/mission2/src/main.tsxWeek04/gureum/mission2/src/pages/HomePage.tsxWeek04/gureum/mission2/src/pages/LoginPage.tsxWeek04/gureum/mission2/src/pages/SignupPage.tsxWeek04/gureum/mission2/src/types/auth.tsWeek04/gureum/mission2/src/types/common.tsWeek04/gureum/mission2/src/vite-env.d.tsWeek04/gureum/mission2/tsconfig.app.jsonWeek04/gureum/mission2/tsconfig.jsonWeek04/gureum/mission2/tsconfig.node.jsonWeek04/gureum/mission2/vite.config.ts
✅ Files skipped from review due to trivial changes (12)
- Week04/gureum/mission2/src/index.css
- Week04/gureum/mission2/src/components/auth/AuthDivider.tsx
- Week04/gureum/mission2/.gitignore
- Week04/gureum/mission2/src/types/common.ts
- Week04/gureum/mission2/src/main.tsx
- Week04/gureum/mission2/src/vite-env.d.ts
- Week04/gureum/mission2/eslint.config.js
- Week04/gureum/mission2/tsconfig.json
- Week04/gureum/mission2/tsconfig.app.json
- Week04/gureum/mission2/index.html
- Week04/gureum/mission2/src/types/auth.ts
- Week04/gureum/mission2/README.md
Comment on lines
+9
to
+13
| const accessToken = localStorage.getItem(LOCAL_STORAGE_KEYS.accessToken); | ||
|
|
||
| if (accessToken) { | ||
| config.headers.Authorization = `Bearer ${accessToken}`; | ||
| } |
There was a problem hiding this comment.
Authorization 헤더에 따옴표가 포함될 수 있습니다.
Week04/gureum/mission2/src/pages/LoginPage.tsx Line 46에서 저장된 accessToken이 JSON 문자열이면 현재 코드는 Bearer "token" 형태로 전송됩니다. 토큰 파싱(또는 저장 방식 통일)이 필요합니다.
수정 예시
axiosInstance.interceptors.request.use((config) => {
- const accessToken = localStorage.getItem(LOCAL_STORAGE_KEYS.accessToken);
+ const rawToken = localStorage.getItem(LOCAL_STORAGE_KEYS.accessToken);
+ let accessToken: string | null = null;
+
+ if (rawToken) {
+ try {
+ accessToken = JSON.parse(rawToken) as string;
+ } catch {
+ accessToken = rawToken;
+ }
+ }
if (accessToken) {
config.headers.Authorization = `Bearer ${accessToken}`;
}
return config;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const accessToken = localStorage.getItem(LOCAL_STORAGE_KEYS.accessToken); | |
| if (accessToken) { | |
| config.headers.Authorization = `Bearer ${accessToken}`; | |
| } | |
| const rawToken = localStorage.getItem(LOCAL_STORAGE_KEYS.accessToken); | |
| let accessToken: string | null = null; | |
| if (rawToken) { | |
| try { | |
| accessToken = JSON.parse(rawToken) as string; | |
| } catch { | |
| accessToken = rawToken; | |
| } | |
| } | |
| if (accessToken) { | |
| config.headers.Authorization = `Bearer ${accessToken}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission2/src/api/axios.ts` around lines 9 - 13, The
Authorization header may include extra quotation marks because accessToken from
localStorage can be a JSON string; in axios request config where you read
accessToken (variable accessToken) and set config.headers.Authorization =
`Bearer ${accessToken}`, parse or normalize the stored value first (e.g.,
attempt JSON.parse(accessToken) inside a try/catch and fall back to the raw
string, or trim surrounding quotes) before composing the header so it becomes
Bearer tokenWithoutQuotes; update the logic in the module that reads
LOCAL_STORAGE_KEYS.accessToken to ensure consistent token formatting.
Comment on lines
+11
to
+14
| if (!values.email.trim()) { | ||
| errors.email = '이메일을 입력해주세요.'; | ||
| } else if (!emailRegex.test(values.email)) { | ||
| errors.email = '유효하지 않은 이메일 형식입니다.'; |
There was a problem hiding this comment.
이메일 정규식 검사 전에 trim 값을 사용해야 합니다.
현재는 공백만 제거해 빈 값 검사만 하고, 형식 검사는 원본 값으로 해서 test@example.com 같은 입력이 실패합니다.
수정 예시
export const validateLoginForm = (values: LoginFormValues) => {
const errors: Partial<Record<keyof LoginFormValues, string>> = {};
+ const email = values.email.trim();
- if (!values.email.trim()) {
+ if (!email) {
errors.email = '이메일을 입력해주세요.';
- } else if (!emailRegex.test(values.email)) {
+ } else if (!emailRegex.test(email)) {
errors.email = '유효하지 않은 이메일 형식입니다.';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!values.email.trim()) { | |
| errors.email = '이메일을 입력해주세요.'; | |
| } else if (!emailRegex.test(values.email)) { | |
| errors.email = '유효하지 않은 이메일 형식입니다.'; | |
| const email = values.email.trim(); | |
| if (!email) { | |
| errors.email = '이메일을 입력해주세요.'; | |
| } else if (!emailRegex.test(email)) { | |
| errors.email = '유효하지 않은 이메일 형식입니다.'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission2/src/constants/loginValidation.ts` around lines 11 -
14, The email validation currently trims only for the empty check but uses the
original values.email for the regex test, causing inputs like "test@example.com
" to fail; fix by computing a trimmed variable (e.g., const email =
values.email.trim()) and use that trimmed email for both the empty check and
emailRegex.test, updating assignments to errors.email accordingly so both checks
operate on the same trimmed value.
Comment on lines
+2
to
+9
| const setItem = (value: unknown) => { | ||
| window.localStorage.setItem(key, JSON.stringify(value)); | ||
| }; | ||
|
|
||
| const getItem = <T>() => { | ||
| const item = window.localStorage.getItem(key); | ||
| return item ? (JSON.parse(item) as T) : null; | ||
| }; |
There was a problem hiding this comment.
토큰 저장/조회 직렬화 방식 때문에 인증 헤더가 깨질 수 있습니다.
Line 3에서 항상 JSON.stringify로 저장하면, accessToken이 "token" 형태로 저장됩니다.
이 값은 Week04/gureum/mission2/src/pages/LoginPage.tsx Line 46에서 저장되고, Week04/gureum/mission2/src/api/axios.ts Line 8-13에서 raw 문자열로 읽혀 Bearer "token"이 될 수 있어 인증 실패를 유발할 수 있습니다.
추가로 Line 8의 JSON.parse는 예외 처리가 없어 비정상 값에서 런타임 에러가 납니다.
제안 변경안
export const useLocalStorage = (key: string) => {
const setItem = (value: unknown) => {
- window.localStorage.setItem(key, JSON.stringify(value));
+ if (typeof value === 'string') {
+ window.localStorage.setItem(key, value);
+ return;
+ }
+ window.localStorage.setItem(key, JSON.stringify(value));
};
const getItem = <T>() => {
const item = window.localStorage.getItem(key);
- return item ? (JSON.parse(item) as T) : null;
+ if (item === null) return null;
+ try {
+ return JSON.parse(item) as T;
+ } catch {
+ return item as T;
+ }
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission2/src/hooks/useLocalStorage.ts` around lines 2 - 9, The
current setItem/getItem in useLocalStorage always JSON.stringify values and
JSON.parse without error handling which causes tokens to be stored as quoted
strings (e.g., "\"token\"") and JSON.parse to throw on malformed values. Update
setItem in useLocalStorage.ts to store raw strings as-is when value is a string
(use typeof value === 'string'), otherwise JSON.stringify non-strings; update
getItem<T> to safely parse: if stored value starts with "{" or "[" try
JSON.parse inside a try/catch and return parsed result, otherwise return the raw
string; ensure getItem returns null when no entry and does not throw on parse
errors. Reference functions: setItem and getItem in useLocalStorage.ts.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
Week04/gureum/mission3/src/components/auth/SocialLoginButton.tsx (1)
3-5: onClick 핸들러 추가 권장현재 버튼에 클릭 핸들러가 없어서 사용자 상호작용을 처리할 수 없습니다. 향후 소셜 로그인 기능 구현을 위해
onClickprop을 추가하는 것을 권장합니다.♻️ 제안된 수정
interface SocialLoginButtonProps { label: string; + onClick?: () => void; } -const SocialLoginButton = ({ label }: SocialLoginButtonProps) => { +const SocialLoginButton = ({ label, onClick }: SocialLoginButtonProps) => { return ( <button type="button" + onClick={onClick} className="w-full flex items-center justify-center px-4 py-3 border border-gray-600 rounded-md bg-transparent text-white hover:bg-gray-800 transition-colors" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission3/src/components/auth/SocialLoginButton.tsx` around lines 3 - 5, Add an onClick prop to the SocialLoginButtonProps interface and pass it through in the SocialLoginButton component so the button can handle user interactions; specifically, update SocialLoginButtonProps to include onClick (type React.MouseEventHandler<HTMLButtonElement> or () => void) and ensure the SocialLoginButton component accepts that prop and assigns it to the rendered <button> element's onClick handler, preserving existing label handling.Week04/gureum/mission3/src/hooks/useForm.ts (1)
25-26:validate함수 안정성 주의
useMemo의존성 배열에validate가 포함되어 있습니다. 호출자가validate함수를useCallback으로 메모이제이션하지 않으면 매 렌더링마다errors가 재계산됩니다.♻️ 제안된 수정 (useRef 활용)
+import { useMemo, useState, useRef, useEffect } from 'react'; -import { useMemo, useState } from 'react'; import type { ChangeEvent } from 'react'; // ... export const useForm = <T extends Record<string, string>>({ initialValues, validate, }: UseFormOptions<T>) => { + const validateRef = useRef(validate); + useEffect(() => { + validateRef.current = validate; + }, [validate]); + const [values, setValues] = useState<T>(initialValues); // ... - const errors = useMemo(() => validate(values), [validate, values]); + const errors = useMemo(() => validateRef.current(values), [values]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission3/src/hooks/useForm.ts` around lines 25 - 26, The current useMemo for computing errors depends on validate, causing recomputation if the caller doesn't memoize validate; fix by storing the latest validate in a ref and reading from it inside useMemo: create const validateRef = useRef(validate), update validateRef.current = validate inside a useEffect that watches validate, and change the errors computation to const errors = useMemo(() => validateRef.current(values), [values]); this keeps errors stable across renders unless values change while still using the latest validate implementation.Week04/gureum/mission3/src/components/auth/AuthField.tsx (1)
20-31: 접근성(A11y) 개선 권장입력 필드에
<label>또는aria-label속성이 없어 스크린 리더 사용자가 필드의 용도를 파악하기 어렵습니다.placeholder만으로는 접근성 요구사항을 충족하지 못합니다.♻️ 제안된 수정
<input type={type} placeholder={placeholder} value={value} onChange={onChange} onBlur={onBlur} + aria-label={placeholder} + aria-invalid={!!error} className={`w-full px-4 py-3 bg-transparent border rounded-md text-white placeholder-gray-400 focus:outline-none focus:ring-1 ${🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission3/src/components/auth/AuthField.tsx` around lines 20 - 31, The input in AuthField (the <input> in AuthField.tsx) lacks an accessible name; update the AuthField component to accept and use either a visible <label> tied to the input via an id/htmlFor or an aria-label/aria-labelledby prop so screen readers can identify the field; implement by adding a unique id (e.g., inputId) to the input, rendering a <label htmlFor={inputId}> when a label prop is provided (or setting aria-label={label} when the label is not visible), and ensure the className/props remain unchanged so visual styles are preserved.Week04/gureum/mission3/src/hooks/useLocalStorage.ts (1)
12-22: 타입 안전성 개선 권장
getItem함수에서as T타입 단언은 런타임 검증 없이 사용되어, 실제 저장된 값과 예상 타입T가 일치하지 않을 경우 런타임 오류가 발생할 수 있습니다. 학습 프로젝트에서는 허용 가능하지만, 실무에서는 런타임 검증을 추가하거나 Zod와 같은 스키마 검증 라이브러리를 활용하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Week04/gureum/mission3/src/hooks/useLocalStorage.ts` around lines 12 - 22, The getItem<T> function performs an unchecked cast (returning JSON.parse(item) as T or item as T), which is unsafe; replace the blind casts by returning a safe type (e.g., T | null | unknown) or validate the parsed value at runtime before asserting T — either: 1) change getItem's signature to return unknown | null and let callers validate, or 2) accept a schema/validator parameter (e.g., a Zod schema or a predicate) and run it against the parsed value inside getItem to ensure it matches T, then only cast after validation; locate the getItem function and implement one of these approaches to remove the unverified as T assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Week04/gureum/mission3/src/pages/LoginPage.tsx`:
- Around line 34-36: Remove the redundant validity guard in handleLogin: delete
the `!isValid` check so the function only early-returns on `isSubmitting`,
relying on handleSubmit to perform validation; update the `handleLogin`
implementation (the function named handleLogin handling LoginFormValues) to
remove `if (!isValid || isSubmitting) return;` and replace it with a guard that
only checks `isSubmitting` to avoid blocking legitimate submits due to timing of
isValid.
In `@Week04/gureum/mission3/src/pages/SignupPage.tsx`:
- Around line 72-73: The current isStepThreeValid uses only
watchedNickname.trim().length >= 2 which allows names >20; update the Step 3
enablement to enforce the signup schema (minimum 2 and maximum 20 characters) by
changing the check to something like const isStepThreeValid = (() => { const len
= watchedNickname.trim().length; return len >= 2 && len <= 20; })(); and apply
the same bounds check to the duplicate logic around the other occurrence
referenced (the block at ~260-267) so both places match the schema's 2–20
character requirement for nickname.
---
Nitpick comments:
In `@Week04/gureum/mission3/src/components/auth/AuthField.tsx`:
- Around line 20-31: The input in AuthField (the <input> in AuthField.tsx) lacks
an accessible name; update the AuthField component to accept and use either a
visible <label> tied to the input via an id/htmlFor or an
aria-label/aria-labelledby prop so screen readers can identify the field;
implement by adding a unique id (e.g., inputId) to the input, rendering a <label
htmlFor={inputId}> when a label prop is provided (or setting aria-label={label}
when the label is not visible), and ensure the className/props remain unchanged
so visual styles are preserved.
In `@Week04/gureum/mission3/src/components/auth/SocialLoginButton.tsx`:
- Around line 3-5: Add an onClick prop to the SocialLoginButtonProps interface
and pass it through in the SocialLoginButton component so the button can handle
user interactions; specifically, update SocialLoginButtonProps to include
onClick (type React.MouseEventHandler<HTMLButtonElement> or () => void) and
ensure the SocialLoginButton component accepts that prop and assigns it to the
rendered <button> element's onClick handler, preserving existing label handling.
In `@Week04/gureum/mission3/src/hooks/useForm.ts`:
- Around line 25-26: The current useMemo for computing errors depends on
validate, causing recomputation if the caller doesn't memoize validate; fix by
storing the latest validate in a ref and reading from it inside useMemo: create
const validateRef = useRef(validate), update validateRef.current = validate
inside a useEffect that watches validate, and change the errors computation to
const errors = useMemo(() => validateRef.current(values), [values]); this keeps
errors stable across renders unless values change while still using the latest
validate implementation.
In `@Week04/gureum/mission3/src/hooks/useLocalStorage.ts`:
- Around line 12-22: The getItem<T> function performs an unchecked cast
(returning JSON.parse(item) as T or item as T), which is unsafe; replace the
blind casts by returning a safe type (e.g., T | null | unknown) or validate the
parsed value at runtime before asserting T — either: 1) change getItem's
signature to return unknown | null and let callers validate, or 2) accept a
schema/validator parameter (e.g., a Zod schema or a predicate) and run it
against the parsed value inside getItem to ensure it matches T, then only cast
after validation; locate the getItem function and implement one of these
approaches to remove the unverified as T assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a69b2d63-02ee-46c9-a04c-078ca2eb2830
⛔ Files ignored due to path filters (1)
Week04/gureum/mission3/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
Week04/gureum/mission3/.gitignoreWeek04/gureum/mission3/README.mdWeek04/gureum/mission3/eslint.config.jsWeek04/gureum/mission3/index.htmlWeek04/gureum/mission3/package.jsonWeek04/gureum/mission3/src/App.cssWeek04/gureum/mission3/src/App.tsxWeek04/gureum/mission3/src/api/auth.tsWeek04/gureum/mission3/src/api/axios.tsWeek04/gureum/mission3/src/components/Layout.tsxWeek04/gureum/mission3/src/components/Navbar.tsxWeek04/gureum/mission3/src/components/auth/AuthDivider.tsxWeek04/gureum/mission3/src/components/auth/AuthField.tsxWeek04/gureum/mission3/src/components/auth/AuthPageTitle.tsxWeek04/gureum/mission3/src/components/auth/SocialLoginButton.tsxWeek04/gureum/mission3/src/constants/key.tsWeek04/gureum/mission3/src/constants/loginValidation.tsWeek04/gureum/mission3/src/hooks/useForm.tsWeek04/gureum/mission3/src/hooks/useLocalStorage.tsWeek04/gureum/mission3/src/index.cssWeek04/gureum/mission3/src/main.tsxWeek04/gureum/mission3/src/pages/HomePage.tsxWeek04/gureum/mission3/src/pages/LoginPage.tsxWeek04/gureum/mission3/src/pages/SignupPage.tsxWeek04/gureum/mission3/src/schemas/authSchema.tsWeek04/gureum/mission3/src/types/auth.tsWeek04/gureum/mission3/src/types/common.tsWeek04/gureum/mission3/src/vite-env.d.tsWeek04/gureum/mission3/tsconfig.app.jsonWeek04/gureum/mission3/tsconfig.jsonWeek04/gureum/mission3/tsconfig.node.jsonWeek04/gureum/mission3/vite.config.ts
✅ Files skipped from review due to trivial changes (16)
- Week04/gureum/mission3/src/index.css
- Week04/gureum/mission3/index.html
- Week04/gureum/mission3/src/vite-env.d.ts
- Week04/gureum/mission3/.gitignore
- Week04/gureum/mission3/src/types/common.ts
- Week04/gureum/mission3/src/pages/HomePage.tsx
- Week04/gureum/mission3/tsconfig.app.json
- Week04/gureum/mission3/README.md
- Week04/gureum/mission3/tsconfig.json
- Week04/gureum/mission3/src/components/auth/AuthPageTitle.tsx
- Week04/gureum/mission3/package.json
- Week04/gureum/mission3/tsconfig.node.json
- Week04/gureum/mission3/src/components/auth/AuthDivider.tsx
- Week04/gureum/mission3/src/api/auth.ts
- Week04/gureum/mission3/src/types/auth.ts
- Week04/gureum/mission3/src/constants/loginValidation.ts
Comment on lines
+34
to
+36
| const handleLogin = async (values: LoginFormValues) => { | ||
| if (!isValid || isSubmitting) return; | ||
|
|
There was a problem hiding this comment.
제출 핸들러의 !isValid 가드는 제거하는 편이 안전합니다.
handleSubmit가 이미 유효성 검증을 보장하므로, 여기서 isValid를 다시 체크하면 상태 타이밍 차이로 정상 제출이 막힐 수 있습니다.
수정 예시 diff
- const handleLogin = async (values: LoginFormValues) => {
- if (!isValid || isSubmitting) return;
+ const handleLogin = async (values: LoginFormValues) => {
+ if (isSubmitting) return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleLogin = async (values: LoginFormValues) => { | |
| if (!isValid || isSubmitting) return; | |
| const handleLogin = async (values: LoginFormValues) => { | |
| if (isSubmitting) return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission3/src/pages/LoginPage.tsx` around lines 34 - 36, Remove
the redundant validity guard in handleLogin: delete the `!isValid` check so the
function only early-returns on `isSubmitting`, relying on handleSubmit to
perform validation; update the `handleLogin` implementation (the function named
handleLogin handling LoginFormValues) to remove `if (!isValid || isSubmitting)
return;` and replace it with a guard that only checks `isSubmitting` to avoid
blocking legitimate submits due to timing of isValid.
Comment on lines
+72
to
+73
| const isStepThreeValid = watchedNickname.trim().length >= 2; | ||
|
|
There was a problem hiding this comment.
닉네임 버튼 활성화 조건이 실제 스키마 검증과 불일치합니다.
현재 조건은 최소 길이만 보므로(2자 이상) 최대 길이(20자 이하) 위반 상태에서도 제출 버튼이 활성화될 수 있습니다. Step3 활성화 조건을 스키마 기준으로 맞춰주세요.
수정 예시 diff
- const isStepThreeValid = watchedNickname.trim().length >= 2;
+ const isStepThreeValid = signupSchema.shape.nickname.safeParse(watchedNickname).success;Also applies to: 260-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Week04/gureum/mission3/src/pages/SignupPage.tsx` around lines 72 - 73, The
current isStepThreeValid uses only watchedNickname.trim().length >= 2 which
allows names >20; update the Step 3 enablement to enforce the signup schema
(minimum 2 and maximum 20 characters) by changing the check to something like
const isStepThreeValid = (() => { const len = watchedNickname.trim().length;
return len >= 2 && len <= 20; })(); and apply the same bounds check to the
duplicate logic around the other occurrence referenced (the block at ~260-267)
so both places match the schema's 2–20 character requirement for nickname.
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
1 participant
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.
📝 미션 번호
4주차 Misson 1, 2, 3
📋 구현 사항
📎 스크린샷
2026-04-07.9.30.20.mov
2026-04-08.12.26.02.mov
2026-04-08.12.56.28.mov
✅ 체크리스트
🤔 질문 사항
Summary by CodeRabbit
새로운 기능
영화 애플리케이션 출시
인증 및 회원가입 시스템