Skip to content
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

Feat/kakao login #134

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

josworks27
Copy link
Member

@josworks27 josworks27 commented Jul 31, 2022

Description

  • 카카오 로그인 구현

Help Wanted 👀

Related Issues

Checklist ✋

  • PR 타이틀을 {PR type}: {PR title}로 맞췄습니다. (type 예시: feat | fix | BREAKING CHANGE | chore | ci | docs | style | refactor | perf | test) (참고: conventional commits)
  • 모든 변경점들을 확인했으며 적절히 설명했습니다.
  • 빌드와 테스트가 정상적으로 수행됨을 확인했습니다. (yarn build, yarn test)
  • 깃헙 이슈를 연결하겠습니다. (커밋에 resolve #이슈넘버 적거나 PR생성 후 Linked Issue 추가)

- 리팩터링 필요
- ProtectedRoute 추가
- useAccessToken 추가
- useLocalStorage 추가
lifeisegg123
lifeisegg123 previously approved these changes Jul 31, 2022
Copy link
Member

@lifeisegg123 lifeisegg123 left a comment

Choose a reason for hiding this comment

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

👍

Comment on lines +10 to +20
export const ProtectedRoute: FC<ProtectedRouteProps> = ({
isAllowed,
redirectTo,
children,
}) => {
if (!isAllowed) {
return <Navigate replace to={redirectTo} />
}

return children
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export const ProtectedRoute: FC<ProtectedRouteProps> = ({
isAllowed,
redirectTo,
children,
}) => {
if (!isAllowed) {
return <Navigate replace to={redirectTo} />
}
return children
}
export const ProtectedRoute: FC<ProtectedRouteProps> = ({
isAllowed,
redirectTo,
element,
path
}) => {
return (
<Route
path={path}
element={isAllowed ? element : <Navigate replace to={redirectTo} />}
/>
}

이런 구현은 어떨까요??
사용하는 곳에서 아래처럼 쓸 수 있을 것 같아요!

 <Routes>
  <Route path="/" element={<MapAppContainer />} />
  <Route path="/login" element={<Login />} />
  <ProtectedRoute
    path="/protectePath"
    isAllowed={isAvailable} 
    redirectTo="/login"
    element={
        <div>Protected Page</div>
    }
  />
</Routes>

Comment on lines +32 to +43
const {
data: {
signIn: {accessToken, expiredAt},
},
} = await urqlClient
.mutation(SignInDocumnet, {
input: {
code: access_token,
type: 'KAKAO',
},
})
.toPromise()
Copy link
Member

Choose a reason for hiding this comment

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

useMutation훅 사용하지 않고 이렇게 처리하신 이유가 있을까요??? 👀👀👀

- 함수표현식으로 변경
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants