[2단계 - 상세 정보 & UI/UX 개선하기] 유월 미션 제출합니다. #308
Open
vlmbuyd wants to merge 12 commits intowoowacourse:vlmbuydfrom
Open
[2단계 - 상세 정보 & UI/UX 개선하기] 유월 미션 제출합니다. #308vlmbuyd wants to merge 12 commits intowoowacourse:vlmbuydfrom
vlmbuyd wants to merge 12 commits intowoowacourse:vlmbuydfrom
Conversation
워크스루이 변경사항은 영화 목록 인터페이스를 페이지네이션 방식에서 무한 스크롤 방식으로 마이그레이션하면서 동시에 모달 기반 영화 상세보기 기능을 추가합니다. API 계층에 예상 코드 리뷰 난이도🎯 4 (복잡함) | ⏱️ ~50분 🚥 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 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: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/movieApi.ts`:
- Around line 4-20: apiRequest currently returns parsed JSON even for 4xx/5xx
responses; update apiRequest to check response.ok and, if false, parse the error
body (safely with try/catch) and throw a descriptive error (preferably a small
ApiError class containing status, statusText and parsed body) so that
fetchPopularMovies, fetchSearchMovies and fetchMovieDetail will reject on HTTP
errors instead of returning error payloads as success; ensure the thrown error
preserves the original response info for caller handling and keep the existing
function signatures for those three fetch* helpers.
In `@src/components/modal/modal.ts`:
- Around line 31-41: The modal lacks dialog semantics and focus management: set
attributes role="dialog" and aria-modal="true" on the modal element (the modal
variable) and ensure an aria-labelledby points to the modal title element
(create or assign an id to the title element and reference it). Make the modal
focusable (tabindex="-1"), capture and store document.activeElement before
opening, and in open() move focus into the modal (preferably to the closeBtn or
the first focusable control inside the modal); in close() restore focus to the
previously focused element. Update closeBtn and the modal close flow in the
close() method to ensure focus is returned and that the aria-labelledby/id pair
is present and stable.
In `@src/components/movie.ts`:
- Around line 60-63: The clickable list item only handles mouse clicks; make it
keyboard-accessible by adding li.tabIndex = 0 and li.setAttribute("role",
"button"), keep li.style.cursor = "pointer", and attach a keydown handler on li
that calls onMovieClick(id) when event.key is "Enter" or " " (Space); ensure the
existing click listener for onMovieClick(id) remains and that the handler
prevents default for Space to avoid page scroll.
In `@src/features/infiniteScroll.ts`:
- Around line 9-15: The IntersectionObserver callback sets isLoading = true then
awaits onLoadMore() but if onLoadMore or container.appendChild(trigger) throws
the isLoading flag is never reset; update the callback used when creating the
IntersectionObserver to wrap the await onLoadMore() and the
container.appendChild(trigger) in a try/finally and move isLoading = false into
the finally block so isLoading is always cleared (keep the early return that
checks !entry.isIntersecting || isLoading unchanged).
- Around line 20-23: There’s a race where the cleanup returned (which
disconnects observer and removes trigger) can run before the async callback
resumes, allowing the callback to re-append trigger; add a stopped (or
destroyed) boolean flag scoped alongside observer/trigger, set stopped = true in
the cleanup that calls observer.disconnect() and trigger.remove(), and then
guard any code that appends or restores the trigger (the code path that
currently appends `trigger`) to skip append/state restoration when stopped is
true; reference and update the logic around `observer` and `trigger` to check
this flag before mutating DOM.
In `@src/features/popular.ts`:
- Around line 19-28: The infinite-scroll callback passed to setupInfiniteScroll
should guard against fetch failures so the skeleton is always cleaned up and the
scroll loader can recover: in the anonymous async callback that increments page,
createSkeleton()/mainEl.appendChild(skeleton) and then wrap the
fetchPopularMovies(page) + skeleton.replaceWith(createMovieList(...,
onMovieClick)) sequence in a try/catch/finally — on success do the replace and
stop() check, on error remove the skeleton (and optionally append an error/retry
UI or console.error) and do not rethrow so setupInfiniteScroll’s loading state
can reset; reference the existing symbols setupInfiniteScroll,
fetchPopularMovies, createSkeleton, mainEl, stop, createMovieList, and
onMovieClick to locate where to add the try/catch/finally.
In `@src/features/search.ts`:
- Around line 18-49: Wrap both initial and paginated calls to fetchSearchMovies
inside try/catch blocks so failures always run cleanup: ensure createSkeleton()
instances are removed or replaced, any loading state inside setupInfiniteScroll
is cleared (call the stop function or the scroll loader's cancel method), and
display a user-friendly error element instead of leaving the skeleton.
Specifically, modify the search handler that uses createSkeleton(),
fetchSearchMovies(query, page), setupInfiniteScroll(mainEl, ...) and the
moreSkeleton replacement logic so that on any fetch error you remove/replace the
skeleton, call stop() (or the infinite scroll's cleanup) to release loading
state, append an error message to mainEl, and still return a valid stop
function.
In `@src/main.ts`:
- Around line 52-58: The submit handler can let an earlier slower handleSearch
overwrite stopInfiniteScroll and UI; to fix, serialize or cancel previous
requests so only the latest search's results and stopInfiniteScroll are applied:
add a shared "currentRequestId" (or store an AbortController) in the outer
scope, increment it (or abort previous controller) before calling handleSearch,
pass the id (or signal) into handleSearch, and inside handleSearch only update
DOM and return/assign stopInfiniteScroll if the received id matches
currentRequestId (or if the signal is not aborted). Update the form submit
listener to cancel/mark previous requests before awaiting handleSearch and
ensure handleSearch signature accepts the id or AbortSignal and ignores stale
results.
🪄 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: 13f42c62-528e-44a8-832d-9d5ed4a383d9
📒 Files selected for processing (15)
index.htmlsrc/api/api.tssrc/api/movieApi.tssrc/components/modal/modal.htmlsrc/components/modal/modal.tssrc/components/movie.tssrc/features/infiniteScroll.tssrc/features/movieRenderer.tssrc/features/popular.tssrc/features/search.tssrc/main.tssrc/styles/main.csssrc/styles/modal.csssrc/styles/thumbnail.csssrc/utils/constants.ts
💤 Files with no reviewable changes (1)
- src/components/modal/modal.html
Comment on lines
+4
to
+20
| export const fetchPopularMovies = (page: number): Promise<MovieResponse> => | ||
| apiRequest<MovieResponse>({ | ||
| url: `/movie/popular?language=ko-KR&page=${page}`, | ||
| }); | ||
|
|
||
| export const fetchSearchMovies = ( | ||
| query: string, | ||
| page: number, | ||
| ): Promise<MovieResponse> => | ||
| apiRequest<MovieResponse>({ | ||
| url: `/search/movie?language=ko-KR&query=${encodeURIComponent(query)}&page=${page}`, | ||
| }); | ||
|
|
||
| export const fetchMovieDetail = (id: number): Promise<MovieDetail> => | ||
| apiRequest<MovieDetail>({ | ||
| url: `/movie/${id}?language=ko-KR`, | ||
| }); |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
set -e
# apiRequest가 HTTP status 실패를 throw 하는지 확인
rg -n --type=ts -C4 'export const apiRequest|res\.ok|throw new Error|res\.json\('
# movieApi 호출부에서 실패 분기 처리가 있는지 확인
rg -n --type=ts -C3 'fetchPopularMovies\(|fetchSearchMovies\(|fetchMovieDetail\('Repository: woowacourse/javascript-movie-review
Repository: woowacourse/javascript-movie-review
Exit code: 0
stdout:
src/utils/api.ts-2- url: string;
src/utils/api.ts-3- method?: "GET" | "POST" | "PUT" | "DELETE";
src/utils/api.ts-4-}
src/utils/api.ts-5-
src/utils/api.ts:6:export const apiRequest = async <T>({
src/utils/api.ts-7- url,
src/utils/api.ts-8- method = "GET",
src/utils/api.ts-9-}: FetchOptions): Promise<T> => {
src/utils/api.ts-10- return await fetch(`${import.meta.env.VITE_BASE_URL}${url}`, {
--
src/utils/api.ts-12- headers: {
src/utils/api.ts-13- Authorization: `Bearer ${import.meta.env.VITE_TMDB_API_KEY}`,
src/utils/api.ts-14- "Content-Type": "application/json",
src/utils/api.ts-15- },
src/utils/api.ts:16: }).then((res) => res.json());
src/utils/api.ts-17-};
src/main.ts-18- // 카드 클릭 이벤트 핸들러
src/main.ts-19- const onMovieClick = async (id: number) => {
src/main.ts-20- try {
src/main.ts:21: const detail = await fetchMovieDetail(id);
src/main.ts-22- modal.open({
src/main.ts-23- id: detail.id,
src/main.ts-24- title: detail.title,
--
src/features/search.ts-19- mainEl.appendChild(skeleton);
src/features/search.ts-20-
src/features/search.ts-21- let page = 1;
src/features/search.ts:22: const data = await fetchSearchMovies(query, page);
src/features/search.ts-23- skeleton.remove();
src/features/search.ts-24-
src/features/search.ts-25- if (data.results.length === 0) {
--
src/features/search.ts-40- const moreSkeleton = createSkeleton();
src/features/search.ts-41- mainEl.appendChild(moreSkeleton);
src/features/search.ts-42-
src/features/search.ts:43: const nextData = await fetchSearchMovies(query, page);
src/features/search.ts-44- moreSkeleton.replaceWith(createMovieList(nextData.results, onMovieClick));
src/features/search.ts-45-
src/features/search.ts-46- if (nextData.total_pages <= page) stop();
--
src/features/popular.ts-10-): Promise<() => void> => {
src/features/popular.ts-11- let page = 1;
src/features/popular.ts-12-
src/features/popular.ts:13: const data = await fetchPopularMovies(page);
src/features/popular.ts-14- skeletonEls.replaceWith(createMovieList(data.results, onMovieClick));
src/features/popular.ts-15-
src/features/popular.ts-16- if (data.total_pages <= page) return () => {};
--
src/features/popular.ts-21- const skeleton = createSkeleton();
src/features/popular.ts-22- mainEl.appendChild(skeleton);
src/features/popular.ts-23-
src/features/popular.ts:24: const nextData = await fetchPopularMovies(page);
src/features/popular.ts-25- skeleton.replaceWith(createMovieList(nextData.results, onMovieClick));
src/features/popular.ts-26-
src/features/popular.ts-27- if (nextData.total_pages <= page) stop();
--
src/features/movieRenderer.ts-17- mainEl.append(skeletonEls, loadMoreBtnEl);
src/features/movieRenderer.ts-18-
src/features/movieRenderer.ts-19- const data = query
src/features/movieRenderer.ts:20: ? await fetchSearchMovies(query, page)
src/features/movieRenderer.ts:21: : await fetchPopularMovies(page);
src/features/movieRenderer.ts-22-
src/features/movieRenderer.ts-23- if (data) {
src/features/movieRenderer.ts-24- loadMoreBtnEl.disabled = false;
HTTP 오류 응답이 성공으로 전파될 위험이 있습니다.
apiRequest 구현을 보니 res.ok 검증이 없어서, 4xx/5xx 상태 코드도 응답의 JSON 파싱에 성공하면 그대로 반환됩니다. 이렇게 되면 호출부에서 예상한 MovieResponse나 MovieDetail 타입 구조와 일치하지 않는 에러 응답을 받게 되어 런타임 오류를 유발할 수 있어요.
현재 fetchPopularMovies, fetchSearchMovies, fetchMovieDetail을 호출하는 대부분의 곳에서 try/catch로 감싸지 않고 있다는 점도 고려해보세요. 다음을 점검해보시길 권합니다:
apiRequest에서 HTTP 상태 코드를 어떻게 검증할지- 에러 발생 시 호출부에서 이를 적절히 처리할 수 있도록 설계할지
- 현재 설계에서 정말로 모든 호출부가 에러에 안전한지
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/movieApi.ts` around lines 4 - 20, apiRequest currently returns parsed
JSON even for 4xx/5xx responses; update apiRequest to check response.ok and, if
false, parse the error body (safely with try/catch) and throw a descriptive
error (preferably a small ApiError class containing status, statusText and
parsed body) so that fetchPopularMovies, fetchSearchMovies and fetchMovieDetail
will reject on HTTP errors instead of returning error payloads as success;
ensure the thrown error preserves the original response info for caller handling
and keep the existing function signatures for those three fetch* helpers.
Comment on lines
+31
to
+41
| const modal = document.createElement("div"); | ||
| modal.className = "modal"; | ||
|
|
||
| const closeBtn = document.createElement("button"); | ||
| closeBtn.className = "close-modal"; | ||
| closeBtn.setAttribute("aria-label", "모달 닫기"); | ||
| const closeImg = document.createElement("img"); | ||
| closeImg.src = closeButtonSrc; | ||
| closeImg.alt = "닫기"; | ||
| closeBtn.appendChild(closeImg); | ||
| closeBtn.addEventListener("click", () => this.close()); |
There was a problem hiding this comment.
모달에 다이얼로그 의미론과 포커스 이동이 빠져 있습니다.
닫기 버튼의 aria-label은 좋지만, .modal 자체에 role="dialog" / aria-modal="true"가 없고 open()에서도 초기 포커스를 모달 내부로 옮기지 않고 있습니다. 이 상태면 보조기기와 키보드 사용자가 모달이 열렸는지, 어디가 활성 영역인지 파악하기 어렵습니다. 제목을 어떤 방식으로 연결할지(aria-labelledby)와 열릴 때/닫힐 때 포커스를 어디로 보낼지부터 먼저 정리해 보시면 좋겠습니다.
Also applies to: 67-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/modal/modal.ts` around lines 31 - 41, The modal lacks dialog
semantics and focus management: set attributes role="dialog" and
aria-modal="true" on the modal element (the modal variable) and ensure an
aria-labelledby points to the modal title element (create or assign an id to the
title element and reference it). Make the modal focusable (tabindex="-1"),
capture and store document.activeElement before opening, and in open() move
focus into the modal (preferably to the closeBtn or the first focusable control
inside the modal); in close() restore focus to the previously focused element.
Update closeBtn and the modal close flow in the close() method to ensure focus
is returned and that the aria-labelledby/id pair is present and stable.
Comment on lines
+60
to
+63
| if (onMovieClick) { | ||
| li.style.cursor = "pointer"; | ||
| li.addEventListener("click", () => onMovieClick(id)); | ||
| } |
There was a problem hiding this comment.
클릭 가능한 li가 키보드 사용자에게는 동작하지 않습니다.
현재는 마우스 클릭만 처리하고 있어 접근성 이슈가 있습니다. button/a 사용 또는 tabIndex, role, Enter/Space 키 처리까지 포함되는지 점검해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/movie.ts` around lines 60 - 63, The clickable list item only
handles mouse clicks; make it keyboard-accessible by adding li.tabIndex = 0 and
li.setAttribute("role", "button"), keep li.style.cursor = "pointer", and attach
a keydown handler on li that calls onMovieClick(id) when event.key is "Enter" or
" " (Space); ensure the existing click listener for onMovieClick(id) remains and
that the handler prevents default for Space to avoid page scroll.
Comment on lines
+9
to
+15
| const observer = new IntersectionObserver(async ([entry]) => { | ||
| if (!entry.isIntersecting || isLoading) return; | ||
| isLoading = true; | ||
| await onLoadMore(); | ||
|
|
||
| container.appendChild(trigger); | ||
| isLoading = false; |
There was a problem hiding this comment.
onLoadMore 실패 시 무한 스크롤이 영구 정지될 수 있습니다.
Line 12에서 예외가 나면 isLoading이 false로 복구되지 않아 이후 교차 이벤트를 계속 무시합니다. try/finally로 잠금 해제를 보장하는 방향으로 점검해보실까요?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/infiniteScroll.ts` around lines 9 - 15, The IntersectionObserver
callback sets isLoading = true then awaits onLoadMore() but if onLoadMore or
container.appendChild(trigger) throws the isLoading flag is never reset; update
the callback used when creating the IntersectionObserver to wrap the await
onLoadMore() and the container.appendChild(trigger) in a try/finally and move
isLoading = false into the finally block so isLoading is always cleared (keep
the early return that checks !entry.isIntersecting || isLoading unchanged).
Comment on lines
+20
to
+23
| return () => { | ||
| observer.disconnect(); | ||
| trigger.remove(); | ||
| }; |
There was a problem hiding this comment.
정리 함수 호출 이후에도 trigger가 다시 붙는 경쟁 상태가 있습니다.
로딩 중 cleanup이 먼저 호출되면, 콜백 재개 후 Line 14에서 trigger를 다시 append할 수 있습니다. “중지됨” 상태 플래그를 두고 append/상태복구를 건너뛰는지 확인해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/infiniteScroll.ts` around lines 20 - 23, There’s a race where
the cleanup returned (which disconnects observer and removes trigger) can run
before the async callback resumes, allowing the callback to re-append trigger;
add a stopped (or destroyed) boolean flag scoped alongside observer/trigger, set
stopped = true in the cleanup that calls observer.disconnect() and
trigger.remove(), and then guard any code that appends or restores the trigger
(the code path that currently appends `trigger`) to skip append/state
restoration when stopped is true; reference and update the logic around
`observer` and `trigger` to check this flag before mutating DOM.
Comment on lines
+19
to
+28
| stop = setupInfiniteScroll(mainEl, async () => { | ||
| page++; | ||
| const skeleton = createSkeleton(); | ||
| mainEl.appendChild(skeleton); | ||
|
|
||
| const nextData = await fetchPopularMovies(page); | ||
| skeleton.replaceWith(createMovieList(nextData.results, onMovieClick)); | ||
|
|
||
| if (nextData.total_pages <= page) stop(); | ||
| }); |
There was a problem hiding this comment.
추가 로드 실패가 무한 스크롤을 사실상 멈추게 할 수 있습니다.
Line 24의 요청이 reject되면 이 콜백이 그대로 setupInfiniteScroll(...) 밖으로 전파됩니다. 그런데 src/features/infiniteScroll.ts Line 7-16은 await onLoadMore()를 try/finally로 감싸지 않아서 isLoading이 true에 남고, 이후에는 교차 감지가 와도 더 이상 로드되지 않습니다. 여기서는 적어도 실패 시 스켈레톤 정리와 재시도 가능한 상태 복구가 보장되도록 흐름을 한 번 감싸 두는 편이 좋겠습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/popular.ts` around lines 19 - 28, The infinite-scroll callback
passed to setupInfiniteScroll should guard against fetch failures so the
skeleton is always cleaned up and the scroll loader can recover: in the
anonymous async callback that increments page,
createSkeleton()/mainEl.appendChild(skeleton) and then wrap the
fetchPopularMovies(page) + skeleton.replaceWith(createMovieList(...,
onMovieClick)) sequence in a try/catch/finally — on success do the replace and
stop() check, on error remove the skeleton (and optionally append an error/retry
UI or console.error) and do not rethrow so setupInfiniteScroll’s loading state
can reset; reference the existing symbols setupInfiniteScroll,
fetchPopularMovies, createSkeleton, mainEl, stop, createMovieList, and
onMovieClick to locate where to add the try/catch/finally.
Comment on lines
+18
to
+49
| const skeleton = createSkeleton(); | ||
| mainEl.appendChild(skeleton); | ||
|
|
||
| let page = 1; | ||
| const data = await fetchSearchMovies(query, page); | ||
| skeleton.remove(); | ||
|
|
||
| if (data.results.length === 0) { | ||
| const noSearchResultEl = document.createElement("p"); | ||
| noSearchResultEl.textContent = "검색 결과가 없습니다."; | ||
| noSearchResultEl.className = "no-search-result"; | ||
| mainEl.appendChild(noSearchResultEl); | ||
| return; | ||
| const noResultEl = document.createElement("p"); | ||
| noResultEl.className = "no-search-result"; | ||
| noResultEl.textContent = "검색 결과가 없습니다."; | ||
| mainEl.appendChild(noResultEl); | ||
| return () => {}; | ||
| } | ||
|
|
||
| if (data.results.length > 0) { | ||
| const searchResult = createMovieList(data.results); | ||
| mainEl.append(searchResult, loadMoreBtnEl); | ||
| } | ||
| mainEl.appendChild(createMovieList(data.results, onMovieClick)); | ||
|
|
||
| if (data.total_pages <= page) return () => {}; | ||
|
|
||
| loadMoreBtnEl.onclick = () => { | ||
| let stop = () => {}; | ||
| stop = setupInfiniteScroll(mainEl, async () => { | ||
| page++; | ||
| handleLoadMoreButton( | ||
| `/search/movie?language=ko-KR&query=${query}&page=${page}`, | ||
| loadMoreBtnEl, | ||
| mainEl, | ||
| skeletonEls, | ||
| ); | ||
| }; | ||
| const moreSkeleton = createSkeleton(); | ||
| mainEl.appendChild(moreSkeleton); | ||
|
|
||
| const nextData = await fetchSearchMovies(query, page); | ||
| moreSkeleton.replaceWith(createMovieList(nextData.results, onMovieClick)); | ||
|
|
||
| if (nextData.total_pages <= page) stop(); | ||
| }); | ||
|
|
||
| return stop; |
There was a problem hiding this comment.
검색 fetch 실패 시 화면과 스크롤 상태를 복구할 경로가 없습니다.
Line 22나 Line 43이 실패하면 스켈레톤 제거/교체가 건너뛰어지고, 추가 로드 쪽은 src/features/infiniteScroll.ts의 로딩 상태도 같이 풀리지 않습니다. 그 결과 검색 화면에 로딩 UI가 남거나 이후 스크롤 로드가 멈출 수 있습니다. 첫 페이지와 다음 페이지 모두 실패 시 정리 로직과 사용자 피드백이 동일하게 보장되도록 한 단계 묶어 두는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/search.ts` around lines 18 - 49, Wrap both initial and paginated
calls to fetchSearchMovies inside try/catch blocks so failures always run
cleanup: ensure createSkeleton() instances are removed or replaced, any loading
state inside setupInfiniteScroll is cleared (call the stop function or the
scroll loader's cancel method), and display a user-friendly error element
instead of leaving the skeleton. Specifically, modify the search handler that
uses createSkeleton(), fetchSearchMovies(query, page),
setupInfiniteScroll(mainEl, ...) and the moreSkeleton replacement logic so that
on any fetch error you remove/replace the skeleton, call stop() (or the infinite
scroll's cleanup) to release loading state, append an error message to mainEl,
and still return a valid stop function.
src/main.ts
Outdated
Comment on lines
+52
to
+58
| form.addEventListener("submit", async (event) => { | ||
| event.preventDefault(); | ||
| const query = input.value.trim(); | ||
| if (!query) return; | ||
|
|
||
| stopInfiniteScroll(); | ||
| stopInfiniteScroll = await handleSearch(query, mainEl, titleEl, onMovieClick); |
There was a problem hiding this comment.
연속 검색 시 오래 걸린 이전 응답이 최신 결과를 덮어쓸 수 있어요.
지금 흐름은 submit마다 handleSearch(...)를 바로 기다리기만 해서, 먼저 보낸 검색이 더 늦게 끝나면 최신 결과와 stopInfiniteScroll 핸들을 다시 덮어쓸 수 있습니다. 그럼 화면에 현재 입력값과 다른 목록이 보이거나, 이후 cleanup이 엉뚱한 observer를 끊는 상태가 생깁니다. 마지막 요청만 반영되도록 요청 순번을 두거나 취소 가능한 흐름으로 묶는 쪽이 안전해 보입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.ts` around lines 52 - 58, The submit handler can let an earlier
slower handleSearch overwrite stopInfiniteScroll and UI; to fix, serialize or
cancel previous requests so only the latest search's results and
stopInfiniteScroll are applied: add a shared "currentRequestId" (or store an
AbortController) in the outer scope, increment it (or abort previous controller)
before calling handleSearch, pass the id (or signal) into handleSearch, and
inside handleSearch only update DOM and return/assign stopInfiniteScroll if the
received id matches currentRequestId (or if the signal is not aborted). Update
the form submit listener to cancel/mark previous requests before awaiting
handleSearch and ensure handleSearch signature accepts the id or AbortSignal and
ignores stale results.
vlmbuyd
changed the title
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.
🎯 미션 소개
🕵️ 셀프 리뷰(Self-Review)
제출 전 체크 리스트
리뷰 요청 & 논의하고 싶은 내용
1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점
Repository 패턴 적용
이 조건이 이번 미션의 핵심이라 생각하고 여러 방향으로 고민했지만, 스스로 만족스러운 답을 찾지는 못했습니다.
크루들과 토론하던 중 Repository 패턴을 적용한 코드를 접하게 되었고, AI의 도움을 받아 이를 적용해보았습니다.
이 과정에서 개방-폐쇄 원칙이 왜 필요한지는 어느 정도 이해했지만,
스스로 고민해서 도달한 결론이 아니다 보니 완전히 납득하기 어려운 부분도 남아있습니다.
해당 패턴이 이번 미션의 의도에 부합하는 방향인지 여쭤봐도 될까요?
2) 이번 리뷰를 통해 논의하고 싶은 부분
SRP
이번 단계에서는 기능 구현을 우선시하다 보니, 1단계 PR에서 피드백받은 모듈별 책임 분리에는 충분히 신경 쓰지 못했습니다.
modal.ts를 예로 들면, UI를 컴포넌트처럼 구성하고 싶어 createBackground, renderContent 등에서 요소를 생성해 반환하는 방식을 택했는데, 그러다 보니 개별 메서드가 지나치게 길어졌습니다.
프리코스 때부터 SRP를 잘 지키고 있는지 가늠하는 기준으로 메서드 길이를 봐왔는데, DOM 생성·클래스·텍스트 설정 등 절차적이지만 생략할 수 없는 코드들이 포함되다 보니 어쩔 수 없이 길어지는 부분도 있었습니다.
애초에 컴포넌트 방식으로 설계한 것 자체가 잘못된 방향이었는지, 아니면 구조적으로 개선할 여지가 있는지 여쭤보고 싶습니다.
시간 관리에 대한 조언
돌이켜보면 고등학교 때부터 비슷한 패턴이 있었던 것 같습니다.
잠을 줄여가며 열심히 했지만 성적이 따라오지 못하는 경우도 있었고, 욕심이 많아 일을 이것저것 늘리다 정작 중요한 것에 집중하지 못하기도 했습니다. 결과적으로 입시에서 쓴 경험을 했고, 그것을 온전히 극복하지 못했다는 생각도 듭니다.
우테코에서도 비슷한 상황이 반복되는 것 같아 고민이 됩니다.
딥다이브, 오픈소스, JS, FP 등 스터디를 여럿 병행하다 보니 이번 미션에 충분히 집중하지 못했고, 좋은 리뷰어 분을 만났음에도 논의와 토론을 충분히 활용하지 못한 점이 아쉽습니다.
하고있는 것들을 포기하기는 싫어 주말을 포함한 모든 시간을 쏟고 있는데, 그럼에도 어느 하나 제대로 얻어가지 못하는 것 같다는 느낌이 듭니다.
리뷰어님께서는 시간 관리를 어떻게 하시는지, 지금의 저에게 해주실 수 있는 조언이 있다면 조심스럽게 부탁드려 봐도 될까요?
✅ 리뷰어 체크 포인트