Skip to content
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

[4주차] 주효정 미션 제출합니다. #1

Closed
wants to merge 22 commits into from

Conversation

jhj2713
Copy link
Member

@jhj2713 jhj2713 commented May 5, 2022

안녕하세요. 15기 주효정입니다! 오랜만에 프론트 과제를 해서 그런지 약간 낯설면서도 즐겁게 한 것 같아요. UI를 어떻게 구성할까 하다가 디자인 쪽으로 소질이 없는 편이라(디자이너분들 존경합니다) 프짱님의 결과 화면이랑 거의 똑같이 구현해봤습니다..:sweat_smile:

지난번에 발표 준비하면서 오류들을 정리했었는데, 다음에 또 같은 오류 겪으면 찾아보기 편하고 좋더라구요. 그래서 이번에도 과제하면서 겪었던 오류들이랑 고민했던 부분들을 간단하게 정리해봤습니다. 혹시 제가 잘못 알고있는 부분이 있거나 더 좋은 해결책 있으면 알려주시면 감사하겠습니다.


배포 링크

https://react-messenger-15th-7cq31ga7x-jhj2713.vercel.app/


마주했던 오류들

1. typescript 적용 오류

React 18 버전이 업데이트 되면서 typescript 적용할때 오류가 나더라구요.

Untitled

시작부터 굉장히 당황스러웠습니다:cry: 그래도 오류가 비교적 명확하게 나와서 null 형식을 체크만 해주면 되겠구나 싶었습니다.

const rootElement = document.getElementById("root");
if (!rootElement) throw new Error("Failed to find the root element");
const root = ReactDOM.createRoot(rootElement);

아래 링크 참조해서 조건문을 추가해주니까 바로 해결됐습니다.

TypeScript에 React18 적용하기



2. ref 적용 오류

MessageChat.tsx 에서 메시지가 추가될 경우 스크롤바를 아래로 내리는 작동을 추가하기 위해서 ref를 사용했습니다.

const messageWrapperRef = useRef<HTMLElement>();

그런데 useRef 를 추가해서 scroll 작업을 처리하려고 할때 오류가 났습니다.

Untitled (1)

엄청 긴 오류문이 나왔는데 핵심은 useRef를 선언하면서 값을 넣어주지 않아 초기값이 undefined가 되었기 때문에 오류가 발생했다는 말이었습니다.

const messageWrapperRef = useRef<HTMLElement>(null);

ref의 초기화값을 null로 바꿔주니까 해결됐습니다.

Typescript React에서 useRef의 3가지 정의와 각각의 적절한 사용법



3. target이 innerText를 찾지 못하는 오류

이모티콘을 전송하기 위해서 해당 태그에 있는 innerText를 가져오는 작업이 필요했습니다. 그런데 평소랑 같이 innerText를 불러오는 코드를 짜니까 오류가 나더라구요.

Untitled (2)

e.target을 출력해봐도 분명히 innerText가 있는데 왜 안되는가 한참 고민했습니다...

그런데 e.target의 type을 출력해보니까 그냥 object더라구요. 그래서 typescript가 target에 innerText가 있다는 것을 인식 못하고 오류를 출력하는 것 같았습니다.

Untitled (3)

e.target이 HTMLElement라는 명시적 선언을 하고 나니까 오류가 사라졌습니다.

참조


전부 다 typescript로 발생한 오류더라구요... typescript 너무 까탈스러운 친구네요:joy:

고민했던 부분

1. MessageBalloon 컴포넌트 분리

상대방 메시지와 내 메시지의 ui가 조금 다르다보니 따로 컴포넌트를 만들어야 하나 마지막까지 고민했습니다... 그래도 중복된 부분이 많을 것 같아 하나로 묶어서 MessageBalloon.tsx에 선언했습니다. (그런데 조건문이 많아서 너무 복잡해져서 여전히 마음에 쏙 들지는 않네요:disappointed_relieved:)


2. Alert 창의 Enter 작동

window 기본 alert 창을 싫어해서 간단하게 모달창을 구현했습니다. 엔터로 alert창을 닫는 부분도 해보려 했는데 InputMessageForm.tsx의 form이랑 연결돼서 잘 안되더라구요... 다음에 다시 도전해봐야겠습니다. 좋은 해결책을 아시는 분은 공유해주시면 감사하겠습니다:thumbsup:


@codeKiuk
Copy link

codeKiuk commented May 5, 2022

안녕하세요 CEOS 14기 프론트엔드였던 양기욱이라고 합니다다다
멘토긴 멘토인데 코드리뷰를 지금까지 못 해서 너무 죄송해요ㅜㅜ 세오스 활동은 재밌게 하고 계신지요!!
지난 과제 코드들 보니 다들 너모 잘하시는 거 아닙니까!!
아무튼.. 너무 반갑구요,,, 과제하시느라 너무 고생하셨습니다!!!

  • useRef type관련 글 정말 큰 도움 됐어요! 앞으로 라이브러리들 타입 정의한 코드들 자세히 볼 동기가 생겼습니다,, 굳!

  • Context 에서 쓰는 interface들은 한 군데 모아주는 게 좋아보입니다.

현재 모든 Interface들이 한 파일에 있어서 음.. interface 디렉토리 아래에 index.ts로 다 모아주는 게 어떨까 생각이 드네요
Context 파일에서 타입을 보려면 너무 많이 이동해야하는 것 같아유
아래처럼 인터페이스들을 하나의 디렉토리에 모아주고, context마다 분리해서 관리한다든지? 하면 좋을 것 같네요

+--Interfaces
|  +--index
|  +--chat
|  +--user
	
  • React Context 를 쓸 때 왜 resize context, user context, messages context 등 이렇게 데이터마다 Context를 구분해서 만들까요?

만약 아래처럼 하나의 global context를 만들고, 해당 context에 global state를 담는 객체를 넣고 전역 상태를 핸들링한다면 어떻게 될까요?

const globalContext = createContext({
	messages: IMessage[],
	user: IUser,
	resize: boolean
})

이렇게 되면 user가 바뀔 때 user를 구독하고 있지 않고 resize만 구독하는 컴포넌트도 리렌더된다고 알고 있습니다.
저에게 도움이 많이 된 글을 첨부해요!
https://ui.toast.com/weekly-pick/ko_20200616#context-api를-사용하는-것은-어떨까

위에서 말한 문제(하나의 context로 전역 상태 핸들링) 를 해결하는 건 context를 만들 때 관련없는 데이터 셋마다 context를 나눠서 만들면 되죠!
그럼에도 불구하고,, 이렇게 해도 해결하기 힘든 문제가 있는데 이에 대한 설명은 위 글에서 잘 나와 있고,

  • 저는 custom hook과 context를 같이 썼을 때 발생할 수 있는 문제를 보려고 합니다.

그래서 context를 잘 분리한 효정님의 앱을 들어가서 profiling을 해봤습니다!
아래 이미지는 user 토글 후 리렌더 된 컴포넌트를 표시해주는 이미지입니다.
user rerender
user를 토글할 때마다 user와는 상관없는 메세지 form까지 리렌더되는 게 보입니다.

메세지 form 컴포넌트가 useMessage 훅을 구독하고 있고, useMessage는 useUser를 통해 user context를 구독하고 있기 때문에 user 데이터의 변경에 따라 리렌더가 필요없음에도 리렌더됐습니다!
아래와 같은 순서로 변경사항이 반영됐겠죠?

user context -> useUser -> useMessage -> InputMessageForm

커스텀 훅은 컴포넌트단에서 데이터를 다루는 로직을 숨겨줘서 좋지만 이렇게 불필요한 리렌더를 유발하기도 하는 것 같아요.

useMessage 훅에서 addMessage만 쓰는 경우에는 해당 컴포넌트 -InputMessageForm- 의 리렌더가 현재 앱 라이프 사이클에서는 전혀 필요하지 않다고 생각해서 불필요하다고 생각했슴다..

  • 그럼 이 문제는 어떻게 해결하면 좋을까.. 생각해봤는데요!

단순히 데이터를 보내기만 하는 경우 (InputMessageForm 컴포넌트처럼요!),

현재 코드처럼 useMessage 훅의 addMessage를 직접 import 하기보다, 상위 컴포넌트로 이벤트를 바로 뱉어주는 게 좋을 것 같다는 생각을 해봤습니다. (EmoticonPopover 구현하신 것처럼 콜백함수 받아서 올려주는 걸 얘기하는 거에요!)

App.tsx

const { messages, addMessage } = useMessage();

...
return (
    <InputMessageForm sendMessage={addMessage} />
    ...
) 
InputMessageForm.tsx

const InputMessageForm = ({ sendMessage }) => {
    ...

    const _addInputMessage = (e: React.FormEvent): void => {
    e.preventDefault();

    if (!text.trim()) setVisibleAlert(true);
    else {
      sendMessage(text);
    }
    resetText();
  };
  
    return (
        <form onSubmit={_addInputMessage} > 

        </form>
    )

}

이렇게 하면 InputMessageForm은 useMessage 훅을 구독하고 있지 않기 때문에 user 토글로 인한 리렌더로부터 해방(?)될 수 있습니다.

  • 생각해보니 이 방법도 이슈가 있을 것 같슴다!

핸들러 함수를 prop으로 내려주기 때문에 상위 컴포넌트가 리렌더될 때 함수 재선언이 일어나면 핸들러 함수를 prop으로 받는 자식 컴포넌트도 리렌더가 될 것 같아요..
그래서 확인해봤는데요,

아래 두 개의 사진은 message form의 텍스트 데이터가 변경될때마다 상관없는 Emoticon 영역이 리렌더되는 걸 막기 위해 useCallback과 memo 적용해서 실제 리렌더 안 되는 걸 보여주는 이미지입니다! (첫 번째가 현재 코드고, 두 번째 이미지가 해결된 모습입니다)

emoticon comp rerender issue

useCallback

만약에 custom hook - context 문제 해결을 위해 콜백함수를 하위 컴포넌트에 내려줘서 하위 컴포넌트의 직접적인 데이터 조작(custom hook사용이든 dispatch이든) 을 막았을 경우,
하위 컴포넌트에 내려주는 콜백함수를 useCallback으로 감싸주고 하위 컴포넌트를 memo로 감싸줘서 prop으로 내려주는 콜백함수의 재선언에 따른 리렌더를 막을 수 있습니다.

Emoticon 영역을 담당하는 컴포넌트가 InputMessageForm의 자식 컴포넌트이기 때문에, 아래처럼 코드를 바꿔보면 위에서 본 두번째 이미지처럼 리렌더가 일어나지 않는 걸 확인할 수 있었슴다!

InputMessageForm = () => {

...

    const addEmoticonMessage = useCallback((emo: string): void => {
        addMessage(emo);
        resetText();
    }, []);


    return (
        <InputEmoticon addEmoticon={addEmoticon} />
       ....
    )
}
InputEmoticon = ({ addEmoticon}) => {...}

export default memo(InputEmoticon)
}

관련해서 참고하면 좋은 글입니당당구리 ㅈㅅ
https://leehwarang.github.io/2020/05/02/useMemo&useCallback.html

아무튼.. 발생할 수 있는 여러 이슈랑 해당 이슈 해결, 한계 등등 여러 얘기를 해봤는데요,,, 저도 잘 모르고 공부하는 분야기 때문에 이해가 잘 되게 쓴 지는 잘 모르겠네요! 설명이 이상한 부분, 잘못된 부분 모두 지적해주시면 감사하겠습니다!
과제하시느라 너모 고생하셨으요~~~~~!

@jhj2713
Copy link
Member Author

jhj2713 commented May 6, 2022

안녕하세요 멘토님! 코멘트 너무 정성스럽게 남겨주셔서 정말 감동했습니다...:sob:

interface 분리하는 부분 너무 좋은 것 같아요. 저도 타입 수정할때 스크롤 쭉 내리면서 귀찮아했으면서 왜 분리할 생각을 못했나 싶네요...:cry:

context는 항상 습관처럼 분리해서 만들었어서 global로 묶어서 사용하면 다른 state를 구독한 컴포넌트도 리렌더링 된다는 부분은 처음 알았네요..! recoil은 언젠가 해봐야지 해봐야지 하면서 한 번도 사용해보지 않았는데 이번 기회에 공부해서 적용해봐야겠습니다! 링크도 공유해주셔서 감사합니다~!

form 컴포넌트도 함께 리렌더링 되는 부분은 차마 고려하지 못했었는데 이렇게 해결법까지 알려주신 점 너무 감사드립니다. 해결하는 방법을 단계별로 상세하게 설명해주셔서 이해가 쏙쏙 됐어요... 특히나 useCallback이랑 memo에 아직 완전 익숙하지는 않았는데 직접 코드로 보니까 더 감이 오는 것 같아요! 말씀해주신 점 꼭 모두 반영해서 수정해보도록 하겠습니다!!:thumbsup:

corinthionia pushed a commit to corinthionia/react-messenger-15th that referenced this pull request May 7, 2022
[3주차] 박준열 미션 제출합니다.
corinthionia added a commit to corinthionia/react-messenger-15th that referenced this pull request May 7, 2022
Copy link

@chaaerim chaaerim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 ! 코드리뷰 파트너 김채림입니다 🥳
효정님 .. . 정말 코드 꼼꼼하게 작성하시는군요 .. 코드리뷰하면서 정말 많이 배웠습니다 그리고 반성했어요 ^6^..
제가 감히 효정님 코드를 리뷰할 수준이 아닌 것 같지만 위에서 남겨주신 MessageBalloon을 간단하게 쓸 수 있는 방법에 대해 같이 고민해봤습니다!
그럼 스터디 시간에 뵐게요!!

Comment on lines +14 to +31
const _handleResize = () => {
if (window.innerWidth <= 640) {
setIsMobile(true);
} else {
setIsMobile(false);
}
};

useEffect(() => {
if (window.innerWidth <= 640) {
setIsMobile(true);
}

window.addEventListener("resize", _handleResize);
return () => {
window.removeEventListener("resize", _handleResize);
};
}, []);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

가장 먼저 제출하셨는데 심지어 반응형으로 만드셨군요 .. 효정님 .. 사랑합ㄴㅣ다..

Comment on lines +9 to +27
return (
<MessageBox>
{!isUser ? (
<MessageImg alt="profile" src={message.user.profileImg} height={30} />
) : (
<MessageTime isUser={isUser}>{message.time}</MessageTime>
)}
<section>
<MessageUser isUser={isUser}>{message.user.name}</MessageUser>
<MessageText isUser={isUser}>{message.text}</MessageText>
</section>
{isUser ? (
<MessageImg alt="profile" src={message.user.profileImg} height={30} />
) : (
<MessageTime isUser={isUser}>{message.time}</MessageTime>
)}
</MessageBox>
);
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user인지 아닌지에 따라서 스타일컴포넌트로flex-direction: row-reverse; 사용해서 이미지, 이름, 텍스트, 시간 나열 순서 조절하시면 더 간단하게 코드 작성해볼 수 있을 것 같아요 !!
저도 몰랐던 속성인데 이번에 어떻게 하면 간단하게 작성할 수 있을까 꼼수를 부리다가 ㅎㅎ....flexbox 속성 정리해놓은 글을 발견해서 남기고 갑니다 !!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오오 이 코드 너무 마음에 걸렸는데 flex-direction으로 조절할 수 있군요! 좋은 글 공유해주셔서 감사합니다!!:thumbsup:

id: new Date().valueOf(),
user: user.currentUser,
time: curTime,
text,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

key: value에 맞게 text: text, 로 작성하시면 더 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아핫 감사합니다!

height: 19%;
border-bottom: 1px solid lightgrey;
display: grid;
grid-template-columns: 1fr 150px 1fr;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

효정님 덕분에 그리드도 공부하고 가요 !!! 🤩

isMobile={isMobile}
/>
<InputButton>
<img alt="send" src="send.png" width={20} />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이미지 크기를 css가 아닌 태그에서 제어해주신 경우가 종종 보이던데 이유가 있을까요 ?!?!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

특별한 이유는 없고 width 하나만 설정해주기 위해서 styled-components를 하나 더 정의해야 한다는게 조금 번거로워서(ㅎㅎ) img 태그의 속성에 사용했어요!

Copy link

@siwonblue siwonblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요. 효정님 ✋🤚
코드리뷰 파트너 전시원입니다.
제가 좀 근본없이 코드쓰는 스타일인데 꼼꼼하게 해두신 거 보고 많이 배웠습니다.
저한테 리뷰 달아주신 것도 도움이 많이 됐구요
전반적인 동작 흐름이나 컴포넌트 구조를 짜임새 있게 설정하는 것에서부터
css style 에 recoil 을 이용한 상태관리까지 전반적으로 많은 부분을 배울 수 있어서 좋았습니다.

이따 스터디 시간에 뵐게요 ~ !

Comment on lines +22 to +31
useEffect(() => {
if (window.innerWidth <= 640) {
setIsMobile(true);
}

window.addEventListener("resize", _handleResize);
return () => {
window.removeEventListener("resize", _handleResize);
};
}, []);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그냥 너무 꼼꼼하시네요 .. 👍

);
};

const Wrapper = styled.section`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semantic tag 잘 활용하시는 거 저도 본 받아야겠어요!

Comment on lines +7 to +9
const rootElement = document.getElementById("root");
if (!rootElement) throw new Error("Failed to find the root element");
const root = ReactDOM.createRoot(rootElement);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const root = ReactDOM.createRoot(
  document.getElementById('root') as HTMLInputElement
);

저도 비슷한 문제를 겪었는데
이렇게 해줘도 해결 되더라구요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오오 그렇군요. 좋은 방법 알려주셔서 감사합니다~!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants