Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (21)
Walkthrough링크 관련 클라이언트 호출들을 SafeFetch/withAuth 패턴에서 clientApiClient/serverApiClient 호출로 부분 마이그레이션했고, fetchLinks, fetchLink, createLink, updateLink, checkDuplicateLink, scrapeLinkMeta 등에서 응답의 Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/apis/linkApi.ts (1)
178-180:⚠️ Potential issue | 🟠 Major중복 검사 응답에서
success검증이 빠져 있습니다.Line 178-180은
data.exists만 확인하고 있어,success: false인데data가 포함된 경우를 정상 응답으로 처리할 수 있습니다. 중복 저장 흐름에서 오판 가능성이 있습니다.제안 diff
- if (!body?.data || typeof body.data.exists !== 'boolean') { + if (!body?.success || !body?.data || typeof body.data.exists !== 'boolean') { throw new Error(body?.message ?? 'Invalid response'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/linkApi.ts` around lines 178 - 180, The response validation currently only checks body?.data.exists and can treat responses with success: false as valid; update the validation logic that consumes body (the block referencing body, data.exists and body?.message) to also assert body?.success === true and that body?.data?.exists is a boolean, and throw an Error (using body?.message ?? 'Invalid response') if success is not true or data/exists are missing/invalid so duplicate-check flows won't misclassify failures as valid responses.src/app/(dev)/link-api-demo/LinkApiDemo.tsx (1)
26-26:⚠️ Potential issue | 🟡 Minor사용되지 않는 상태 변수
duplicateQueryUrl을 제거해야 합니다.
useDuplicateLinkMutation으로 변경하면서 쿼리 기반 URL 상태가 더 이상 필요하지 않습니다. 이 변수는 선언만 되어 있고 사용되지 않아 데드 코드입니다.🧹 데드 코드 제거 제안
const [createForm, setCreateForm] = useState(defaultCreate); const [duplicateUrl, setDuplicateUrl] = useState(''); - const [duplicateQueryUrl, setDuplicateQueryUrl] = useState<string | undefined>(); const { data, isLoading, isError, refetch } = useGetLinks({ size: 20 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(dev)/link-api-demo/LinkApiDemo.tsx at line 26, Remove the dead state declaration duplicateQueryUrl and its setter setDuplicateQueryUrl from LinkApiDemo.tsx (they are defined via useState but never used after switching to useDuplicateLinkMutation); also remove any related unused imports or references (e.g., remove the useState import if it becomes unused) and ensure no remaining code references duplicateQueryUrl or setDuplicateQueryUrl so the component compiles without unused-variable warnings.
🧹 Nitpick comments (4)
src/components/basics/Modal/Modal.style.ts (1)
16-17: 공용 모달 body 기본 패딩 제거는 변형(variant)으로 분리하는 편이 안전합니다.현재 변경은 모든 모달 본문 레이아웃에 영향을 줄 수 있습니다. 기본값은 유지하고, 중복 링크 모달만
noPadding변형을 쓰는 구조를 권장합니다.제안 diff
export const modalBodyStyle = tv({ - base: '', + base: 'p-3', + variants: { + padding: { + default: 'p-3', + none: '', + }, + }, + defaultVariants: { + padding: 'default', + }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/basics/Modal/Modal.style.ts` around lines 16 - 17, Revert removal of the default modal body padding in modalBodyStyle and instead add a variant (e.g., variant key noPadding) that removes padding; specifically, keep modalBodyStyle.base with the original padding value, add a variants: { noPadding: { true: 'p-0' } } (or equivalent) so the default layout is preserved, and update the duplicate-link modal to use modalBodyStyle({ variant: 'noPadding' }) (or the project’s variant API) where needed.src/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsx (1)
14-17: 중요 경고 배너에 접근성 알림 role을 추가해 주세요.중복 덮어쓰기 경고는 핵심 정보라 스크린리더에도 즉시 전달되도록
role="alert"(또는aria-live="assertive") 적용을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsx` around lines 14 - 17, The DuplicateBanner component's warning div (in DuplicateBanner.tsx) lacks an accessibility live region; update the outer container element (the div wrapping SVGIcon and paragraph) to include role="alert" (or aria-live="assertive") so screen readers immediately announce the duplicate-overwrite warning; ensure any non-essential aria attributes aren't conflicting and keep the SVGIcon and text unchanged.src/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsx (1)
44-46: Image 컴포넌트에objectFit스타일 추가를 권장합니다.
fillprop을 사용할 때 이미지 비율이 컨테이너와 맞지 않으면 왜곡될 수 있습니다. 썸네일 이미지가 다양한 비율을 가질 수 있으므로object-cover또는object-contain스타일을 적용하는 것이 좋습니다.♻️ 스타일 추가 제안
- <Image src={previewImageUrl} alt="link thumbnail" fill /> + <Image src={previewImageUrl} alt="link thumbnail" fill className="object-cover" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsx` around lines 44 - 46, In LinkThumbnailTitleSection.tsx the Image rendering (Image src={previewImageUrl} alt="link thumbnail" fill) needs an object-fit style to avoid distortion for varying aspect ratios; update the Image element to include a className or style that applies object-cover (or object-contain if you prefer letterboxing) so the thumbnail preserves its aspect ratio when using the fill prop—locate the Image usage in LinkThumbnailTitleSection and add the appropriate className="object-cover" or style={{ objectFit: 'cover' }} on that Image.src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.ts (1)
51-71:onRetry콜백이 메모이제이션되지 않으면 불필요한 effect 재실행이 발생할 수 있습니다.
onRetry가 의존성 배열에 포함되어 있어, 부모 컴포넌트에서useCallback으로 감싸지 않으면 매 렌더링마다 effect가 재실행될 수 있습니다. 또한, 토스트의onAction콜백이 effect 생성 시점의onRetry를 캡처하므로, 이후onRetry가 변경되면 이전 버전이 호출됩니다.이 훅을 사용하는 곳에서
onRetry를useCallback으로 감싸거나, 훅 내부에서 ref를 사용하여 최신 콜백을 참조하는 방식을 고려해 주세요.♻️ ref를 사용한 최신 콜백 참조 방식
+ const onRetryRef = useRef(onRetry); + useEffect(() => { + onRetryRef.current = onRetry; + }, [onRetry]); useEffect(() => { if (!createLink.isError || !createLink.error) return; if (lastSubmitErrorRef.current === createLink.error) return; lastSubmitErrorRef.current = createLink.error; setHasSubmitError(true); const toastId = showToast({ message: '링크를 저장하지 못했습니다. 잠시 후 다시 시도해 주세요.', variant: 'error', showIcon: true, placement: 'modal-bottom', actionLabel: '다시 시도', actionLabelIcon: 'IC_Regenerate', onAction: () => { const payload = lastSubmitPayloadRef.current; if (!payload) return; hideToast(toastId); lastSubmitErrorRef.current = null; - onRetry(payload); + onRetryRef.current(payload); }, }); - }, [createLink.isError, createLink.error, onRetry]); + }, [createLink.isError, createLink.error]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.ts` around lines 51 - 71, The effect in useCreateLinkError captures the onRetry callback causing unnecessary re-runs and stale references; fix by creating a ref (e.g., latestOnRetryRef) inside useCreateLinkError, update latestOnRetryRef.current = onRetry in a small effect that depends on onRetry, then remove onRetry from the dependency array of the main useEffect that shows the toast and instead call latestOnRetryRef.current(payload) from the toast onAction; keep existing refs like lastSubmitErrorRef and lastSubmitPayloadRef and ensure hideToast and onRetry invocations use the ref to access the latest callback.
🤖 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/app/api/links/duplicate/route.ts`:
- Around line 5-14: GET currently forwards an empty url to the backend; add
validation in the GET handler to check the extracted url (the url variable) and
return early with a clear error response (e.g., 400/validation JSON) instead of
calling serverApiClient when url is null/empty, and keep existing error handling
via handleApiError for other failures; locate this logic in the GET function
where searchParams and url are read and before the
serverApiClient(`/v1/links/duplicate?...`) call.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useAddLinkForm.ts`:
- Line 12: The title field in the Zod schema (in useAddLinkForm.ts) allows
all-space strings because it uses z.string().min(1); change the schema to trim
the input before length validation by applying a transform (e.g., .transform(s
=> s.trim())) or Zod's .trim() helper, then validate with .min(1, { message:
'제목을 입력해 주세요.' }); update any form submission or value handling to use the
transformed value so the stored title is the trimmed string.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useDuplicateCheck.ts`:
- Around line 29-49: Before calling duplicateCheck.mutate, capture the current
trimmedUrl into a local constant (e.g., const urlAtCall = trimmedUrl) and then
in the onSuccess handler (and after the await fetchLink(linkId) resolves)
validate that urlAtCall still equals the latest trimmedUrl; if it does not,
abort setting state (setIsDuplicate/setDuplicateLinkId/setDuplicateLinkData) to
avoid stale data from a previous request. This ensures the asynchronous
fetchLink and subsequent state updates only apply if the URL hasn’t changed
since the mutation was initiated.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsx`:
- Around line 94-116: The current onSubmit swallows all errors (empty catch),
hiding failures from updateLink when isDuplicate is true; modify onSubmit so
updateLink failures surface to the user by either wrapping the duplicate-branch
updateLink call in its own try/catch that forwards the error to the existing UI
error handler (e.g., call the same handler used by useCreateLinkError or a
toast/ setError) or by capturing the caught error and rethrowing it when
duplicateLinkId is set; reference onSubmit, updateLink, duplicateLinkId and
useCreateLinkError to locate where to add the per-branch error
propagation/notification logic.
- Around line 188-190: The submit button's disabled logic only checks
createLink.isPending so it stays enabled during an update and allows duplicate
submissions; update the disabled condition on the submit control (where
type="submit" and label is set) to also include the updateLink pending state
(e.g., check updateLink.isPending or a combined isPending flag) and any other
relevant loading flags (isValid, metaLoading) so the button is disabled while
either createLink or updateLink is pending.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsx`:
- Line 49: The div in LinkThumbnailTitleSection (the element with className
"flex flex-3 flex-col") uses a non-existent Tailwind class `flex-3`; update that
element to use a valid utility (e.g., replace `flex-3` with `flex-1`,
`flex-auto`, or `flex-none`) or alternatively add a custom `flex-3` utility in
your Tailwind config if you intentionally need that exact behavior; modify the
className on the div in the LinkThumbnailTitleSection component accordingly and
ensure any tests/styles referencing `flex-3` are updated to the chosen solution.
In `@src/hooks/useLinkMetaScrape.ts`:
- Around line 103-113: The code is overwriting user edits because the async
response handler uses a stale dirtyFields snapshot; fix by gating the setValue
calls with a request-version check and fresh dirty state: before starting the
fetch increment a latestRequestIdRef and capture that id locally, and on
response verify latestRequestIdRef.current === localId and
skipAutoFillRef.current is still false; also read current dirty state from a ref
or getValues (e.g., use dirtyFieldsRef.current or form.getValues/getDirtyFields)
rather than the stale closed-over dirtyFields, then only call
setValue(titlePath, data.title...) and setValue(memoPath, data.description...)
when the verified request id matches and the current dirty state shows the field
is not dirty.
In `@src/types/api/linkApi.ts`:
- Line 2: Replace the default zod import in this module with the named import
used across the codebase: change the current "import z from 'zod'" to "import {
z } from 'zod'" so type-checking and build won't fail under stricter TS interop
settings; update any usages in src/types/api/linkApi.ts that refer to the
default import (e.g., any references to z) to use the same named symbol and
ensure consistency with other files using { z }.
---
Outside diff comments:
In `@src/apis/linkApi.ts`:
- Around line 178-180: The response validation currently only checks
body?.data.exists and can treat responses with success: false as valid; update
the validation logic that consumes body (the block referencing body, data.exists
and body?.message) to also assert body?.success === true and that
body?.data?.exists is a boolean, and throw an Error (using body?.message ??
'Invalid response') if success is not true or data/exists are missing/invalid so
duplicate-check flows won't misclassify failures as valid responses.
In `@src/app/`(dev)/link-api-demo/LinkApiDemo.tsx:
- Line 26: Remove the dead state declaration duplicateQueryUrl and its setter
setDuplicateQueryUrl from LinkApiDemo.tsx (they are defined via useState but
never used after switching to useDuplicateLinkMutation); also remove any related
unused imports or references (e.g., remove the useState import if it becomes
unused) and ensure no remaining code references duplicateQueryUrl or
setDuplicateQueryUrl so the component compiles without unused-variable warnings.
---
Nitpick comments:
In `@src/components/basics/Modal/Modal.style.ts`:
- Around line 16-17: Revert removal of the default modal body padding in
modalBodyStyle and instead add a variant (e.g., variant key noPadding) that
removes padding; specifically, keep modalBodyStyle.base with the original
padding value, add a variants: { noPadding: { true: 'p-0' } } (or equivalent) so
the default layout is preserved, and update the duplicate-link modal to use
modalBodyStyle({ variant: 'noPadding' }) (or the project’s variant API) where
needed.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsx`:
- Around line 14-17: The DuplicateBanner component's warning div (in
DuplicateBanner.tsx) lacks an accessibility live region; update the outer
container element (the div wrapping SVGIcon and paragraph) to include
role="alert" (or aria-live="assertive") so screen readers immediately announce
the duplicate-overwrite warning; ensure any non-essential aria attributes aren't
conflicting and keep the SVGIcon and text unchanged.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.ts`:
- Around line 51-71: The effect in useCreateLinkError captures the onRetry
callback causing unnecessary re-runs and stale references; fix by creating a ref
(e.g., latestOnRetryRef) inside useCreateLinkError, update
latestOnRetryRef.current = onRetry in a small effect that depends on onRetry,
then remove onRetry from the dependency array of the main useEffect that shows
the toast and instead call latestOnRetryRef.current(payload) from the toast
onAction; keep existing refs like lastSubmitErrorRef and lastSubmitPayloadRef
and ensure hideToast and onRetry invocations use the ref to access the latest
callback.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsx`:
- Around line 44-46: In LinkThumbnailTitleSection.tsx the Image rendering (Image
src={previewImageUrl} alt="link thumbnail" fill) needs an object-fit style to
avoid distortion for varying aspect ratios; update the Image element to include
a className or style that applies object-cover (or object-contain if you prefer
letterboxing) so the thumbnail preserves its aspect ratio when using the fill
prop—locate the Image usage in LinkThumbnailTitleSection and add the appropriate
className="object-cover" or style={{ objectFit: 'cover' }} on that Image.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/apis/linkApi.tssrc/app/(dev)/link-api-demo/LinkApiDemo.tsxsrc/app/api/links/[id]/route.tssrc/app/api/links/duplicate/route.tssrc/app/api/links/meta-scrape/route.tssrc/app/api/links/route.tssrc/components/basics/Modal/Modal.style.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/AddLinkButton.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/AddLinkUrlInput.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useAddLinkForm.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useDuplicateCheck.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLinkModal.tsxsrc/components/layout/SideNavigation/components/MenuSection/MenuSection.tsxsrc/hooks/useCheckDuplicateLink.tssrc/hooks/useLinkMetaScrape.tssrc/types/api/linkApi.ts
💤 Files with no reviewable changes (1)
- src/components/layout/SideNavigation/components/MenuSection/AddLinkModal.tsx
src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useAddLinkForm.ts
Outdated
Show resolved
Hide resolved
src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useDuplicateCheck.ts
Show resolved
Hide resolved
src/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsx
Show resolved
Hide resolved
src/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsx
Outdated
Show resolved
Hide resolved
...omponents/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsx
Show resolved
Hide resolved
Bangdayeon
force-pushed
the
feat/#363-saved-link-modal
branch
from
abd57ab to
3ee4fe1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apis/linkApi.ts (1)
173-179:⚠️ Potential issue | 🟠 Major중복 체크 응답에서도
success를 함께 검증해 주세요.Line 177은
data만 확인해서,success=false응답이 200으로 내려올 때 실패를 정상으로 처리할 수 있습니다.🔧 제안 diff
export const checkDuplicateLink = async (url: string) => { const usp = new URLSearchParams({ url }); const body = await clientApiClient<DuplicateLinkApiResponse>( `/api/links/duplicate?${usp.toString()}` ); - if (!body?.data) { + if (!body?.data || !body.success) { throw new Error(body?.message ?? 'Failed to check duplicate'); } return body.data; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/apis/linkApi.ts` around lines 173 - 179, The current duplicate check only validates body?.data and can treat a 200 response with success: false as OK; update the validation after the clientApiClient<DuplicateLinkApiResponse>(...) call to also verify body?.success is true (in addition to body?.data) and throw an Error including body?.message when either check fails; look for the variable body and the generic type DuplicateLinkApiResponse to implement the combined success+data check.
♻️ Duplicate comments (1)
src/hooks/useLinkMetaScrape.ts (1)
112-120:⚠️ Potential issue | 🟠 Major비동기 응답에서 최신 dirty 상태를 보지 않아 사용자 입력을 덮어쓸 수 있습니다.
Line 112, Line 118이
.then()내부에서dirtyFields스냅샷을 참조하고 있어, 요청 중 사용자가 수정한 값을 auto-fill이 덮어쓸 수 있습니다. 도입한 ref(dirtyTitleRef,dirtyMemoRef)로 일관되게 체크해 주세요.🔧 제안 diff
- if (!dirtyFields.title && !skipAutoFillRef.current) { + if (!dirtyTitleRef.current && !skipAutoFillRef.current) { // skipAutoFill 조건 추가 setValue(titlePath, (data.title ?? '') as PathValue<T, typeof titlePath>, { shouldValidate: true, }); } - if (!dirtyFields.memo && !skipAutoFillRef.current) { + if (!dirtyMemoRef.current && !skipAutoFillRef.current) { // skipAutoFill 조건 추가 setValue(memoPath, (data.description ?? '') as PathValue<T, typeof memoPath>, { shouldValidate: true, }); }검증은 아래처럼 현재 파일에서 async 블록과 dirty 참조를 같이 확인하면 됩니다.
#!/bin/bash set -e target="$(fd '^useLinkMetaScrape\.ts$' -t f | head -n 1)" echo "target: $target" rg -n "mutateAsync|then\\(|dirtyFields\\.(title|memo)|dirty(Title|Memo)Ref\\.current" "$target"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLinkMetaScrape.ts` around lines 112 - 120, The async callback that sets title/memo uses a stale dirtyFields snapshot (inside the .then / mutateAsync continuation) and can overwrite user edits; change those checks to use the refs dirtyTitleRef.current and dirtyMemoRef.current instead of dirtyFields.title/memo, i.e. before calling setValue(titlePath, ...) or setValue(memoPath, ...) verify !skipAutoFillRef.current && !dirtyTitleRef.current / !dirtyMemoRef.current respectively, leaving shouldValidate: true and keeping the existing titlePath/memoPath/setValue calls intact.
🧹 Nitpick comments (1)
src/app/(dev)/link-api-demo/LinkApiDemo.tsx (1)
72-75: 중복 체크 요청 전에 URL 정규화(트림/빈값 가드) 추가를 권장합니다.공백 입력으로 인한 불필요한 요청/오류를 줄일 수 있습니다.
🔧 제안 diff
const handleCheckDuplicate = (e: React.FormEvent) => { e.preventDefault(); - checkDuplicate(duplicateUrl); + const normalizedUrl = duplicateUrl.trim(); + if (!normalizedUrl) return; + checkDuplicate(normalizedUrl); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(dev)/link-api-demo/LinkApiDemo.tsx around lines 72 - 75, The handleCheckDuplicate handler should trim the input and guard against empty strings before calling checkDuplicate: update handleCheckDuplicate to compute const normalized = duplicateUrl.trim(), return early (or set an error state / show a message) if normalized is empty, and only call checkDuplicate(normalized) when non-empty so you avoid unnecessary requests; reference the handleCheckDuplicate function and the checkDuplicate(duplicateUrl) call for the change.
🤖 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/components/basics/Modal/Modal.style.ts`:
- Line 17: Revert the shared Modal default padding by restoring the 'base' value
in Modal.style.ts to the original non-empty spacing (do not leave base: ''), and
instead remove padding only in the specific duplicate-link modal by overriding
the Modal styles there (e.g., via its component-specific className/prop or a
dedicated style key in DuplicateLinkModal). Locate the 'base' key in
Modal.style.ts to restore the default, and update the specific modal component
that needs no padding to explicitly override the Modal's padding rather than
changing the shared 'base'.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.ts`:
- Around line 63-69: In the onAction callback, immediately clear the
submit-error flag so the error UI is removed as soon as retry is clicked: after
verifying payload via lastSubmitPayloadRef.current and before calling
hideToast/to clear lastSubmitErrorRef, call the boolean state setter (e.g.,
setHasSubmitError(false)) or update the hasSubmitError ref to false so the error
flag is reset prior to invoking onRetry(payload); keep the existing
hideToast(toastId), lastSubmitErrorRef.current = null and onRetry(payload)
calls.
---
Outside diff comments:
In `@src/apis/linkApi.ts`:
- Around line 173-179: The current duplicate check only validates body?.data and
can treat a 200 response with success: false as OK; update the validation after
the clientApiClient<DuplicateLinkApiResponse>(...) call to also verify
body?.success is true (in addition to body?.data) and throw an Error including
body?.message when either check fails; look for the variable body and the
generic type DuplicateLinkApiResponse to implement the combined success+data
check.
---
Duplicate comments:
In `@src/hooks/useLinkMetaScrape.ts`:
- Around line 112-120: The async callback that sets title/memo uses a stale
dirtyFields snapshot (inside the .then / mutateAsync continuation) and can
overwrite user edits; change those checks to use the refs dirtyTitleRef.current
and dirtyMemoRef.current instead of dirtyFields.title/memo, i.e. before calling
setValue(titlePath, ...) or setValue(memoPath, ...) verify
!skipAutoFillRef.current && !dirtyTitleRef.current / !dirtyMemoRef.current
respectively, leaving shouldValidate: true and keeping the existing
titlePath/memoPath/setValue calls intact.
---
Nitpick comments:
In `@src/app/`(dev)/link-api-demo/LinkApiDemo.tsx:
- Around line 72-75: The handleCheckDuplicate handler should trim the input and
guard against empty strings before calling checkDuplicate: update
handleCheckDuplicate to compute const normalized = duplicateUrl.trim(), return
early (or set an error state / show a message) if normalized is empty, and only
call checkDuplicate(normalized) when non-empty so you avoid unnecessary
requests; reference the handleCheckDuplicate function and the
checkDuplicate(duplicateUrl) call for the change.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/apis/linkApi.tssrc/app/(dev)/link-api-demo/LinkApiDemo.tsxsrc/app/api/links/[id]/route.tssrc/app/api/links/duplicate/route.tssrc/app/api/links/meta-scrape/route.tssrc/app/api/links/route.tssrc/components/basics/Modal/Modal.style.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/AddLinkButton.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/AddLinkUrlInput.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useAddLinkForm.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useDuplicateCheck.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLinkModal.tsxsrc/components/layout/SideNavigation/components/MenuSection/MenuSection.tsxsrc/hooks/useCheckDuplicateLink.tssrc/hooks/useLinkMetaScrape.tssrc/types/api/linkApi.ts
✅ Files skipped from review due to trivial changes (1)
- src/components/layout/SideNavigation/components/MenuSection/AddLinkModal.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- src/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsx
- src/app/api/links/[id]/route.ts
- src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useDuplicateCheck.ts
- src/app/api/links/duplicate/route.ts
- src/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsx
- src/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsx
- src/app/api/links/route.ts
- src/components/layout/SideNavigation/components/MenuSection/MenuSection.tsx
src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.ts
Show resolved
Hide resolved
Bangdayeon
force-pushed
the
feat/#363-saved-link-modal
branch
from
3ee4fe1 to
7715f22
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useLinkMetaScrape.ts (1)
93-98:⚠️ Potential issue | 🟠 Major
skipAutoFill가 URL 변경 시 초기화 분기에는 반영되지 않습니다.이 분기에서는
skipAutoFill가true여도 title/memo를 비워서 값 소실이 발생할 수 있습니다. 같은 기준(dirty*Ref+skipAutoFillRef)으로 통일해 주세요.🛠️ 제안 수정
- if (!dirtyFields.title && getValues(titlePath)) { + if (!dirtyTitleRef.current && !skipAutoFillRef.current && getValues(titlePath)) { setValue(titlePath, '' as PathValue<T, typeof titlePath>, { shouldValidate: true }); } - if (!dirtyFields.memo && getValues(memoPath)) { + if (!dirtyMemoRef.current && !skipAutoFillRef.current && getValues(memoPath)) { setValue(memoPath, '' as PathValue<T, typeof memoPath>, { shouldValidate: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLinkMetaScrape.ts` around lines 93 - 98, The current clear-on-URL-change branches unconditionally clear title/memo when not dirty (dirtyFields.title / dirtyFields.memo), which discards values even if skipAutoFill (skipAutoFillRef) is true; update the conditions used before calling setValue(titlePath, ...) and setValue(memoPath, ...) to require both "not dirty" AND "not skipAutoFill" (mirror the same combined check used elsewhere with dirty*Ref + skipAutoFillRef), e.g. use the same predicate that checks dirtyTitleRef/dirtyMemoRef and skipAutoFillRef together with getValues(...) before calling setValue so values are preserved when skipAutoFillRef is set.
🧹 Nitpick comments (3)
src/hooks/useCheckDuplicateLink.ts (1)
4-7: 응답 타입을 API 타입과 직접 연결하는 편이 안전합니다.
DuplicateResult를 별도 인터페이스로 중복 선언하면 API 스키마 변경 시 드리프트가 생길 수 있습니다.DuplicateLinkApiResponse['data']기반으로 묶어두는 쪽을 권장합니다.제안 수정안
import { checkDuplicateLink } from '@/apis/linkApi'; +import type { DuplicateLinkApiResponse } from '@/types/api/linkApi'; import { useMutation } from '@tanstack/react-query'; -interface DuplicateResult { - exists: boolean; - linkId?: number; -} +type DuplicateResult = NonNullable<DuplicateLinkApiResponse['data']>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCheckDuplicateLink.ts` around lines 4 - 7, Replace the locally declared interface DuplicateResult with a type alias that references the API response shape (use DuplicateLinkApiResponse['data']) so the hook's return/response types stay in sync with the backend; update any usages in useCheckDuplicateLink (and exports) to use that alias instead of DuplicateResult and remove the redundant interface declaration.src/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsx (1)
7-18: 중복 안내 배너를 라이브 리전으로 노출해 주세요.현재 배너가 동적으로 나타나도 스크린리더가 즉시 읽지 못할 수 있습니다.
role="status"와aria-live="polite"를 추가하면 상태 변화 전달이 더 안정적입니다.♿ 제안 수정
- <div className="m-6"> + <div className="m-6" role="status" aria-live="polite">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsx` around lines 7 - 18, The DuplicateBanner component's container div (the one with className "m-6") should announce its appearance to assistive tech; add accessibility live-region attributes role="status" and aria-live="polite" to that root element so screen readers reliably read the banner when it appears. Update the root div in DuplicateBanner.tsx (the <div className="m-6"> element) to include these attributes and ensure they are present only when the banner is rendered.src/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsx (1)
60-64: 중복 데이터 프리필은 사용자 수정값을 덮어쓰지 않게 가드해 주세요.
duplicateLinkData가 다시 들어오는 시점에setValue가 재실행되면 사용자가 수정한 title/memo가 복원될 수 있습니다. dirty 상태일 때는 프리필을 건너뛰는 방어가 있으면 안전합니다.♻️ 제안 수정
useEffect(() => { if (!isDuplicate || !duplicateLinkData || metaLoading) return; - form.setValue('title', duplicateLinkData.title, { shouldValidate: true }); - form.setValue('memo', duplicateLinkData.memo ?? '', { shouldValidate: true }); + const { title, memo } = form.formState.dirtyFields; + if (title || memo) return; + form.setValue('title', duplicateLinkData.title, { + shouldValidate: true, + shouldDirty: false, + }); + form.setValue('memo', duplicateLinkData.memo ?? '', { + shouldValidate: true, + shouldDirty: false, + }); }, [isDuplicate, duplicateLinkData, metaLoading, form]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsx` around lines 60 - 64, The current useEffect pre-fills title/memo unconditionally when duplicateLinkData arrives, which can overwrite user edits; modify the effect in the useEffect that references isDuplicate, duplicateLinkData, metaLoading and form so it first checks the form's dirty state and only calls form.setValue for fields that are not dirty (e.g., use form.formState.isDirty or form.formState.dirtyFields to guard per-field), skipping prefill for title/memo if the user has already edited them; keep the existing early return for !isDuplicate || !duplicateLinkData || metaLoading.
🤖 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/apis/linkApi.ts`:
- Around line 74-75: The clientApiClient calls (e.g., the call returning
LinkListApiResponse via clientApiClient(`/api/links${buildQuery(params)}`) and
similar calls around the indicated locations) currently lack the 15_000ms
timeout that existed on the safeFetch+withAuth path; update clientApiClient to
accept a timeout or AbortSignal option and implement request cancellation (e.g.,
using fetch with an AbortController or a timeout wrapper inside
clientApiClient), then update each call site (the clientApiClient invocations
for LinkListApiResponse and the other similar calls) to pass the 15000ms timeout
or an AbortSignal so requests are aborted after 15s and the UI won't hang.
In `@src/hooks/useCheckDuplicateLink.ts`:
- Around line 11-13: The mutationFn in useCheckDuplicateLink calls
checkDuplicateLink(url) without guarding for empty or whitespace-only input,
causing unnecessary API calls and error toasts; update mutationFn (inside
useCheckDuplicateLink) to trim the incoming url, check if the trimmed string is
non-empty, and if empty return an appropriate resolved value (e.g., null/false
or a skipped result) instead of calling checkDuplicateLink, otherwise call
checkDuplicateLink with the trimmed URL; ensure you reference mutationFn and
checkDuplicateLink when making the change.
---
Outside diff comments:
In `@src/hooks/useLinkMetaScrape.ts`:
- Around line 93-98: The current clear-on-URL-change branches unconditionally
clear title/memo when not dirty (dirtyFields.title / dirtyFields.memo), which
discards values even if skipAutoFill (skipAutoFillRef) is true; update the
conditions used before calling setValue(titlePath, ...) and setValue(memoPath,
...) to require both "not dirty" AND "not skipAutoFill" (mirror the same
combined check used elsewhere with dirty*Ref + skipAutoFillRef), e.g. use the
same predicate that checks dirtyTitleRef/dirtyMemoRef and skipAutoFillRef
together with getValues(...) before calling setValue so values are preserved
when skipAutoFillRef is set.
---
Nitpick comments:
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsx`:
- Around line 7-18: The DuplicateBanner component's container div (the one with
className "m-6") should announce its appearance to assistive tech; add
accessibility live-region attributes role="status" and aria-live="polite" to
that root element so screen readers reliably read the banner when it appears.
Update the root div in DuplicateBanner.tsx (the <div className="m-6"> element)
to include these attributes and ensure they are present only when the banner is
rendered.
In
`@src/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsx`:
- Around line 60-64: The current useEffect pre-fills title/memo unconditionally
when duplicateLinkData arrives, which can overwrite user edits; modify the
effect in the useEffect that references isDuplicate, duplicateLinkData,
metaLoading and form so it first checks the form's dirty state and only calls
form.setValue for fields that are not dirty (e.g., use form.formState.isDirty or
form.formState.dirtyFields to guard per-field), skipping prefill for title/memo
if the user has already edited them; keep the existing early return for
!isDuplicate || !duplicateLinkData || metaLoading.
In `@src/hooks/useCheckDuplicateLink.ts`:
- Around line 4-7: Replace the locally declared interface DuplicateResult with a
type alias that references the API response shape (use
DuplicateLinkApiResponse['data']) so the hook's return/response types stay in
sync with the backend; update any usages in useCheckDuplicateLink (and exports)
to use that alias instead of DuplicateResult and remove the redundant interface
declaration.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/apis/linkApi.tssrc/app/(dev)/link-api-demo/LinkApiDemo.tsxsrc/app/api/links/[id]/route.tssrc/app/api/links/duplicate/route.tssrc/app/api/links/meta-scrape/route.tssrc/app/api/links/route.tssrc/components/basics/Modal/Modal.style.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/AddLinkButton.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/AddLinkUrlInput.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/DuplicateBanner.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/LinkThumbnailTitleSection.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useAddLinkForm.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useDuplicateCheck.tssrc/components/layout/SideNavigation/components/MenuSection/AddLink/index.tsxsrc/components/layout/SideNavigation/components/MenuSection/AddLinkModal.tsxsrc/components/layout/SideNavigation/components/MenuSection/MenuSection.tsxsrc/hooks/useCheckDuplicateLink.tssrc/hooks/useLinkMetaScrape.tssrc/types/api/linkApi.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/components/layout/SideNavigation/components/MenuSection/AddLink/AddLinkButton.tsx
- src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useAddLinkForm.ts
- src/app/(dev)/link-api-demo/LinkApiDemo.tsx
- src/app/api/links/meta-scrape/route.ts
- src/app/api/links/[id]/route.ts
- src/app/api/links/route.ts
- src/types/api/linkApi.ts
- src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useCreateLinkError.ts
- src/components/layout/SideNavigation/components/MenuSection/AddLink/hooks/useDuplicateCheck.ts
- src/app/api/links/duplicate/route.ts
Bangdayeon
force-pushed
the
feat/#363-saved-link-modal
branch
from
7715f22 to
da0bc5b
Compare
Seong-Myeong
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
관련 이슈
PR 설명