Skip to content

Conversation

@nuagenic
Copy link
Contributor

@nuagenic nuagenic commented Sep 1, 2023

요약

담당자 시작일 마감일 관련 이슈
백창인 23-09-01 23-09-01 #31

관련 이슈

#31

체크리스트

PR 달 때

  • Enhancement나 Fix 라벨을 달았나요?
  • Review를 요청했나요?
  • yarn build로 테스트를 했나요?

머지하기 전에

  • 리뷰 코멘트를 모두 해결했나요?
  • squash merge 임을 확인했나요?

작업 내용

  • 현재까지 작업한 사항 올립니다. 아직 회원가입 기능을 정상적으로 구현하지 못했습니다. 도움 필요해서 코드 리뷰 한 번 해주시면 감사하겠습니다!
  • 메인으로 바꾼 부분은 RegisterContainer.js 와 SubmitButton.js입니다.
  • SubmitButton.js 를 태그에서 태그로 바꿨습니다. 그리고 라우터 기능을 useRouter를 이용해 RegisterContainer.js로 옮겼습니다. (회원가입과 라우팅 기능이 같은 버튼에서 동작하는데 이를 더 잘 관리하기 위함).
  • 에러 사항: '가입' 버튼을 눌러도 아무 일도 일어나지 않습니다. 원래 의도라면 handleRegister() 가 작동해야 하는데 디버깅해보니 함수 호출 자체가 되고 있지 않는 것 같습니다.
  • 그리고 전체적인 코드 방향성이나 맞게 하고 있는지 등 이슈로 짧게 30분 정도? 허들할 수 있다면 좋을 것 같습니다!

논의가 필요한 사항

@nuagenic nuagenic added enhancement New feature or request frontend 프론트엔드 작업 labels Sep 1, 2023
Copy link
Contributor

@designDefined designDefined left a comment

Choose a reason for hiding this comment

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

이상한 에러가 나길래 이것저것 엄청 뜯어봤는데... next 자체 에러였습니다ㅠㅜ

아래 달아둔 코멘트 외에, 작업하실 때
next.config.js 파일의 output: 'export' 속성이 있는 행을 주석처리해주세요(nextConfig 오브젝트 안에 아무 것도 없게) 그러면 회원가입 관련 리스폰스가 제대로 올 겁니다

추가적으로 코멘트로 개선점들 좀 달아두었습니다! 좀 간단히 적어두어서, 애매한 부분은 회의 때 같이 봅시다

import Link from "next/link";

export default function SubmitButton() {
export default function SubmitButton({ status }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

여기 onClick prop이 버튼 태그로 전달이 되지 않는 것 같습니다!

Comment on lines 29 to 31
headers: {
"Content-Type": "application/json",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

json으로 보내면 굳이 요 헤더는 없어도 자동으로 헤더 달아서 보내줄거에요 한 번 확인해보시지요

const [PWRegisterValue, setPWRegisterValue] = useState("");
const [PWConfirmValue, setPWConfirmValue] = useState("");
const [nameValue, setNameValue] = useState("");
const [registerSuccess, setRegisterSuccess] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

굳이 이 state를 둔 이유는?

Comment on lines 56 to 58
if (registerSuccess === true) {
// 성공한 경우 다른 페이지로 이동
router.push("/login");
Copy link
Contributor

Choose a reason for hiding this comment

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

한 번 성공하면 registerSuccess가 true가 되고, 다시 register 버튼을 눌렀을 때 이 if문의 함수가 작동해서 /login으로 이동을 하는데, 성공 시 바로 /login으로 이동한다면 굳이 필요는 없는 부분인 것 같습니다. 그보다는 이미 회원가입 요청이 보내져 진행 중일 때 handleButtonAction이 두 번 작동되게 하는 If문이 좀 더 어울리지 않을까 하네요

// 아직 작업을 실행하지 않은 경우
try {
await handleRegister(); // 회원가입 요청 완료를 기다림
setRegisterSuccess(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

handleRegister에도 setRegisterSuccess(true) 함수가 있는데, 여기에도 있을 필요는 없을 것 같습니다. 그리고 handleRegister 자체는 성공/실패를 리턴하지 않아서 그 아래에 있는 setRegisterSuccess가 항상 작동하는 것 같아요! 실제로 에러가 발생했을 때에도 성공했다는 메시지가 표시됩니다

@nuagenic
Copy link
Contributor Author

nuagenic commented Sep 7, 2023

스크린샷 2023-09-07 오후 10 15 28

@designDefined 현재 양식에 맞게 정보를 입력하고 '가입' 버튼 클릭 시, internal server error 발생합니다.
그 상태에서 '가입' 버튼을 클릭하면, username already exists (console 상에서 code 400에 해당) 에러가 발생합니다.

이거 혹시 봐주실 수 있을까요? 지금까지 한 작업 커밋해 두겠습니다!

@nuagenic
Copy link
Contributor Author

nuagenic commented Sep 7, 2023

아직 handleRegister에 리턴 값을 넣는 작업은 시작 안 했는데, 현재는 handleButtonAction 자체를 지워버렸기 때문에 위의 에러는 이와 상관없겠죠??

@nuagenic
Copy link
Contributor Author

nuagenic commented Sep 8, 2023

@designDefined 나머지 todo는 일단 완료했습니다! contextAPI 는 다음 PR에서 해보겠습니다!

Copy link
Contributor

@designDefined designDefined left a comment

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

enhancement New feature or request frontend 프론트엔드 작업

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants