-
Notifications
You must be signed in to change notification settings - Fork 0
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
[#20] 유저가 작성한 코드 결과 서버로 보내기 #106
The head ref may contain hidden characters: "20-\uC720\uC800\uAC00-\uC791\uC131\uD55C-\uCF54\uB4DC-\uACB0\uACFC-\uC11C\uBC84\uB85C-\uBCF4\uB0B4\uAE30"
Conversation
useEffect(() => { | ||
if (!socket.hasListeners('message')) { | ||
socket.on('message', handleMessage); | ||
const totalSubmissionResult = 10; |
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 totalSubmissionResult = 10; | ||
setScoreResults( | ||
range(0, totalSubmissionResult).map((_, index) => ({ | ||
problemId: -1, |
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.
따로 변수 안 만들고 -1 넣어서 로딩 걸어주고 진짜 문제 번호 들어오면 점수 넣는 거 너무 좋네요
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.
동작 안시킨 케이스를 빼먹었네요 감사합니다
axios | ||
.get(`http://101.101.208.240:3000/competitions/${competitionId}`) | ||
.then((response) => { | ||
setCompetition(response.data); | ||
}) | ||
.catch((error) => { | ||
console.error('Error fetching competition data:', error); | ||
}); | ||
updateCompetition(competitionId); |
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.
api분리한 거 좋네요
import type { ManagerOptions, SocketOptions } from 'socket.io-client'; | ||
import { io } from 'socket.io-client'; | ||
|
||
export type { Socket } from 'socket.io-client'; |
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 하시네요 👍🏻
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.
네 여기는 Socket.io를 사용한다는 로직을 socket 유틸 아래로 숨긴거에요
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.
다른 훅에서는 axios를 사용한 api를 분리하셨는데, 이 훅에서는 axios를 사용하는 이유가 있나요?
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.
그냥 분리하는걸 깜빡한 겁니다. 감사합니다
}; | ||
|
||
useEffect(() => { | ||
if (!socket.hasListeners('message')) { |
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.
아 message라는 이벤트가 소켓 객체를 만들면 항상 리슨하고 있다고 생각했는데,
scoket.on('message',cb) 하는 행위가 이벤트 등록이네요 ! 👍🏻
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.
네 소켓에 hasListeners API가 있습니다. 정확히는 Socket은 Emitter를 상속하는데 그래서 Emitter에 있는 hasListeners를 쓸 수 있는 거에요
아래 링크 참고
https://socket.io/docs/v3/client-api/#socketoneventname-callback
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.
와우.
const form = { | ||
problemId: currentProblem.id, | ||
code, | ||
} satisfies SubmissionForm; |
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.
satisfies는 처음 보네요. 진짜 고수..
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.
satisfies,
socket.io 객체를 커스텀해서 utils/socket 으로 사용한거 진짜 좋았습니다.
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.
LGTM
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.
Nice!
…o-with-me into 20-유저가-작성한-코드-결과-서버로-보내기
한 일
기존에 우찬님이 작성한 코드, 유석님이 작성한 코드를 변경한 내역이 많아서 먼저 Draft PR로 공유 드립니다. 미리 리뷰 해주세요. 이후 서버 쪽 채점 로직 배포되면 테스트 수행하고 반영해서 새 PR로 올리겠습니다.
스크린 샷
현재 소켓이 연결되어 connected 라는 문구가 뜨지만, 응답을 받고 있지 못해 무한 로딩이 도는 상황