-
Notifications
You must be signed in to change notification settings - Fork 0
Fe/bugfix/#442 비밀번호를 틀리고 재입장 시도시 404에러 #514
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
The head ref may contain hidden characters: "FE/bugfix/#442-\uBE44\uBC00\uBC88\uD638\uB97C-\uD2C0\uB9AC\uACE0-\uC7AC\uC785\uC7A5-\uC2DC\uB3C4\uC2DC-404\uC5D0\uB7EC"
Conversation
- 다른 코드와 통일해주기 위해
- 기존에는 메인화면으로 갈건데 한번 더 물어봄. 이는 유저 경험을 하락시키므로 바로 이동시킴
- useOutletContext를 사용해 관리하면 더 복잡해졌기에 zustand를 사용해 관리해줌
- 기본적으로 tailwindcss의 클래스들은 @tailwind utilities에 존재함 - 그리고 layer의 순서는 tailwind.css에 정의된 순서로 적용함 - 그런데 utilities에 사용자 정의 클래스를 선언하면 나중에 적용됨 - 그래서 이렇게 해도 hidden이 나중에 적용됨 <div className={`flex-with-center hidden`}> - 우선순위를 피하기 위해 따로따로 적용해줌
- typescript 확장 하는 방식을 수정함
- 더 편하게 입력 테스트를 하기 위해
- 이벤트가 여러번 등록되던게 문제였음 - 최초 한번만 등록되도록 수정함
- 기존에는 함수의 입출력 결과만 확인했는데 이러면 테스트의 수정이 빈번할것같음 - 그래서 이번에는 유저행동 기반으로 테스트를 작성함
- 다음 들어와서도 이전 상태를 유지했기에 수정
- 해당 상태는 전역으로 있을 필요가 없음 - 그리고 전역으로 두면 오히려 헷갈림 - 그래서 특정 컴포넌트의 상태라는걸 알리기 위해 useOutletContext를 사용해줌
- 클래스 이름 -> 조건에 맞아야 렌더되게
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.
이번에 테스트 리팩토링 하면서 테스트 설명 다 뜯어 고쳤는데 오빠 테스트 설명은 진짜 자세히 잘 적은 것 같아!! 나중에 에러가 발생하더라도 어떤 부분에서 오류가 났는지 추적하기 쉬울 것 같아 👍👍
// export function randomString(length: number = 8) { | ||
// const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; | ||
// return chars.slice(0, length); | ||
// } |
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.
💬 모킹 함수가 안 붙은 거 외엔 아래 함수랑 로직이 동일한 것 같은데 주석으로 남겨놓은 이유가 있어?
|
||
humanSocket.connect(); | ||
humanSocket.emit('generateRoomName'); | ||
humanSocket.on('roomNameGenerated', (roomName: string) => { | ||
setHost(true); | ||
becomeHost(); |
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.
👍 의미가 좀 더 명확해 진 것 같아!!
}; | ||
}; | ||
const roomName = '123lk12j3'; | ||
// const password = '1234'; |
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.
💬 password
사용하지 않는다면 없애도 될 것 같아!
expect(socket.on).toBeCalledWith('roomCreated', expect.any(Function)); | ||
}); | ||
|
||
it('방 생성 후 Popup에서 확인을 누를시 비밀번호와 함께socket의 createRoom 이벤트를 emit한다', async () => { |
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.
💬 사소한 거지만 함께
뒤에 띄어쓰기 누락된 같아!
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.
💬 HumanChatPage에 있는 hook들 business/hook으로 안옮겨도 될까??
변경 사항
고민과 해결 과정
방 생성 취소시 바로 메인화면으로 돌아감
기존에는 방 생성 도중에 취소를 누르면
나갈거야?
라는 팝업을 띄운 후 확인을 한번 더 눌러야함그런데 취소를 누른다는건 나간다는 말과 다를바 없음
그래서 취소를 누르면 바로 메인화면으로 보내버림
HumanChattingPage 상태 전역으로 관리
전역 상태로 바꿔봤는데 이게 어디서 온 데이터인지 더 알기 어려웠음
그래서 HumanChattingPage와 관련된 상태는 OutletContext로 하위 라우터들에게 내려주도록 다시변경함
tailwindcss @layer 적용순서 수정
사용자 정의 클래스를 addUtilities로 해서 적용순서와 관련된 문제가 발생했음
이유는 css의 cascading 순서 때문임 mdn문서
tainwind.css 파일 안쪽을 보면 상단에 아래와 같이 정의되어 있음
즉 @layer를 base, components, utilities 순으로 적용하겠다는 소리임
tailwindcss에서 제공하는 w-100과 같은 토큰들은 저기
utilities
에 적용되어 있음그리고 사용자가 정의한 클래스는 제일 뒤에 정의되게됨
예를들면 flex-with-center 는 아래와 같이
addUtilities
메서드를 통해 추가해줬음이 뜻은 즉 @layer utilities 에 추가하겠다는 말과 같음
평소에는 아무문제 없지만 다른 클래스와 섞어서 쓰면 문제가 발생 할 수 있음
이런식으로 스타일을 선언하면 hidden이 나중에 적용될것 같지만 실제로는 그렇지 않았음
hidden이 먼저 적용되고 -> flex-with-center가 나중에 적용됨
이건 cascading 순서때문에 일어나는 일인데 tailwindcss가 선언한 스타일 -> 내가 선언한 스타일 순으로 적용되기 때문에 문제가 생김
그래서 hidden이 먼저 적용되고, flex-with-center는 나중에 적용됨
이걸 해결하려면
3, 4, 5중 아무거나 선택해도 됐었는데 5번을 선택 할 때가 비용이 가장 낮다고 생각했기에 특정 조건이 맞을때만 렌더링시켜줌
vi가 any가 됐던 문제 해결
vi의 타입이 any가 돼서 테스트 할 때 불편했음 그래서 vitest.d.ts 파일 자체를 확장시키는게 아니라
별도의 d.ts 파일에서 vitest를 import한 후 matchers를 추가해 주는 방식으로 변경함
테스트 할 때 발생하는 유저의 이벤트를 fireEvent -> userEvent로 발생시킴
만약 fireEvent를 사용하면 유저가 입력하는 상황을 테스트 할 때 번거로움
이런식으로 작성해야 하는데 userEvent를 사용하면
아래처럼 덜 번거롭게 작성할 수 있음
추가적으로 userEvent를 사용하면 실제 유저가 클릭할때와 유사하게 이벤트가 실행됨.
하지만 fireEvent를 사용하면 코드상으로만 클릭이 됨
공식 문서에서도 userEvent사용을 권장 하고 있음
단, 커스텀 이벤트를 테스트 하거나 특정 이벤트를 직접 발생시켜야 하는 단위 테스트등에서는
fireEvent가 더 적합하다고 함.
404 에러 뜨는 #442 이슈 해결
단순히 이벤트 핸들러가 여러번 등록 돼서 잘못된 경로로 사용자가 갔기에 뜨는 에러였음
상태기반 테스트 -> 행동기반 테스트로 변경
useSignalingSocket 훅은 Popup이 뜨고
확인
취소
버튼을 누르는거에 따라 다른 결과가 나옴버튼을 눌러야 하기에 실제로 버튼을 눌러보는식으로 테스트를 진행함
(선택) 테스트 결과
ex) 베이스 브랜치에 포함되기 위한 코드는 모두 정상적으로 동작해야 합니다. 결과물에 대한 스크린샷, GIF, 혹은 라이브 데모가 가능하도록 샘플API를 첨부할 수도 있습니다.

close #442