hook의 인터페이스 개선하기
코드 리뷰를 하던 중에 다음과 같은 훅이 사용되는 것을 발견하였다.
useDisableBack(true, () => {
if (isOpen) {
handleClose();
}
});
해당 훅은 다음과 같은 목적을 달성하기 위하여 사용되었다.
- 모달이 열려있을 때는 브라우저 뒤로가기를 무효화시키고 열려 있는 모달 창을 닫음(handleClose)
- 모달이 닫혀 있을 때는 브라우저 뒤로가기를 실행
소스 코드를 살펴본 결과 정확히는 다음과 같은 인터페이스로 설계되어 있었다.
useDisableBack(enabled: boolean, onBackPress: () => void)
그리고 다음과 같이 동작을 수행한다.
- enabled가 true일 경우, 브라우저 뒤로가기를 실행시키지 않고 callback을 실행한다.
- enabled가 false일 경우, 브라우저 뒤로가기를 실행시키고 callback을 실행하지 않는다.
내가 이해하기 난해했던 것은 enabled
option이었다. enabled가 true인 경우 브라우저 의 뒤로가기가 enable되는지, 아니면 onBackPress가 enabled되는지에 대한 정보를 내부 코드를 읽기 전까지 알기가 어려웠다. 따라서 동료 개발자님과 대화를 나눈 뒤에, 다음과 같은 목표를 가지고 훅을 재설계하게 되었다.
- 브라우저 뒤로가기의 실행 여부와 callback의 실행 여부를 분리한다.
react native의 BackHandler의 인터페이스 디자인을 참고하여 다음과 같은 형태로 hook이 리팩토링 되었다.
useBackHandler(handleBack: () => 'block' | void);
기존의 useDisableBack
과 비교했을 때 다음과 같은 특징을 가진다.
- boolean flag가 아닌 'block' | void로, 브라우저의 뒤로가기 실행 여부를 나타낸다는 것을 명확하게 표현한다.
- 브라우저 뒤로가기의 실행 여부와 커스텀 handleBack의 실행 여부를 명확하게 구분할 수 있다.
리팩토링한 코드의 예시를 보자.
useBackHandler(() => {
if (isOpen) {
handleClose();
return 'block';
}
});
콜백에서 조건문을 통해 커스텀 로직이 언제 실행되는지를 파악할 수 있다. 그리고 'block', undefined 반환 여부를 통해서 브라우저 뒤로가기를 막는다는 것을 파악할 수 있어 가독성 개선이 이루어졌다고 생각한다.
프로덕션에서의 에러
리팩토링한지 이틀이 되어서, 다른 동료 개발자분으로부터 해당 hook을 사용할 때 페이지 라우팅에 문제가 되어서 이를 롤백했다는 전달을 받았다. 나는 영향을 받는 시나리오에 대해 QA를 모두 수행하였다고 생각하였는데, 다음과 같은 작업을 진행하였다.
- 모달 창이 열려있을 때, 페이지 히스토리가 뒤로가버리는 대신에 모달 창이 닫힌다.
- 모달 창이 닫혀있을 때, 페이지 히스토리가 뒤로 간다.
문제는 내가 QA를 진행한 곳과는 다른 곳에서 발생했다. useBackHandler()에서 effect cleanup을 할 때, history.back()을 호출하는 것이 문제라고 전파해주셨다. 처음에는 해당 코드가 어떻게 문제가 되는지 잘 이해가 되지 않았는데, 곰곰히 생각해보니 다음과 같은 시나리오에서 문제가 발생한다는 것을 알게 되었다.
문제 시나리오
- useBackHandler() hook이 사용되고 있는 페이지 naver.com/a 에서 다른 페이지 naver.com/b로 라우팅을 시도한다.
- 페이지 컴포넌트들이 unmount되면서 useBackHandler의 cleanup 함수가 실행된다. 이 때 history.back()이 실행된다.
- naver.com/b로 라우팅이 되었다가 원래 있던 naver.com/a로 다시 이동해버린다.
이로 인해 naver.com/a url에 있던 사용자는 어떤 다른 페이지로도 이동할 수 없는 감옥에 빠져버린 것이었다. 해당 url에 접속한 사용자는 굉장한 답답함을 느꼈을 것이다!
useBackHandler의 동작 원리에 대한 설명
useBackHandler의 대략적인 코드는 다음과 같다.
function useBackHandler(onBackPress) {
useEffect(() => {
const handleBack = () => {
history.pushState(null, '', '');
}
window.addEventListener('popstate', handleBack);
return () => {
window.removeEventListener('popstate', handleBack);
history.back(); // unmount시 뒤로 가버린다!
}
}, [preservedHandleBack]);
}
위 코드의 cleanup history.back()의 존재로 인해서, 페이지 라우팅 -> cleanup 함수 실행 -> history.back() 실행으로 유저가 감옥에 갇힌다.
대응
일단 해당 useBackHandler를 disable 시켰고, 수정이 되는대로 다시 QA를 해서 넣을 생각이다. useBackHandler를 사용하기 전에 내부 코드에 대해서 완전히 이해했다면 상황을 막을 수 있지 않았을까 싶어, 리팩토링되는 코드에 대해 QA도 중요하지만 코드 확인도 중요하다는 것을 깨닫게 되었다.
Top comments (0)