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

HOT 페스티벌 리스트 페이지 구현 #29

Merged
merged 38 commits into from
Aug 18, 2024
Merged

Conversation

froggy1014
Copy link
Collaborator

@froggy1014 froggy1014 commented Aug 17, 2024

PR 타입 ( 하나 이상의 PR 타입을 선택해주세요 )

FIle Changes 가 많아서 죄송합니다.. 서버컴포넌트로 싹다 바꾸다보니 잦라한 변화가 많아졌네요..

close #26

  • 기능 추가
  • 버그 수정
  • 의존성, 환경 변수, 빌드 관련 코드 업데이트
  • 기타 사소한 수정

개요

처음 Main쪽만 Server Side Fetching으로 페이지네이션은 React-Query Prefetching으로 구현을 할려고 했습니다.

그런데 저희 디자인이 뭐.. 리스팅에서 크게 상호작용도 없고 그냥 서버 컴포넌트로 다 커버할 수 있을 것 같아서 서버 컴포넌트로 전부 구현을 했습니다.

간단한 페이지네이션말고 기능이 없다고 할까나요.

image

그래서 리액트 쿼리 관련해서 만진 코드가 좀 있습니다.

export async function getHotFestival(
  params: PaginationParamter = defaultParams,
) {
  const endpoint = ENDPOINT.mostlike;

  const { data } = await instance.get<HostFestivalData>(
    generateUrlWithParams(endpoint, params),
    {
      next: {
        revalidate: 85600, // 여기
        tags: hotFestivalKeys.all,
      },
    },
  );
  return data;
}

cache revalidate는 하루로 잡아놨고,

tag를 달아서 만약 서버쪽에서 onDand-revalidate로 쏘게끔 할 수 있게하기 위해서 라우트를 만들어놨습니다.

import { revalidateTag } from "next/cache";
import type { NextRequest } from "next/server";

export async function GET(request: NextRequest) {
  const tag = request.nextUrl.searchParams.get("tag");

  if (!tag) {
    return Response.json(
      { revalidated: false, message: "tag is required" },
      { status: 400 },
    );
  }

  revalidateTag(tag);
  return Response.json({ revalidated: true, now: Date.now() }, { status: 200 });
}

추후에 시크릿 키 같은거로 하나 만들어서 막아놔야할 거 같긴한데 지금은 다 열려있긴하네요. 저도 한번 테스트 해보고싶어서..

그리고 페이지네이션 같은 경우는 nuqs라는 쿼리스트링을 서버, 클라이언트 쪽에서 양쪽다 추상화 잘 되어있는 좋은 라이브러리가 있어서 활용해서 구현하였습니다.

https://nuqs.47ng.com/playground/pagination

코드 리뷰시 참고 사항

아 생각해보니까 API가 완성이 안되서 빌드가 안되네요 하ㅏ하;;;

목업으로 빌드하고 확인하셔야할 것 같습니다.

테스트 환경변수

NEXT_PUBLIC_STAGE=test
NEXT_PUBLIC_BASE_URL=https://odiga.shop/api/v1

cmd

pnpm install 
pnpm run build:mock
pnpm run start

빌드하시면 SSG로 메인페이지가 생성이 될거구요,

http://localhost:3000/api/revalidate?tag=hostFestival 여기로 쏘시면 OnDemand Revalidate가 되서
다른 값 가져와질거예요.

그리고 revalidate값을 몇초로 줄여서 변경해서 넣어보면 그 간격대로 다시 페칭해옵니다. ( 새로고침하면 )

아마 인터렉션이 많이 없거나, 유저로 인해서 바뀌는 데이터가 아니라면 이런식으로 진행할 것 같은데.. 의견이 있으시면 감사합니다.

Copy link

vercel bot commented Aug 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fiesta ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 18, 2024 9:52am

@froggy1014 froggy1014 linked an issue Aug 17, 2024 that may be closed by this pull request
@froggy1014 froggy1014 added enhancement New feature or request skip-build labels Aug 17, 2024
Copy link
Member

@seokzin seokzin left a comment

Choose a reason for hiding this comment

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

nuqs 라이브러리 좋네요! 좋은 정보 감사합니다

자세하게 리뷰하진 못했지만, 간단한 리뷰 남기고 가요!

여담이지만 커밋 컨벤션이나 문서 작성한 것을 보니 글을 정말 잘 정리하시네요 👍

Comment on lines 3 to 4
const useDayjs = () => {
const getDday = (date: string) => {
Copy link
Member

Choose a reason for hiding this comment

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

P3

로직이 hook보단 util에 가까운 것 같아요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@seokzin

아..아 ! 맞는것 같습니다 ㅋㅋㅋㅋ.. 훅으로 뺄 필요가 없겠네요

return (
<div className="fixed bottom-0 mb-[40px] flex w-full max-w-[450px] items-center justify-center gap-[16px]">
<Link
href={pageURL(currentPage - 1)}
Copy link
Member

Choose a reason for hiding this comment

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

P4

Pagination 컴포넌트만 보고 질문 남기는 것인데
currentPage가 0이 될 수 있나요? 그렇다면 이 로직 문제 없을까요?

Copy link
Collaborator Author

@froggy1014 froggy1014 Aug 18, 2024

Choose a reason for hiding this comment

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

백엔드 측에서 그렇게 던져주고 있어서 그것에 맞춰 일단 작업을 했습니다만

나중에 Pagination 관련해서.. 한번 나중에 논의드리려구 합니다.

currentPage === 0 ? "pointer-events-none opacity-50" : "",

이렇게 이벤트 안받도록해놨습니다.

Comment on lines 32 to 37
{hotFestivals.content.splice(0, 2).map((v) => (
<TrendFestivalCard
key={v.festivalId}
href={`/featival/${v.festivalId}`}
festival={v}
/>
Copy link
Member

Choose a reason for hiding this comment

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

P4

다른 mapping 로직에선 매핑값 네이밍이 festival 등으로 명시 되어있던데
이 부분도 일관성 있게 hotFestival로 네이밍 개선하는 것이 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일단 리스트 목록들은 모두 같은 festivalModel을 갖고 있어서 festival로 수정하도록 하겠습니다 ! ㅎㅎ

src/lib/PrefetchQueryProvider.tsx Outdated Show resolved Hide resolved
Copy link
Member

@Zero-1016 Zero-1016 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다...! 코드 보고 많이 배워가네요👍

Copy link
Member

@saseungmin saseungmin left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!

src/apis/festivals/hotFestival/hotFestival.ts Outdated Show resolved Hide resolved
src/app/(home)/_components/FestivalHot.tsx Outdated Show resolved Hide resolved
src/components/Pagination/Pagination.tsx Show resolved Hide resolved
@punchdrunkard
Copy link
Member

아 생각해보니까 API가 완성이 안되서 빌드가 안되네요 하ㅏ하;;;

바로 머지하겠습니다...........

@punchdrunkard
Copy link
Member

아 맞다 더미로 넣어둔 이미지들때문에 에러가 나는거같아요

image

아마 나중에 데이터 채워넣을 때도 거의 이 url로 채워넣을 것 같아서

  • https://kfescdn.visitkorea.or.kr/
  • http://tong.visitkorea.or.kr/

nextConfig 에 이미지 도메인에 저거랑

s3 도메인

  • https://fiesta-image.s3.ap-northeast-2.amazonaws.com/

이렇게 추가해주세요!

@punchdrunkard
Copy link
Member

image

- 갯수가 1자리 수에서는 2개, 2자리 수에서는 1개, 3자리 수에서는 2개로 보입니다!

@froggy1014
Copy link
Collaborator Author

froggy1014 commented Aug 18, 2024

image

- 갯수가 1자리 수에서는 2개, 2자리 수에서는 1개, 3자리 수에서는 2개로 보입니다!

어 ... 일단 해당 축제들이 작년에 일어났거나 startDate 기준으로 오늘보다 느리면 이미 지나간 축제다보니까

prefix: D- day: -{day}

이렇게 나와서 생긴거같은데 일단 기간안에 있으면 진행 중으로 표시하도록 수정하겠습니다. 그런데 과거...에 진행했던 행사같은 경우는 안나와야하는게 아닌가 싶은데 어떻게 생각하시나요.

일단 이렇게 수정할게요.

image

@punchdrunkard

@froggy1014 froggy1014 merged commit f687ec2 into develop Aug 18, 2024
4 checks passed
@froggy1014 froggy1014 deleted the feature/hot-festival branch August 18, 2024 09:53
@punchdrunkard
Copy link
Member

이렇게 나와서 생긴거같은데 일단 기간안에 있으면 진행 중으로 표시하도록 수정하겠습니다. 그런데 과거...에 진행했던 행사같은 경우는 안나와야하는게 아닌가 싶은데 어떻게 생각하시나요.

반영하겠습니다~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request skip-build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] Hot Festival
5 participants