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

채팅 관련 컴포넌트 리펙터링 #2188

Draft
wants to merge 12 commits into
base: dev
Choose a base branch
from
Draft

Conversation

storycraft
Copy link
Member

Description

채팅 관련 컴포넌트 전체 코드 구조 개선

Changelog

Migration

@storycraft storycraft added the Type: Maintenance Docs, code tidy, dependency update, etc. label Nov 17, 2023
Copy link
Member

@Su-Yong Su-Yong left a comment

Choose a reason for hiding this comment

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

message-group2chat-factory2가 본래 message-groupchat-factory를 대체하는 용도면 보기 좋네요!

리뷰단부분 하나만 고쳐주세용

Comment on lines 114 to 118
return <EmoticonMessage
src={url}
sound={soundUrl}
alt={alt}
/>;
Copy link
Member

Choose a reason for hiding this comment

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

async function으로 JSX.Element를 리턴하는 경우에는 owner를 명시해줘야 제대로 reactivity추적을 제대로 할수 있습니다
그래서 Promise<JSX.Element>형태로 리턴하는 함수는 runWithOwner를 모두 씌워주시면 됩니다

Copy link
Member

Choose a reason for hiding this comment

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

createEmoticon이 테스트하기 좋은 예시여서 여기에만 댓글을 달았는데, chat-factory2 안에있는 async function모두 바꿔주세요
owner를 잃으면 console에 warning이 뜨니 createEmoticon으로 owner가 제대로 설정되었는지 테스트 해보면 됩니다

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
Type: Maintenance Docs, code tidy, dependency update, etc.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants