-
Notifications
You must be signed in to change notification settings - Fork 5
Refactor(client): use-image-upload 로직 추상화 #502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,4 +43,7 @@ storybook-static | |
|
|
||
| # cursor | ||
| .cursorrules | ||
| .cursorignore | ||
| .cursorignore | ||
|
|
||
| #codex | ||
| AGENTS.md | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,18 +21,14 @@ import { | |
| COMMUNITY_MUTATION_OPTIONS, | ||
| COMMUNITY_QUERY_OPTIONS, | ||
| } from '@shared/api/domain/community/queries'; | ||
| import { | ||
| MUTATION_QUERY_OPTIONS, | ||
| uploadImageToS3, | ||
| } from '@shared/api/domain/queries'; | ||
| import { COMMUNITY_QUERY_KEY } from '@shared/api/keys/query-key'; | ||
| import { | ||
| LIMIT_LONG_TEXT, | ||
| LIMIT_SHORT_TEXT, | ||
| } from '@shared/constants/text-limits'; | ||
| import { useImageUpload } from '@shared/hooks/use-image-upload'; | ||
| import { useLimitedInput } from '@shared/hooks/use-limited-input'; | ||
| import { routePath } from '@shared/router/path'; | ||
| import { extractS3Urls } from '@shared/utils/utils'; | ||
|
|
||
| import * as styles from './community-edit.css'; | ||
|
|
||
|
|
@@ -59,9 +55,7 @@ const CommunityEdit = () => { | |
| }, | ||
| }); | ||
|
|
||
| const { mutate: postImageUploadMutate } = useMutation({ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mutation에 대한 코드를 삭제하셨으니 queries.ts에서 MUTATION_QUERY_OPTIONS.POST_IMAGE()도 같이 삭제해도 괜찮을것 같아요!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 뮤테이션을 제거하지 않은 이유는 인프라 레이어라서 제거하지 않았던건데 옵션 블록은 제거하거나 주석처리하거나 하는게 좋겠네요 일단 제거해둘게요 👍
useImageUpload 내부에도 로딩 및 에러 처리를 위한 상태 반환 값이 존재해서 우려하는 부분은 대체 될 것 같아요 export const useImageUpload = () => {
const [isUploading, setIsUploading] = useState(false);
const [isError, setError] = useState(false);
const uploadImageFiles = async (files: File[]) => {
setIsUploading(true);
setError(false);
try {
return await uploadImageFilesInternal(files);
} catch (e) {
setError(true);
throw e;
} finally {
setIsUploading(false);
}
};
return { uploadImageFiles, isUploading, isError };
};
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아 이해했어요 |
||
| ...MUTATION_QUERY_OPTIONS.POST_IMAGE(), | ||
| }); | ||
| const { uploadImageFiles } = useImageUpload(); | ||
|
|
||
| if (!feedDetailData) { | ||
| throw new Error( | ||
|
|
@@ -90,39 +84,18 @@ const CommunityEdit = () => { | |
| ); | ||
| const { isErrorState } = useLimitedInput(LIMIT_SHORT_TEXT, title.length); | ||
|
|
||
| const handleUploadNewImages = async () => { | ||
| if (newImages.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const data = await new Promise<{ presignedUrls: string[] }>( | ||
| (resolve, reject) => | ||
| postImageUploadMutate( | ||
| newImages.map((item) => item.file.type), | ||
| { onSuccess: resolve, onError: reject }, | ||
| ), | ||
| ); | ||
|
|
||
| await Promise.all( | ||
| data.presignedUrls.map((url, idx) => | ||
| uploadImageToS3(url, newImages[idx].file), | ||
| ), | ||
| const handlePutFeed = async () => { | ||
| const uploadedUrls = await uploadImageFiles( | ||
| newImages.map((item) => item.file), | ||
| ); | ||
|
|
||
| const uploadedUrls = extractS3Urls(data.presignedUrls).map((url, idx) => ({ | ||
| const newUploadedImages = uploadedUrls.map((url, idx) => ({ | ||
| imageUrl: url, | ||
| sequence: updatedImages.length + idx, | ||
| })); | ||
|
|
||
| setUpdatedImages((prev) => [...prev, ...uploadedUrls]); | ||
| setUpdatedImages((prev) => [...prev, ...newUploadedImages]); | ||
| setNewImages([]); | ||
|
|
||
| return uploadedUrls; | ||
| }; | ||
|
|
||
| const handlePutFeed = async () => { | ||
| const newUploadedImages = await handleUploadNewImages(); | ||
|
|
||
| mutate({ | ||
| body: { | ||
| newTitle: title, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import { useState } from 'react'; | ||
|
|
||
| import { postImage, uploadImageToS3 } from '@shared/api/domain/queries'; | ||
| import { extractS3Urls } from '@shared/utils/utils'; | ||
|
|
||
| const uploadImageFilesInternal = async (files: File[]): Promise<string[]> => { | ||
| if (files.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const { presignedUrls } = await postImage(files.map((file) => file.type)); | ||
|
|
||
| await Promise.all( | ||
| files.map((file, idx) => uploadImageToS3(presignedUrls[idx], file)), | ||
| ); | ||
|
|
||
| return extractS3Urls(presignedUrls); | ||
| }; | ||
|
|
||
| export const useImageUpload = () => { | ||
| const [isLoading, setIsLoading] = useState(false); | ||
| const [isError, setError] = useState(false); | ||
|
|
||
| const uploadImageFiles = async (files: File[]) => { | ||
| setIsLoading(true); | ||
| setError(false); | ||
|
|
||
| try { | ||
| return await uploadImageFilesInternal(files); | ||
| } catch (e) { | ||
| setError(true); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 에러가 발생했을 때 setError 상태가 변경되지만 이후 에러에 대한 대응은 없어보이는것 같아요! 그래서 이미지 업로드 실 패 시 토스트 메시지를 띄우는 방안도 괜찮아 보이네요!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. toast 메시지와 같은 에러 핸들링을 하기 위해서 상태를 설정하고 그걸 return 하고있는거에용 여기선 상태만 선언하고 이 함수에서는 에러 상태만 반환해요 그래야 사용처별로 달라지는 ux ex) 토스트 메시지 등등을 대응할 수 있어요 loading 값도 마찬가지 이유로 반환하는거구용
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 훅에서 상태만 보고 사용처에서 UX를 결정하는 의도 확인했습니당
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
이해했습니다. 에러에 따른 UI는 사용처에서 관리하도록 구성하신거군요 👍
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@hansoojeongsj 에러처리에 관련된건 별도의 스코프에서 따로 처리하려고해요 ㅎㅎ 센트리 도입부터 시작해서 한번에 진행하려구용 |
||
| throw e; | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| return { uploadImageFiles, isLoading, isError }; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지욱님의 AGENTS.md 탐나네요.. 공유해주세요~