refactor: 캡챠 입력 값 초기화 #276
Conversation
…captcha-input-initial refactor: 캡챠 입력값 초기화
| useEffect(() => { | ||
| return onClearInput(); | ||
| }, []); |
There was a problem hiding this comment.
이걸 clean up 함수에서 실행시키고픈 의도면 다음과 같이 변경해보시기 바라요~!
어떤 의도일까요??
return () => {
onClearInput();
}There was a problem hiding this comment.
해당 캡챠가 열릴 때, onClearInput() 함수를 통해서 input 값을 클린업 시키려고 작성했습니다!
저는 그냥 해당 컴포넌트가 마운트 됐을 때 onClearInput() 함수를 반환하고자 했습니다. 그런데 clean up이 목적이라면 보내주신 로직으로 컴포넌트가 언마운트 됐을 때만, onClearInput() 함수가 실행되는 것이 clean up 목적에는 더 맞네요!!
수정해보겠습니다, 감사합니다!!
There was a problem hiding this comment.
useEffect 의 클린업 함수를 의미하는거였어요!
말씀하신대로면 useEffect 함수에서 실행하는데 맞는거 같은데.. 어떻게 생각하시나용?
There was a problem hiding this comment.
저는 onClearInput가 input 값을 지우는 역할만 하므로, useEffect의 클린업 함수에 들어가는 게 맞다고 생각합니다!! 혹시 제가 개념적으로 놓치고 있는 부분이나 잘못 이해하고 있는 부분이 있을까요??
There was a problem hiding this comment.
이걸 clean up 함수에서 실행시키고픈 의도면 다음과 같이 변경해보시기 바라요~! 어떤 의도일까요??
return () => { onClearInput(); }
이전 PR에서 놓쳤었네요.... 확인해주셔서 감사합니다 👍
There was a problem hiding this comment.
익명함수의 경우 nodejs에서는 바로 gc로 넘어가기 때문에 메모리 측면에서 큰 차이가 없습니다.
There was a problem hiding this comment.
그럼 return onClearInput처럼 함수 자체를 반환하는 방법과 return () => { onClearInput() };처럼 익명 함수 내에서 함수의 결과를 반환하는 방법 중 어떤 방법이 해당 코드에는 더 적합하다고 생각하시나요??
저는 현재는 input 값을 초기화하는 간단한 작업만 들어있지만, 향후 로직이 추가될 가능성을 열어두었을 때도 생각한다면 return () => { onClearInput() }; 방식이 더 맞다고 생각합니다!!
There was a problem hiding this comment.
익명함수의 경우 nodejs에서는 바로 gc로 넘어가기 때문에 메모리 측면에서 큰 차이가 없습니다.
아하 새로운 사실을 알았네요 감사합니다.
그렇지만 메모리 측면에서 차이가 없다고 하더라도 제 생각에는 불필요한 익명함수를 제거하는 것이 가독성 측면에서 더 좋다고 생각합니다.
There was a problem hiding this comment.
저는 onClearInput가 input 값을 지우는 역할만 하므로, useEffect의 클린업 함수에 들어가는 게 맞다고 생각합니다!! 혹시 제가 개념적으로 놓치고 있는 부분이나 잘못 이해하고 있는 부분이 있을까요??
아닙니다! 제가 코멘트를 잘못 읽었었던 것 같아요.
There was a problem hiding this comment.
여쭤봐 주신 덕분에, 몰랐던 내용들을 많이 알아가는 거 같습니다!! 감사드립니다!! 😁
loopy-lim
left a comment
loopy-lim
left a comment
There was a problem hiding this comment.
한번만 다시 확인해주시면 감사하겠습니다!
| @@ -1,25 +1,32 @@ | |||
| import { ChangeEventHandler } from 'react'; | |||
| import { ChangeEventHandler, Dispatch, SetStateAction, useEffect } from 'react'; | |||
There was a problem hiding this comment.
쓰지 않는 import는 제거해주시기 바랍니다.
| useEffect(() => { | ||
| return onClearInput(); | ||
| }, []); |
There was a problem hiding this comment.
익명함수의 경우 nodejs에서는 바로 gc로 넘어가기 때문에 메모리 측면에서 큰 차이가 없습니다.
|
리뷰가 길어져서 미안해요 ㅠㅠ 정 바쁘면 바로 머지하시고 테스트하셔도됩니다. |
smb0123
left a comment
smb0123
left a comment
There was a problem hiding this comment.
일단 동작에는 문제가 없으니 이후에 수정해서 다시 PR 올리시면 될 것 같아요~
|
급한건 아니라 다시 수정해서 pr 날리겠습니다!! |
|
요청 사항 반영해서 main으로 머지하겠습니다!! |
주요 변경사항
리뷰어에게...
바로 운영 서버로 머지해서 테스트해보기 위해 바로 pr 날립니다!!
체크리스트
reviewers설정label설정