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

INV-50 와일드카드 서브도메인 rewrite 라우팅 처리 #25

Merged
merged 6 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions next.config.mjs
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
/** @type {import('next').NextConfig} */
const nextConfig = {
redirects: async () => {
return [
{
source: "/",
destination: "/ut",
permanent: true,
},
];
},
};
const nextConfig = {};

export default nextConfig;
3 changes: 3 additions & 0 deletions src/app/(playground)/pg/[subdomain]/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page({ params }: { params: { subdomain: string } }) {
return <div>Subdomain: {params.subdomain}</div>;
}
31 changes: 31 additions & 0 deletions src/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";

export const config = {
matcher: ["/((?!api/|_next/|_static/|_vercel|[\\w-]+\\.\\w+).*)"],
};

export function middleware(request: NextRequest) {
const url = request.nextUrl;
const hostname = request.headers
.get("host")!
.replace(".localhost:3000", `.${process.env.NEXT_PUBLIC_ROOT_DOMAIN}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

NEXT_PUBLIC_ROOT_DOMAIN는 테스트용으로 따로 설정하는건가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

요거 필요없는 로직같아서 제거하였습니다~ dd25c3b

const searchParams = request.nextUrl.searchParams.toString();

const path = `${url.pathname}${
searchParams.length > 0 ? `?${searchParams}` : ""
}`;

const subDomain = hostname.split(".")[0];

switch (true) {
case subDomain !== hostname:
Comment on lines +20 to +21
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 여기서 switch문을 쓴 특별한 이유가 있나요?
현재 케이스에선 if-else 조건문이 더 선언적이고 직관적인 것 같아요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

제 취향일 수 있는데 여기에서는 switch 를 사용하는게 더 직관적인 것 같아요
(예를 들어 특정 서브도메인을 rewrite로직에서 제거하고 싶을 때,(우리가 특정 도메인을 사용할 겨우) case에 추가하여 핸들링)

Copy link
Collaborator

Choose a reason for hiding this comment

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

요런 느낌인가요?
swtich 안에 true가 들어가는게 조금 어색하네요.

swtich (true) {
  case subDomain === "pg":
    return  NextResponse.rewrite(new URL(`/pg`, request.url));
  ...
}

if문으로 한다면,

if (subDomain === "pg") {
  return return  NextResponse.rewrite(new URL(`/pg`, request.url));
}
if (subDomain !== hostname) {
  return return  NextResponse.rewrite(new URL(`/pg/${subDomain}${path}`, request.url));
}
return NextResponse.rewrite(new URL(`${path}`, request.url));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저도 switch (true) 패턴을 처음봤을 때 어색해보였는데, typescript 최신 버전에서도 타입 narrowing 지원을 강화할 만큼 많이 사용하는 패턴이에요!
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-3.html#switch-true-narrowing

Copy link
Collaborator

Choose a reason for hiding this comment

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

오호 switch (true) 패턴이라고 따로 있군요.
복잡한 조건문일 때도 일관된 댑스를 유지하는게 좋고 fall-through 되는 것도 의미가 있네요 좋습니다 👍🏻

Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 에러가 있는데 한번 봐주세요.
hostnameinvi-csdgt32gp-invis-projects.vercel.app 일 때
invi-csdgt32gp-invis-projects !== invi-csdgt32gp-invis-projects.vercel.app 되면서 의도되지 않는 동작을 하네요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

invi-csdgt32gp-invis-projects와 invi-csdgt32gp-invis-projects.vercel.app가 일치하지 않을 때,
vercel.app/pg/invi-csdgt32gp-invis-projects로 rewrite 하는 게 의도된 동작인데 질문을 잘 이해못했어요..

Copy link
Collaborator

Choose a reason for hiding this comment

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

as-is

https://invi-csdgt32gp-invis-projects.vercel.app//pg/subdomain으로 랜딩

to-be

https://invi-csdgt32gp-invis-projects.vercel.app//으로 랜딩

스크린샷

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vercel에서 preview를 제공할 때, wildcard 서브도메인으로 제공해서 /pg/subdomain으로 rewrite 되는 것이 맞지 않나요?
실제 배포때는 wan.invi.my 가 invi.my/pg/wan 요렇게 rewrite되는 것처럼요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

아아 맞네요 vercel preview는 서브도메인이기 때문에 rewrite 되는게 맞네요.

다만 https://invi-csdgt32gp-invis-projects.vercel.app//으로 랜딩되고,
https://wan.invi-csdgt32gp-invis-projects.vercel.app//pg/wan으로 랜딩 되는 것이 저희 의도와 맞으니,
vercel.app 호스트일 경우 예외처리 부탁드려요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

구두로 협의한 내용:

vercel은 preview를 이미 서브도메인 형식으로 제공하고 있고, 아래와 같이 [sub-domain].[preview-domain].vercel.app으로 접근하면 에러가 발생하고 있습니다. preview만을 위해 로직은 변경하기보다, main와 dev에서 의도한대로 작동하여 이대로 머지하겠습니다~

console.log(`/pg/${subDomain}${path}`);
return NextResponse.rewrite(
new URL(`/pg/${subDomain}${path}`, request.url),
);

default:
return NextResponse.rewrite(new URL(`${path}`, request.url));
}
}
Loading