[문지영] sprint7#228
Hidden character warning
Conversation
addiescode-sj
left a comment
There was a problem hiding this comment.
수고하셨습니다!
지영님 점점 코드 퀄리티가 좋아지는게 보이네요 :) 👍
제가 드리는 리팩토링 포인트 보시고, 더 깊이 공부해보시면 도움 되실거예요! :)
주요 리뷰 포인트
- api 연결 시점 생각해보며 컴포넌트 구조 바꾸기
- 더 많은 요구사항을 품게 될 경우를 대비해 합성 컴포넌트 기반으로 설계 바꿔보기
- useEffect 내부에서 사용되는 함수 메모리 누수 줄이는 방법
- 리턴문 가독성 높이기
- 공통적으로 반복 처리되는 패턴을 묶어 재사용 가능한 함수로 분리하기 (커스텀 훅)
| function CommentEditList({ onEditClick, commentId, onDeleteClick }) { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
|
|
||
| const handleEdit = () => { | ||
| onEditClick(commentId); | ||
| setIsOpen(false); | ||
| }; | ||
|
|
||
| const handleDelete = () => { | ||
| onDeleteClick(commentId); | ||
| setIsOpen(false); | ||
| }; |
There was a problem hiding this comment.
지금은 comments 배열을 부모 컴포넌트에서 state로 관리하고있고, 수정/삭제 될때마다 해당 배열을 업데이트해야하다보니 이런 구조를 쓰는게 맞지만, 추후에 관심사 분리 관점에서 api 연결을 통해 수정/삭제할때도 여전히 이 구조를 유지하는지 맞을지에 대해서는 고민해볼 필요가 있겠네요! :)
해당 컴포넌트에서 담당하는 일을
- 수정 삭제 옵션을 isOpen state에 따라 보여주는것
- 수정, 삭제 완료 시 기존 state 업데이트와 함께 UI 갱신
이렇게 api를 연결하는 시점에선 두가지를 담당할수있도록 리팩토링해봅시다! :)
There was a problem hiding this comment.
현재 이 컴포넌트가 프로그램이 복잡해지고 규모가 커지면서 더 많은 요구사항을 품게 된다면 어떻게 될까요?
props가 늘어날테고, 분기도 늘어날테고, 전체적인 복잡도가 크게 증가하겠죠?
하나의 컴포넌트에서 너무 많은 요구사항을 처리하려하면 이런 일이 쉽게 발생할 수 있습니다.
React는 상속보다 조합에 특화되어있습니다.
컴포넌트를 기능과 역할에 맞게 잘게 쪼게 합성(조합)하는 방식으로 바꿔보는건 어떨까요?
예시를 보여드릴게요!
There was a problem hiding this comment.
- 합성 컴포넌트 패턴을 사용한 예시
import { useState } from "react";
import styled from "styled-components";
import searchIcon from "../../assets/images/icons/ic_search.svg";
import closeEye from "../../assets/images/icons/ic_visibility_off.png";
import openEye from "../../assets/images/icons/ic_visibility_on.png";
import { applyFontStyles } from "../../styles/mixins";
import { ColorTypes, FontTypes } from "../../styles/theme";
// Base Input Container
const InputContainer = ({ children, label, error, id }) => (
<StyledContainer>
{label && <label htmlFor={id}>{label}</label>}
{children}
{error && <ErrorMessage>{error}</ErrorMessage>}
</StyledContainer>
);
// Main Input Component
function Input({ children, label, error, id, ...props }) {
return (
<InputContainer label={label} error={error} id={id} {...props}>
{children}
</InputContainer>
);
}
// Text Input
Input.Text = ({
id,
type = "text",
placeholder,
value,
onChange,
onBlur,
autoComplete,
...props
}) => (
<InputWrapper>
<input
id={id}
type={type}
placeholder={placeholder}
value={value}
onChange={onChange}
onBlur={onBlur}
autoComplete={autoComplete}
{...props}
/>
</InputWrapper>
);
// Password Input
Input.Password = ({ id, placeholder, value, onChange, onBlur, ...props }) => {
const [isPwVisible, setIsPwVisible] = useState(false);
return (
<InputWrapper $hasPasswordToggle={true}>
<input
id={id}
type={isPwVisible ? "text" : "password"}
placeholder={placeholder}
value={value}
onChange={onChange}
onBlur={onBlur}
autoComplete="current-password"
{...props}
/>
<EyeButton
type="button"
onClick={() => setIsPwVisible((prev) => !prev)}
tabIndex={-1}
>
<img
src={isPwVisible ? openEye : closeEye}
alt={isPwVisible ? "비밀번호 숨기기" : "비밀번호 보기"}
/>
</EyeButton>
</InputWrapper>
);
};
// Email Input
Input.Email = ({ id, placeholder, value, onChange, onBlur, ...props }) => (
<Input.Text
id={id}
type="email"
placeholder={placeholder}
value={value}
onChange={onChange}
onBlur={onBlur}
autoComplete="username"
{...props}
/>
);
// Nickname Input
Input.Nickname = ({ id, placeholder, value, onChange, onBlur, ...props }) => (
<Input.Text
id={id}
type="text"
placeholder={placeholder}
value={value}
onChange={onChange}
onBlur={onBlur}
autoComplete="nickname"
{...props}
/>
);
// Search Input
Input.Search = ({
id,
placeholder,
value,
onChange,
onBlur,
onSearch,
...props
}) => (
<InputWrapper $hasSearchIcon={true}>
<input
id={id}
type="text"
placeholder={placeholder}
value={value}
onChange={onChange}
onBlur={onBlur}
{...props}
/>
<SearchButton type="button" onClick={onSearch} tabIndex={-1}>
<img src={searchIcon} alt="검색" />
</SearchButton>
</InputWrapper>
);
// Number Input
Input.Number = ({
id,
placeholder,
value,
onChange,
onBlur,
min,
max,
step,
...props
}) => (
<Input.Text
id={id}
type="number"
placeholder={placeholder}
value={value}
onChange={onChange}
onBlur={onBlur}
min={min}
max={max}
step={step}
{...props}
/>
);
// TextArea Input
Input.TextArea = ({
id,
placeholder,
value,
onChange,
rows = 10,
...props
}) => (
<InputWrapper>
<textarea
id={id}
placeholder={placeholder}
value={value}
onChange={onChange}
rows={rows}
{...props}
/>
</InputWrapper>
);
// Error Message Component
Input.ErrorMessage = ({ children }) => <StyledError>{children}</StyledError>;
// Styled Components
const StyledContainer = styled.div`
display: flex;
flex-direction: column;
gap: 8px;
width: 100%;
`;
const InputWrapper = styled.div`
position: relative;
width: 100%;
input {
width: 100%;
padding-right: ${({ $hasPasswordToggle, $hasSearchIcon }) =>
$hasPasswordToggle ? "30px" : $hasSearchIcon ? "40px" : "0"};
border: 1px solid
${({ $hasError, theme }) =>
$hasError
? theme.colors[ColorTypes.ERROR]
: theme.colors[ColorTypes.SECONDARY_GRAY_100]};
}
textarea {
width: 100%;
border: 1px solid
${({ theme }) => theme.colors[ColorTypes.SECONDARY_GRAY_100]};
resize: vertical;
}
`;
const EyeButton = styled.button`
position: absolute;
right: 10px;
top: 50%;
transform: translateY(-50%);
background: none;
border: none;
padding: 0;
cursor: pointer;
display: flex;
align-items: center;
img {
width: 20px;
height: 18px;
margin-right: 18px;
}
`;
const SearchButton = styled.button`
position: absolute;
right: 10px;
top: 50%;
transform: translateY(-50%);
background: none;
border: none;
padding: 0;
cursor: pointer;
display: flex;
align-items: center;
img {
width: 16px;
height: 16px;
}
`;
const StyledError = styled.p`
${applyFontStyles(FontTypes.SEMIBOLD14, ColorTypes.ERROR)}
`;
export default Input;- 실제 사용
// 커스텀 에러 메시지
<Input label="나이" id="age">
<Input.Number
placeholder="나이를 입력하세요"
value={age}
onChange={setAge}
min={1}
max={120}
/>
<Input.ErrorMessage>
나이는 1세 이상 120세 이하여야 합니다.
</Input.ErrorMessage>
</Input>| const fetchProductDetail = async () => { | ||
| const res = await getProductDetail(productId); | ||
| setProduct(res); | ||
| setIsTags(res.tags.length > 0); | ||
| }; |
There was a problem hiding this comment.
React 공식문서상으로는 useEffect 내부에서 사용하는 함수는 의존성에 추가하고, useCallback으로 감싸서 사용할것을 권고하고있습니다 :)
이유는, useEffect가 실행될때마다 fetchProductDetail 함수의 인스턴스가 새로 생성되는데 함수 자체는 생성될때마다 동일한 로직이지만 메모리상은 새로운 객체로 취급되고, 또한 비동기 함수가 실행 중일때 컴포넌트가 언마운트되더라도 함수가 계속 실행 될 가능성이 있어, 메모리 누수 가능성이 있기 때문입니다.
따라서, useCallback을 사용해 의존성이 변경되지않는 한 동일한 함수 참조를 유지하며 AbortController를 사용해 cleanup까지 고려해주시면 좋습니다.
const fetchProductDetail = useCallback(async () => {
const abortController = new AbortController();
try {
const res = await getProductDetail(productId, { signal: abortController.signal });
setProduct(res);
setIsTags(res.tags.length > 0);
} catch (error) {
if (error.name !== 'AbortError') {
console.error('Failed to fetch product:', error);
}
}
return () => abortController.abort();
}, [productId, getProductDetail]);
useEffect(() => {
const cleanup = fetchProductDetail();
return cleanup;
}, [fetchProductDetail]);이런 작은 디테일의 변화들도 놓치지않고 신경써서 코딩한다면 최적화 경험을 늘리면서도 좋은 개발 습관을 들일 수 있을거예요! :)
| <div> | ||
| <StyledOwnerNickname>{product?.ownerNickname}</StyledOwnerNickname> | ||
| <StyledProductCreatedAt> | ||
| {product?.createdAt.split('T')[0].split('-').join('. ')} |
There was a problem hiding this comment.
리턴문 안쪽은 최대한 가독성을 좋게 유지하는 방향으로 습관을 들여볼까요?
| const handleEmailChange = (e) => { | ||
| const value = e.target.value; | ||
| setEmail(value); | ||
| setErrors((prev) => ({ | ||
| ...prev, | ||
| email: validateField('email', value, { email: value, password }, 'login'), | ||
| })); | ||
| }; | ||
|
|
||
| const handlePasswordChange = (e) => { | ||
| const value = e.target.value; | ||
| setPassword(value); | ||
| setErrors((prev) => ({ | ||
| ...prev, | ||
| password: validateField('password', value, { email, password: value }, 'login'), | ||
| })); | ||
| }; | ||
|
|
||
| const handleEmailBlur = (e) => { | ||
| const value = e.target.value; | ||
| setErrors((prev) => ({ | ||
| ...prev, | ||
| email: validateField('email', value, { email: value, password }, 'login'), | ||
| })); | ||
| }; | ||
|
|
||
| const handlePasswordBlur = (e) => { | ||
| const value = e.target.value; | ||
| setErrors((prev) => ({ | ||
| ...prev, | ||
| password: validateField('password', value, { email, password: value }, 'login'), | ||
| })); | ||
| }; |
There was a problem hiding this comment.
핸들러에서 공통적으로 처리되는 패턴이 반복되는것같은데,
코드 중복을 줄이고 간소화하기위해 기존에 쓰고있던 커스텀 훅에 필드 변경이나 블러 이벤트를 처리하는 함수들을 재사용성있게 만들어 추가해보는건 어떨까요?
// 필드 변경 핸들러 생성 함수
const createFieldHandler = useCallback(
(fieldName) => {
return (e) => {
const value = e.target.value;
const newValues = { ...values, [fieldName]: value };
setValues(newValues);
setErrors((prev) => ({
...prev,
[fieldName]: validateField(fieldName, value, newValues, mode),
}));
};
},
[values, mode]
);
// 필드 블러 핸들러 생성 함수
const createBlurHandler = useCallback(
(fieldName) => {
return (e) => {
const value = e.target.value;
setErrors((prev) => ({
...prev,
[fieldName]: validateField(fieldName, value, values, mode),
}));
};
},
[values, mode]
);
// 폼 제출 핸들러 생성 함수
const createSubmitHandler = useCallback(
(onSubmit) => {
return (e) => {
e.preventDefault();
const validationResult = validate(values);
if (Object.keys(validationResult).length > 0) return;
onSubmit(values);
};
},
[values, validate]
);
// 필드별 props 생성 함수
const getFieldProps = useCallback(
(fieldName) => {
return {
value: values[fieldName] || "",
onChange: createFieldHandler(fieldName),
onBlur: createBlurHandler(fieldName),
error: errors[fieldName],
};
},
[values, errors, createFieldHandler, createBlurHandler]
);
// 폼 상태 확인 함수들
const hasError = Object.values(errors).some((v) => !!v);
const isFilled = Object.keys(values).every(
(key) => values[key] && values[key].trim()
);
const isFormValid = !hasError && isFilled;- 실제 사용 예시
...
<InputField
label="이메일"
type="email"
placeholder="이메일을 입력해주세요"
isTextArea={false}
{...getFieldProps("email")}
/>
<InputField
label="비밀번호"
type="password"
placeholder="비밀번호를 입력해주세요"
isTextArea={false}
{...getFieldProps("password")}
/>
...이런식으로 기존 핸들러 함수를 통해 개별 필드별 props를 생성하는 함수를 만들고, createSubmitHandler() 함수로 폼 제출 로직을 관리하는 함수를 만들면 모든 폼에서 동일한 패턴을 사용해 일관성을 높여줄수도있고, 새로운 인풋 필드를 추가한다고해도 getFieldProps('fieldName')만 호출하면 되니까 확장성 측면에서도 유리해질 수 있겠죠? :)
There was a problem hiding this comment.
해당 페이지에서도 LoginPage에서 드렸던 코멘트 보시고 리팩토링 참고해보세요! :)
질문에 대한 답변
ProductInfo 컴포넌트가 제일 복잡해보이네요!
|
요구사항
기본
상품 상세
상품 상세 페이지 주소는 "/items/{productId}" 입니다.
response 로 받은 아래의 데이터로 화면을 구현합니다.
=> favoriteCount : 하트 개수
=> images : 상품 이미지
=> tags : 상품태그
=> name : 상품 이름
=> description : 상품 설명
목록으로 돌아가기 버튼을 클릭하면 중고마켓 페이지 주소인 "/items" 으로 이동합니다
상품 문의 댓글
문의하기에 내용을 입력하면 등록 버튼의 색상은 "3692FF"로 변합니다.
response 로 받은 아래의 데이터로 화면을 구현합니다
=> image : 작성자 이미지
=> nickname : 작성자 닉네임
=> content : 작성자가 남긴 문구
=> description : 상품 설명
=> updatedAt : 문의글 마지막 업데이트 시간
심화
주요 변경사항
스크린샷
멘토에게