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

같은 경로로 이동시 히스토리 쌓이지 않도록 변경 #750

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

Hain-tain
Copy link
Contributor

⚡️ 관련 이슈

📍주요 변경 사항

1. useCustomNavigate 를 생성하여 같은 경로로 이동시 히스토리가 쌓이지 않도록 변경하였습니다.

@Hain-tain Hain-tain added the refactor 요구사항이 바뀌지 않은 변경사항 label Oct 7, 2024
@Hain-tain Hain-tain added this to the 6차 스프린트 🦴 milestone Oct 7, 2024
@Hain-tain Hain-tain self-assigned this Oct 7, 2024
@Hain-tain Hain-tain changed the title Refactor/575 router history 같은 경로로 이동시 히스토리 쌓이지 않도록 변경 Oct 7, 2024
@Jaymyong66 Jaymyong66 added the FE 프론트엔드 label Oct 7, 2024
Copy link
Contributor

@Jaymyong66 Jaymyong66 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 +7 to +12
return (to: To | number, options?: NavigateOptions) => {
if (typeof to === 'number') {
navigate(to);

return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

단순 궁금증) To 타입의 객체 말고 number로 오는 케이스가 있나요?! useNavigate 의 NavigateFunction타입을 보니까 정확히 To 타입만 인 것 같아서요!

혹은 해당 타입의 delta에 대응을 해준 건가요?

export interface NavigateFunction {
    (to: To, options?: NavigateOptions): void;
    (delta: number): void;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

뒤로가기 navigate(-1) 를 대응해주기 위해서 number 를 추가하였습니다!

@@ -11,7 +10,7 @@ interface Props {
}

const NonmemberAlerter = ({ isOpen, content, toggleModal }: Props) => {
const navigate = useNavigate();
const navigate = useCustomNavigate();
Copy link
Contributor

Choose a reason for hiding this comment

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

이제 useNavigate를 사용해야할 때는 useCustomNavigate로 쓰면 될까요?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

사실 반복적으로 접근할 수 있는 곳이 많지 않기때문에 useNavigate 를 써도 무방한 상황이 존재하긴 합니다. 그러나 일관성을 위해 useCustomNavigate를 쓰면 좋을 것 같다고 생각하여 모든 로직에서 변경하였습니다.

추후 navigate가 필요한 부분이 있다면 useCustomNavigate를 써도 좋을 것 같아요!ㅎㅎ

@@ -0,0 +1,24 @@
import { useNavigate, useLocation, NavigateOptions, To } from 'react-router-dom';

export const useCustomNavigate = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

요런 훅을 만들 때는 JSDoc이나 일반 주석으로 설명을 적어놓으면 해당 custom의 의도를 잊지 않을 것 같아요~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은 생각이네요! 추가해두도록 하겠습니다!

vi-wolhwa
vi-wolhwa previously approved these changes Oct 7, 2024
Copy link
Contributor

@Jaymyong66 Jaymyong66 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@Jaymyong66 Jaymyong66 merged commit f7eb3dc into dev/fe Oct 10, 2024
3 checks passed
@Jaymyong66 Jaymyong66 deleted the refactor/575-router-history branch October 10, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 refactor 요구사항이 바뀌지 않은 변경사항
Projects
Status: Weekend Done
Development

Successfully merging this pull request may close these issues.

3 participants