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(app, home): 홈페이지 내부를 React 컴포넌트로 분리하고, 공용 레이아웃으로 감싸도록 페이지 내부를 리팩토링합니다. #74

Merged
merged 14 commits into from
Sep 9, 2023

Conversation

innocarpe
Copy link
Collaborator

📌 이슈 링크


📖 작업 배경

  • JWT 인증 쪽 개발을 잠시 홀딩하고, 홈페이지 내부를 React 컴포넌트로 분리하고 내부를 정리하는 작업을 먼저 진행하려고 합니다.

🛠️ 구현 내용

  • 홈페이지 내부를 각각의 React 컴포넌트로 만들고 전부 분리합니다.
  • 홈페이지 내부 중 컨텐츠 영역을 HomeFeedContents 로 정의하고 분리합니다.
  • 홈페이지 최상위 Wrapper 영역을 HomePageContainer 로 정의하고 분리합니다.
  • 홈페이지를 비롯 각 페이지에서 공통으로 사용할 영역을 Layout 컴포넌트로 분리합니다. (@InSeong-So 님과 페어했습니다 💪)
  • Router 에서 라우팅을 할 때 Layout 컴포넌트로 감싸서 라우팅을 하도록 해 모든 페이지에서 네비바와 푸터를 공용으로 사용할 수 있도록 합니다.

💡 참고사항

  • 평소 작업 방식이라면 이것보다 훨씬 더 작은 단위로 나눌 것 같기는 한데, 아직은 코딩 자체가 익숙하지 않다 보니 작업 단위가 커지는군요 😂
  • 일단 컴포넌트 분리부터 여러 페이지에 대해 최대한 많이 해두는 방향의 작업도 나쁘지 않겠다는 생각이 들었습니다.
  • JWT 핸들링 쪽도 개발도 하긴 해야 할 텐데 어떤 것부터 할지 고민이네요 ㅎㅎ
  • 여기까지 개발하면서 이제 폴더 구조에 대한 고민을 조금씩 하기 시작했는데, @InSeong-So 님과 이런저런 방향으로 얘기 나눠본 결과 지금 구조에서 아주 크게 바꿀 필요는 없을 것 같고 조금씩 바꾸어 나가는 수준으로만 일단 가도 될 것 같네요!

🖼️ 스크린샷

리액트로 전부 분리한 다음 동일한 결과물임을 확인하는 스크린샷입니다.

Screenshot 2023-09-07 at 10 38 52 PM

@innocarpe innocarpe added feature A new feature chore Updating build tasks, package manager configs, etc; no production code change labels Sep 7, 2023
@@ -22,6 +22,7 @@ module.exports = {
'no-undef': 0,
'prettier/prettier': 2, // Means error
'react/react-in-jsx-scope': 'off',
'react/prop-types': 'off', // TypeScript 에서 이미 컴포넌트의 props를 검증하기 위해 타입 체크를 제공
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 설정이 없으니 props 쪽에서 타입 관련 이슈가 발생하네요. 찾아보니 이걸 끄면 된다고 해서 이번 PR부터 적용했습니다.

Copy link
Member

Choose a reason for hiding this comment

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

prop-types를 직접 할당하는 코드가 있을 때 해당 린트 에러가 발생할텐데... 이상하네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

prop-types를 직접 할당하는 코드가 있을 때 해당 린트 에러가 발생할텐데... 이상하네요!

타입 설정 없이 props 선언만 빼두었는데도 그렇더라구요..! ㅠ

Copy link
Member

Choose a reason for hiding this comment

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

어랏,,, 저두 요 타입 이슈는 발생하지 않는데 스터디 날에 함께 확인해봐요!! 궁금궁금

<Layout>
<RouterProvider router={router} />;
</Layout>
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

라우터에서 네비바/푸터를 가진 Layout 으로 감싸서 라우팅을 하도록 변경했습니다.

Copy link
Member

Choose a reason for hiding this comment

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

이런 Layout은 중첩 라우팅 + Outlet을 이용하여 표현해도 좋아보여요!
이 경우에는 모든 페이지가 Layout에 적용되어 각 페이지마다 다른 레이아웃을 적용할 때 어려움이 있을 것 같다는 생각이 들기 때문이에요!

제가 이전에 작성했던 코드 참고가 될까 하여 작성해둘게요 :)

https://github.com/areumsheep/react-payments/blob/a9ea3ecb052ebc87d0e1c4a029ba06050b186879/src/routes/index.tsx#L6

<ReactRouterRoutes>
      <Route element={<Layout />}>
        <Route path="/card-list" element={<CardListPage />} />
        <Route path="/card-name-input" element={<InputCardNamePage />} />
        <Route path="/card-registration" element={<RegistrationCardPage />} />
        <Route path="*" element={<Navigate replace to="/card-list" />} />
      </Route>
</ReactRouterRoutes>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@areumsheep 중첩 라우팅 과 Outlet 둘 다 어떤 뜻인지 몰라서 공유해주신 코드를 보고도 의도하신 부분을 이해를 못했습니다 ㅋㅋ; 나중에 기회 되면 조금 더 알려주시면 좋을 것 같아요!

-> 찾아보니 요런 글이 있네요 ㅎㅎ 제가 아직 이해를 다 못한 부분이, 현재의 제 방식이나 아름님 코드와 같이(중첩 라우팅) 바꾼다고 하더라도 두 방향 다 Layout 이 공용으로 적용되어서 다른 레이아웃(각 페이지 부분이 아닌 Layout 쪽 상하단 영역을 의미하시는 거겠죠?) 적용할 때의 어려움은 같지 않나는 생각이 들어서요!

https://velog.io/@reasonz/2022.07.14-%EB%A6%AC%EC%95%A1%ED%8A%B8-%EB%9D%BC%EC%9A%B0%ED%84%B0-%EC%A4%91%EC%B2%A9-%EB%9D%BC%EC%9A%B0%ED%8C%85-nested-routes-outlet

</div>
</footer>
</>
<HomePageContainer>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아직 예상이기는 하지만 HomePageContainer 에서 스타일 관련 처리를 하도록 만들 것 같아요. 아마도 styled component 쓰지 않을까 싶습니다.

</>
<HomePageContainer>
<HomeBanner />
<HomeFeedContents />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이렇게 두 개로 분리하는게 맞을까 아직 확신이 없기는 합니다. 페이지 구조를 어떻게 만드는지 아직 경험이 많이 없는지라 현재의 제 지식 수준에서 할 수 있는 정도까지만 분리해둔 느낌입니다 ㅎ

@@ -22,6 +22,7 @@ module.exports = {
'no-undef': 0,
'prettier/prettier': 2, // Means error
'react/react-in-jsx-scope': 'off',
'react/prop-types': 'off', // TypeScript 에서 이미 컴포넌트의 props를 검증하기 위해 타입 체크를 제공
Copy link
Member

Choose a reason for hiding this comment

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

prop-types를 직접 할당하는 코드가 있을 때 해당 린트 에러가 발생할텐데... 이상하네요!

Comment on lines +1 to +11
interface ArticlePreviewProps {
authorProfileLink: string;
authorProfileImageUrl: string;
authorName: string;
publishDate: string;
likeCount: number;
articleLink: string;
articleTitle: string;
articleDescription: string;
tags: string[];
}
Copy link
Member

Choose a reason for hiding this comment

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

타입스크립트를 다룰 때 조금 고통스러운 첫 관문이 interface/type 별칭을 어떤 걸로 가져가야 하는가? 입니다.

앞서 말씀드리자면 interface <> type은 완벽하게 호환되나, 사용처를 명확하게 구분하는 컨벤션을 가져가는 것이 좋습니다.

다른 파일에서 type으로 선언한 부분이 보였는데요, interface의 기준을 세우신 것이 있을까요? 🔍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

타입스크립트를 다룰 때 조금 고통스러운 첫 관문이 interface/type 별칭을 어떤 걸로 가져가야 하는가? 입니다.

앞서 말씀드리자면 interface <> type은 완벽하게 호환되나, 사용처를 명확하게 구분하는 컨벤션을 가져가는 것이 좋습니다.

다른 파일에서 type으로 선언한 부분이 보였는데요, interface의 기준을 세우신 것이 있을까요? 🔍

아쉽게도 아직은 기준이 없습니다! 아마도 여러 코드들 참고하면서 제 프로젝트 내부적으로 이것저것 사용하게 되는 상황인 것 같고, 두 개의 차이를 찾아보았을 때에도 그래서 뭘 써라 라고 하는 부분은 여전히 애매하더라구요. 이건 추후 오프라인에서 한 번 여쭤보고 싶습니다 ㅎㅎ

Comment on lines +13 to +23
const ArticlePreview: React.FC<ArticlePreviewProps> = ({
authorProfileLink,
authorProfileImageUrl,
authorName,
publishDate,
likeCount,
articleLink,
articleTitle,
articleDescription,
tags,
}) => (
Copy link
Member

Choose a reason for hiding this comment

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

이러한 props 할당은.... 현재 상황에서는 어쩔 수 없어보이지만요 🥲

모든 값이 required 하기 때문에 차라리 flat한 프로퍼티 값들 보다는 article이라는 객체로 넘기는 것이 좋을 것 같다는 생각도 드네요.

너무 많은 props를 제 3자가 핸들링해야 하는 상황은 썩 유쾌하지 않기 때문에, 개수를 줄이는 것이 효율적이게 됩니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이러한 props 할당은.... 현재 상황에서는 어쩔 수 없어보이지만요 🥲

모든 값이 required 하기 때문에 차라리 flat한 프로퍼티 값들 보다는 article이라는 객체로 넘기는 것이 좋을 것 같다는 생각도 드네요.

너무 많은 props를 제 3자가 핸들링해야 하는 상황은 썩 유쾌하지 않기 때문에, 개수를 줄이는 것이 효율적이게 됩니다!

아하 props 로 넘기는 개수 자체를 줄이는 방식이군요! 이건 추후 개발할 때 반영해보겠습니다 🙏

Comment on lines +11 to +13
<a key={tag} href="" className="tag-pill tag-default">
{tag}
</a>
Copy link
Member

Choose a reason for hiding this comment

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

href 부분의 공백은 추후에 놓치실 수 있으니 꼭 유념해주셔야 합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

href 부분의 공백은 추후에 놓치실 수 있으니 꼭 유념해주셔야 합니다!

오 공백으로 만들어 버렸군요 나중에 제대로 수정해두겠습니다!

@InSeong-So InSeong-So merged commit 460d447 into team6/innocarpe Sep 9, 2023
@InSeong-So InSeong-So deleted the carpe/home-extract-component branch September 9, 2023 08:44
Copy link
Member

@areumsheep areumsheep left a comment

Choose a reason for hiding this comment

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

우와 레이아웃까지 분리를 진행하셨다니 고생 많으셨습니다!!

@@ -22,6 +22,7 @@ module.exports = {
'no-undef': 0,
'prettier/prettier': 2, // Means error
'react/react-in-jsx-scope': 'off',
'react/prop-types': 'off', // TypeScript 에서 이미 컴포넌트의 props를 검증하기 위해 타입 체크를 제공
Copy link
Member

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 +15
export const Layout = ({ children }: LayoutProps) => {
return (
<>
<Navbar />
{children}
<Footer />
</>
);
};
Copy link
Member

Choose a reason for hiding this comment

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

{children} 부분을 react-router-dom의 <Outlet />을 사용할 수 있답니다~!

<Layout>
<RouterProvider router={router} />;
</Layout>
);
Copy link
Member

Choose a reason for hiding this comment

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

이런 Layout은 중첩 라우팅 + Outlet을 이용하여 표현해도 좋아보여요!
이 경우에는 모든 페이지가 Layout에 적용되어 각 페이지마다 다른 레이아웃을 적용할 때 어려움이 있을 것 같다는 생각이 들기 때문이에요!

제가 이전에 작성했던 코드 참고가 될까 하여 작성해둘게요 :)

https://github.com/areumsheep/react-payments/blob/a9ea3ecb052ebc87d0e1c4a029ba06050b186879/src/routes/index.tsx#L6

<ReactRouterRoutes>
      <Route element={<Layout />}>
        <Route path="/card-list" element={<CardListPage />} />
        <Route path="/card-name-input" element={<InputCardNamePage />} />
        <Route path="/card-registration" element={<RegistrationCardPage />} />
        <Route path="*" element={<Navigate replace to="/card-list" />} />
      </Route>
</ReactRouterRoutes>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Updating build tasks, package manager configs, etc; no production code change feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants