-
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
oAuth, 로그인 유무 로직 구현 #86
Conversation
…coko-Front into feature/#67/OAuth_2_0
…coko-Front into feature/#67/OAuth_2_0
…coko-Front into feature/#67/OAuth_2_0
…le-team/8term-coko-Front into feature/#67/OAuth_2_0
src/apis/axios/intercepter.ts
Outdated
const accessToken: string | undefined = getCookie('accessToken'); | ||
if (accessToken) { | ||
config.headers.Authorization = `Bearer ${accessToken}`; | ||
} |
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.
저도 처음엔 그런 줄 알았으나 User
각각의 토큰이 백엔드에 저장이 따로 안 되기 때문에 요청 시에 유저의 토큰을 보내줘야 백엔드에서 그 토큰을 까서 작업을 해줄 수 있다고 합니다. 예를 들면, 만료시간이나 userImail 등이 있습니다.
src/apis/axios/intercepter.ts
Outdated
console.log(response); | ||
return response; | ||
}, | ||
error => { | ||
// HTTP 상태 코드가 에러인 경우 | ||
console.log(error); |
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.
값 체크 후 테스트 console.log
는 지워주세요!!
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.
아이고 예전에 남아있던 거였나보네요 ㅎㅎ
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.
#86 (comment)
해당 코멘트에서 이를 수정하였습니다~
src/hooks/useDropdown.ts
Outdated
import { useState } from 'react'; | ||
|
||
/** | ||
* @description | ||
* 이 훅은 드롭다운의 열림/닫힘 상태를 관리합니다. | ||
* 드롭다운의 외부 클릭 이벤트가 필요한 경우 `useOutsideClick` 훅과 같이 사용할 수 있습니다. | ||
* | ||
* @example | ||
* const { isOpen, toggleDropdown, closeDropdown } = useDropdown(); | ||
* | ||
* return ( | ||
* <> | ||
* <button onClick={toggleDropdown}>Toggle</button> | ||
* {isOpen && <div>Dropdown Content</div> } | ||
* </> | ||
* ); | ||
*/ | ||
const useDropdown = () => { | ||
const [isOpen, setIsOpen] = useState(false); | ||
|
||
const toggleDropdown = () => setIsOpen(prev => !prev); | ||
const closeDropdown = () => setIsOpen(false); | ||
|
||
return { isOpen, toggleDropdown, closeDropdown }; | ||
}; | ||
|
||
export default useDropdown; |
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.
이거 useOutsideClick
도 useDropdown
내부로 넣어서 이벤트함수를 전달받는 형식으로 구현되는게 더 범용성이 좋을 것 같은데 어떤가요?
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.
뭔가 제 생각엔 기능이 다른 것 같아서 일단 분리를 해놨지만 비슷한 고민을 하고 있었습니다. @rhehfl 님께서 말씀하신 것처럼 수정해보고 코멘트 남기겠습니다!
src/hooks/useAuth.ts
Outdated
const useAuth = () => { | ||
const accessToken = getCookie('accessToken'); | ||
|
||
return { | ||
isLoggedIn: !!accessToken, // accessToken이 존재하면 true, 없으면 false | ||
}; | ||
}; |
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.
의견 감사합니다! 생각해보니 로그인 상태를 단순히 판별하는 로직이다보니 유틸함수로 만들면 로직이 훨씬 간결하겠네요.
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.
#86 (comment)
해당 코멘트에서 커스텀 훅 -> 유틸함수로 변경하였습니다!
import { getCookie } from '@utils/cookies';
/**
* @description
* 사용자의 로그인 상태를 판별합니다.
* accessToken이 쿠키에 존재하는지를 확인하여 로그인 여부를 반환합니다.
*
* @returns
* `boolean`: 사용자가 로그인 상태인지 여부
*
* @example
* if (isLoggedIn()) {
* console.log("사용자가 로그인 상태입니다.");
* } else {
* console.log("로그인이 필요합니다.");
* }
*/
const isLoggedIn = (): boolean => {
const accessToken = getCookie('accessToken');
return !!accessToken; // accessToken이 존재하면 true, 없으면 false
};
export default isLoggedIn; 기존 useAuth(커스텀 훅)에서 isLoggedIn만 빼서 더 간결하고 범용성 있도록 유틸함수로 변경하였습니다~ |
src/hooks/useDropdown.ts
Outdated
const useDropdown = () => { | ||
const [isOpen, setIsOpen] = useState(false); | ||
|
||
const toggleDropdown = () => setIsOpen(prev => !prev); | ||
const closeDropdown = () => setIsOpen(false); | ||
|
||
return { isOpen, toggleDropdown, closeDropdown }; | ||
}; |
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.
내부적으로 dropDown이라는 기능이 없이 사실상 useBoolean
혹은 useToggle
훅인 것 같네요 ㅋㅋㅋ😂
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.
맞는 말씀이십니다... 결국 상태만 관리하는 훅인지라 별 의미없는 훅이 된 것 같네요 ㅎㅎ
#86 (comment)
@rhehfl 님께서 말씀하신 것처럼 useDropdown
훅 내부에 useOutsideClick
훅을 이벤트 함수로 전달 받는 게 좋아보입니다!
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.
@bluetree7878 좀 찾아보니 저희가 만들고자 하는 훅이 드롭다운보다는 팝오버에 조금 더 가까운거같은데 네이밍 변경하는건 어떨까용??
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.
@rhehfl
좋은 것 같습니다! 저도 찾아보니 오히려 Popover의 특징에 가장 걸맞는 것 같습니다.
Popover
- 타이틀, 이미지, 버튼, 링크 등 다양한 요소 포함
- 더 많은 보조 설명 또는 추가 정보 제공
- 클릭 인터랙션 작동
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.
import { useState } from 'react';
import useOutsideClick from '@hooks/useOutsideClick';
/**
* @description
* 이 훅은 팝오버의 상태를 관리하고 외부 클릭을 감지하여 팝오버를 닫는 동작을 간단하게 처리할 수 있도록 도와줍니다.
*
* @param excludeRefs 외부 클릭 감지에서 제외할 요소들의 ref 배열 (optional)
*
* @example
* const profileRef = useRef<HTMLDivElement>(null);
* const { isOpen, togglePopover, closePopover, popoverRef } = usePopover({
* excludeRefs: [profileRef]
* });
*
* return (
* <div ref={profileRef} onClick={togglePopover}>
* <button>프로필</button>
* {isOpen && (
* <div ref={popoverRef}>
* <p>유저이름</p>
* <button onClick={closePopover}>닫기</button>
* </div>
* )}
* </div>
* );
*/
const usePopover = (options?: {
excludeRefs?: React.RefObject<HTMLElement>[];
}) => {
const [isOpen, setIsOpen] = useState(false);
const togglePopover = () => setIsOpen(prev => !prev);
const closePopover = () => setIsOpen(false);
const popoverRef = useOutsideClick(() => {
if (isOpen) closePopover();
}, options);
return { isOpen, togglePopover, closePopover, popoverRef };
};
export default usePopover;
이렇게 useOutsideClick
훅을 import하여 usePopover
(useDropdwon에서 네이밍 변경)훅 자체에 팝오버의 상태를 관리하면서 팝오버의 외부를 클릭하였을 때 팝오버가 닫히도록 작성해보았습니다. useOutsideClick
훅에 있는 excludeRefs 와 같이 usePopover
훅에도 똑같이 감지 예외를 지정할 수 있는 옵션이 있습니다.
팝오버 UI 컴포넌트를 구현할 때 usePopover
에서 자체적으로 스타일을 동적으로 넣는 것도 고려를 해봤으나 확장성면에서와 스타일이 구겨짐 현상이 발생할 수 있을 것 같아서 그러한 기능은 넣지 않았습니다.
따라서 팝오버 UI를 만드실 때에는 버튼(팝오버를 나오게 할 요소)에
position: relative;
팝오버 UI 요소에는
position: absolute;
를 주시면 될 것 같습니다.
다르게 스타일 해도 될 수는 있을 것 같은데 저는 현재 이렇게 사용하고 있어요.
혹시 위와 같이 usePopover
훅을 작성했을 때 추가적으로 '뭔가 있으면 좋겠다' 혹은 '이 부분 수정해야 할 것 같다' 라는 부분 있으면 말씀해주시면 감사하겠습니다!
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.
useIsMounted찾아보니까 모던킷에도 비슷한 훅이 있네요
개인적 생각이긴 하지만 사용했을 때 결과 자체는 같지만 훅의 네이밍, 용도 등을 보았을 때 만들어 둔 useIsFirstRender
가 쪼오오끔 더 맞지않나.. 생각이 듭니다🙄
- 근데 그냥
useIsMounted
쓰는게 좋을거같기도 하고... 애매하네요 다른분들 의견이 궁금합니다🤔
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.
@bluetree7878 @rhehfl 그냥 open 함수도 제공하는 것은 어떨까요 ㅎㅎ? open, close, toggle 모두 제공해도 좋을 것 같습니다.
useIsFirstRender
는 현재 ref
를 다루는데 ref는 리렌더링에 영향을 안받기때문에 해당 이슈는 없을까요? 😄
useDropDown이 아니더라도 여러 케이스 고려가 필요해보입니다
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.
그냥 open 함수도 제공하는 것은 어떨까요 ㅎㅎ? open, close, toggle 모두 제공해도 좋을 것 같습니다.
@ssi02014 좋은 것 같습니다! 그냥 여러 함수를 제공하는게 더욱 범용성있게 사용될 것 같네요 ㅎㅎ 😊
useIsFirstRender
자체는 일부러 리랜더링에 영향을 받지 않고 첫 랜더링인지를 체크하는 용도로 제작되었기 때문에 아직까지는 큰 이슈가 없을 것 같습니다!🤔
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.
@rhehfl @ssi02014
useIsFirstRender
훅과 useIsMounted
훅은 둘 다 컴포넌트의 상태를 확인하는 커스텀 훅이지만 약간의 목적성과 동작이 다른 것 같습니다. useIsFirstRender
훅 같은 경우 첫 번째 렌더링 동안에는 true를 반환하고, 이후 렌더링에서는 false를 반환하고, 말씀하신 것처럼 useRef
를 사용하기 때문에 초기 렌더링 뒤로는 리렌더링이 안 되기 때문에 저 또한 useIsFirstRender
을 써서 초기 렌더링 시 콜백함수 실행을 방지시키는 거 정말 좋아보입니다! 👍👍
또한 usePopover
훅에서 openPopover
, closePopover
함수 모두 제공 해도 좋을 것 같다는 의견에도 범용성 있게 가져가려면 확실히 그게 좋을 것 같습니다.
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.
#86 (comment)
@rhehfl @ssi02014
위 코멘트에서 수정 내용 적었습니다!
const isExcluded = options?.excludeRefs?.some( | ||
excludeRef => excludeRef.current && excludeRef.current.contains(target) | ||
); |
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.
exclude 기능 좋네요 😲
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.
src\common\layout\Header.tsx
에서 ProfileWrapper
를 클릭했을 때 useOutsideClick
훅의 외부 클릭 감지가 트리거되기 때문에 ProfileDropdownMenu
가 닫혔다가 다시 열리는 문제가 발생하였는데 @rhehfl 님께서 외부 클릭 감지에서 제외할 요소를 받아내는 것도 좋을 것 같다는 의견을 내주셔서 반영하였습니다 ㅎㅎ
// 응답 인터셉터
api.interceptors.response.use(
response => {
// HTTP 상태 코드가 200번대인 경우
console.log(response);
return response;
},
error => {
// HTTP 상태 코드가 에러인 경우
return Promise.reject(error);
}
); 테스트 콘솔 제거와 추가로 파일명의 오탈자를 수정하였습니다. |
const handleLogout = () => { | ||
removeCookie('accessToken'); | ||
removeCookie('refreshToken'); | ||
window.location.reload(); // 로그아웃 후 새로고침 | ||
}; |
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.
새로고침 시, 토큰과, 쿠키가 없는 상태에서, 로그인이 필요한 페이지로 접근시 접근이 가능하거나 의도치 않은 오류가 발생할 수 있지 않나요?
root
경로로 이동시키는게 아니라, 새로고침을 한 이유에 대해서 궁금합니다🧐
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.
사용자 편의성을 위해 단순히 로그아웃을 했을 때 그 페이지에 남게끔 하고 싶었습니다. 새로고침을 한 이유는 확실하게 쿠키를 날리고 난 뒤에 로그인/비로그인의 보이는 화면을 달리 하고 싶었던 것이었으나, 생각해보니 말씀하신 것처럼 root
경로로 이동을 안 시켜주면 의도치 않는 오류가 발생할 상황이 생길 수 있겠네요.
import { removeCookie } from '@utils/cookies';
const handleLogout = () => {
removeCookie('accessToken');
removeCookie('refreshToken');
window.location.href = '/';
};
export default handleLogout;
이렇게 코드를 수정하겠습니다. 감사합니다!
commit - 53d65a8 import { useState, useEffect, useCallback } from 'react';
import { usePreservedCallback } from '@modern-kit/react';
import useOutsideClick from '@hooks/useOutsideClick';
import useIsFirstMount from '@hooks/useIsFirstMount';
/**
* @description
* 이 훅은 팝오버 상태를 관리하고 외부 클릭을 감지하여 팝오버를 닫는 동작을 처리합니다.
*
* @param {Object} options - 옵션 객체 (optional)
* @param {Function} options.callback - 팝오버 상태가 변경될 때 호출되는 콜백 함수
* 콜백은 현재 상태 (isOpen: boolean)를 매개변수로 받음
* @param {React.RefObject<HTMLElement>[]} options.excludeRefs - 외부 클릭 감지에서 제외할 요소들의 ref 배열
*
* @returns - 팝오버 상태 관리와 관련된 메서드와 ref를 반환
* @returns {boolean} `isOpen` - 팝오버의 현재 열림 상태
* @returns {Function} `togglePopover` - 팝오버 열림/닫힘 상태를 토글하는 함수
* @returns {Function} `openPopover` - 팝오버 여는 함수
* @returns {Function} `closePopover` - 팝오버 닫는 함수
* @returns {React.RefObject<HTMLElement>} `popoverRef` - 팝오버 요소에 연결할 ref 객체
*
* @example
* import { useRef } from 'react';
* import usePopover from '@hooks/usePopover';
*
* function ProfileComponent() {
* const profileRef = useRef<HTMLDivElement>(null);
* const { isOpen, togglePopover, openPopover, closePopover, popoverRef } = usePopover({
* excludeRefs: [profileRef],
* callback: (isOpen) => console.log('Popover state changed:', isOpen),
* });
*
* return (
* <div ref={profileRef} onClick={togglePopover}>
* <button>프로필</button>
* {isOpen && (
* <div ref={popoverRef}>
* <p>유저이름</p>
* </div>
* )}
* </div>
* );
* };
*/
const usePopover = (options?: {
callback?: (isOpen?: boolean) => void;
excludeRefs?: React.RefObject<HTMLElement>[];
}) => {
const [isOpen, setIsOpen] = useState(false);
const isFirst = useIsFirstMount();
const preservedCallback = usePreservedCallback((isOpen: boolean) => {
if (options?.callback) {
options?.callback(isOpen);
}
});
const togglePopover = useCallback(() => setIsOpen(prev => !prev), []);
const openPopover = useCallback(() => setIsOpen(true), []);
const closePopover = useCallback(() => setIsOpen(false), []);
useEffect(() => {
if (isFirst) {
return;
}
preservedCallback(isOpen);
}, [isOpen]);
const popoverRef = useOutsideClick(() => {
if (isOpen) togglePopover();
}, options);
return { isOpen, togglePopover, openPopover, closePopover, popoverRef };
};
export default usePopover; @rhehfl 님께서 추천해주신 팝오버 상태가 변경될 때 실행되는 Callback 함수를 옵션 객체 안에 추가해봤습니다. 현재 isOpen 상태를 매개변수로 받고 있어, 팝오버 상태 변경 시 특정 동작(이벤트나 콘솔 확인)을 추가적으로 처리할 수 있게 됩니다. const togglePopover = () => setIsOpen(prev => !prev); 또한 기존 코드에서 이렇게 작성했던 것을 const togglePopover = useCallback(() => setIsOpen(prev => !prev), []);
@rhehfl 님께서 만드신 |
@bluetree7878 import { useEffect, useState } from 'react';
/**
* @description 해당 컴포넌트가 초기 랜더링인지 체크해주는 훅
*
* @returns {boolean} 초기 렌더링 여부
*/
const useIsFirstMount = (): boolean => {
const [isFirstMount, setIsFirstMount] = useState(true);
useEffect(() => {
setIsFirstMount(false); // 첫 번째 렌더링 이후 false로 설정
}, []);
return isFirstMount;
};
export default useIsFirstMount 기존 const isFirstMount = useIsFirstMount();
return (
<>
{isFirstMount ? <h1>처음랜더링</h1> : <h1>2번째 랜더링</h1>}
</>
); 같이 사용할 경우 이를 위배하는게 아닌가.. 라는 생각이 들었습니다. |
@bluetree7878 options?: {
callback?: (isOpen?: boolean) => void;
excludeRefs?: React.RefObject<HTMLElement>[];
}
const popoverRef = useOutsideClick(
() => {
if (isOpen) togglePopover();
},
{ excludeRefs: options?.excludeRefs }
); 이러한 형식으로 전달해주는것이 더 좋을 것 같아요!!👍 |
commit - 317b7b5 기존 |
🔗 관련 이슈
#67
📝작업 내용
로그인 안 했을 때
로그인 했을 때
많은 이야기가 오갔지만 보안상 이번 oAuth는 백엔드가 모든 책임을 지기로 하였습니다.
근거: OAuth) 구현 시 책임 분배 관련
따라서 위와 같이 https://[백엔드 주소]/auth/[provider]로 리다이렉트 시켜주면 실제 로그인 시 Access Token, Refresh Token이 발급됩니다. 그러나 쿠키로 통신하였을 때 로컬에서 확인이 따로 불가능하여 로컬에선 테스트용 accessToken 및 refreshToken 생성하여 쿠키에 저장시키도록 하였습니다. (현재 저 부분은 주석을 달아놓았습니다. pull 땡겨오실 때 주석 해제 부탁 드려요.)
🔍 변경 사항
💬리뷰 요구사항 (선택사항)
src/features/quiz/ui/PartNavContainer.tsx
파일은 현재 최신화가 돼있는 상태가 아닙니다. 따라서 이건 Section Part #79 이슈에서 PR이 올라갈 때 다시 확인 부탁드릴게요!