chore: 웹 React 19 업그레이드#533
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
전체 변경 사항 요약
Walkthrough
예상 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~60 minutes 추천 리뷰어
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/(home)/_ui/PopularUniversitySection/index.tsx (1)
29-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win아래
Suspensefallback은 현재 구조에선 사실상 안 뜹니다 —Suspense래퍼를 제거하세요
PopularUniversitySection에서PopularUniversityCard를 정적으로 렌더링하고 있어(동기 컴포넌트) 여기서는Suspense가 트리거될 suspend 지점이 없습니다.PopularUniversityCard가 사용하는FallbackImage도useState로 이미지 에러 대체만 처리하며Suspense용 동작은 하지 않습니다.♻️ 정리 방향
- {belowFold.map((university) => ( - <Suspense - key={university.id} - fallback={ - <div className="relative w-[153px]"> - <div className="h-[120px] w-[153px] animate-pulse rounded-lg bg-gray-200" /> - </div> - } - > - <PopularUniversityCard - university={university} - priority={false} - loading="lazy" - fetchPriority="low" - quality={50} // 동적 로딩 이미지는 50으로 최대 압축 - /> - </Suspense> - ))} + {belowFold.map((university) => ( + <PopularUniversityCard + key={university.id} + university={university} + priority={false} + loading="lazy" + fetchPriority="low" + quality={50} // below-fold 이미지는 50으로 최대 압축 + /> + ))}
react의Suspenseimport도 함께 제거하는 게 좋습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(home)/_ui/PopularUniversitySection/index.tsx around lines 29 - 45, The Suspense wrapper around the belowFold.map items should be removed because PopularUniversityCard is rendered synchronously and FallbackImage doesn't suspend; delete the <Suspense ... fallback={...}> wrapper around each PopularUniversityCard in PopularUniversitySection (the map over belowFold) and keep the key on the PopularUniversityCard, and also remove the unused Suspense import from the file; ensure you simply render <PopularUniversityCard university={university} priority={false} loading="lazy" fetchPriority="low" quality={50} /> directly for each item.apps/web/src/app/community/[boardCode]/[postId]/modify/page.tsx (1)
19-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick win1) 수정 페이지도
postId숫자 검증을 넣어주세요.
- Line 25의
Number(postId)결과가NaN일 때 차단 로직이 없습니다.- 라우트 오입력 시 하위 컴포넌트/요청이 비정상 동작할 수 있습니다.
수정 예시
import type { Metadata } from "next"; +import { notFound } from "next/navigation"; @@ const PostModifyPage = async ({ params }: PostModifyPageProps) => { const { boardCode, postId } = await params; + const numericPostId = Number(postId); + if (Number.isNaN(numericPostId)) notFound(); @@ - <PostModifyContent boardCode={boardCode} postId={Number(postId)} /> + <PostModifyContent boardCode={boardCode} postId={numericPostId} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/community/`[boardCode]/[postId]/modify/page.tsx around lines 19 - 25, Validate postId before passing to PostModifyContent: convert params.postId to a number (e.g., const id = Number(postId)) and if Number.isNaN(id) short-circuit (for example call notFound() or render an error/redirect) so the component PostModifyContent and subsequent requests never receive NaN; update the code around params, postId, and the PostModifyContent invocation to use the validated id.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/community/`[boardCode]/[postId]/page.tsx:
- Around line 17-18: params.postIdParam을 Number로 변환만 하고 검증하지 않아 postId가 NaN으로
PostPageContent에 전달될 수 있으니, params와 postIdParam을 사용해 변환 후
Number.isInteger/!Number.isNaN 검사로 유효성 확인하고 유효하지 않을 경우 PostPageContent로 NaN을 넘기지
말고 적절히 처리(예: next/navigation의 notFound() 호출 또는 400 응답/리다이렉트)하도록 변경하세요; 수정 대상
식별자: params, postIdParam, postId, PostPageContent.
In `@apps/web/src/app/mentor/`[id]/page.tsx:
- Around line 16-17: Validate the parsed mentorId after converting from params:
check that Number(id) is a finite integer (not NaN) before passing it to
MentorDetialContent; if validation fails, handle the bad route by returning or
throwing an appropriate response (e.g., invoke notFound() or redirect) so
MentorDetialContent never receives NaN. Update the logic around const mentorId =
Number(id) to perform this check and early-return/route-fail when id is invalid.
In `@apps/web/src/instrumentation-client.ts`:
- Line 10: The configuration currently hardcodes sendDefaultPii: true in
instrumentation-client.ts which risks sending identifiers before user consent;
change the default to sendDefaultPii: false and add a controlled mechanism to
enable it only after explicit consent (e.g., expose a method like
enablePiiCollection or accept a runtime flag in the initialization function used
by the app) so the setting is toggled when the consent signal is received;
update any initialization calls that construct the instrumentation client to
respect this default and call the new enable path after consent.
- Around line 25-32: The lazy-load promise from
Sentry.lazyLoadIntegration("replayIntegration") currently only uses .then(...)
and lacks failure handling; update that chain to append a .catch(...) that
absorbs and logs errors (use Sentry.captureException(error) and/or console.error
with contextual message) so failures to load the replay integration don’t
produce unhandled rejections; ensure the .catch references the same Sentry
symbol and includes enough context (e.g., "replayIntegration lazy load failed")
for debugging.
---
Outside diff comments:
In `@apps/web/src/app/`(home)/_ui/PopularUniversitySection/index.tsx:
- Around line 29-45: The Suspense wrapper around the belowFold.map items should
be removed because PopularUniversityCard is rendered synchronously and
FallbackImage doesn't suspend; delete the <Suspense ... fallback={...}> wrapper
around each PopularUniversityCard in PopularUniversitySection (the map over
belowFold) and keep the key on the PopularUniversityCard, and also remove the
unused Suspense import from the file; ensure you simply render
<PopularUniversityCard university={university} priority={false} loading="lazy"
fetchPriority="low" quality={50} /> directly for each item.
In `@apps/web/src/app/community/`[boardCode]/[postId]/modify/page.tsx:
- Around line 19-25: Validate postId before passing to PostModifyContent:
convert params.postId to a number (e.g., const id = Number(postId)) and if
Number.isNaN(id) short-circuit (for example call notFound() or render an
error/redirect) so the component PostModifyContent and subsequent requests never
receive NaN; update the code around params, postId, and the PostModifyContent
invocation to use the validated id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 83c18adc-cfc3-4520-9be3-9458f2994a33
⛔ Files ignored due to path filters (2)
apps/web/src/app/favicon.icois excluded by!**/*.icopnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
apps/web/next.config.mjsapps/web/package.jsonapps/web/sentry.client.config.tsapps/web/src/app/(home)/_ui/PopularUniversitySection/index.tsxapps/web/src/app/(home)/page.tsxapps/web/src/app/api/revalidate/route.tsapps/web/src/app/community/[boardCode]/[postId]/modify/page.tsxapps/web/src/app/community/[boardCode]/[postId]/page.tsxapps/web/src/app/community/[boardCode]/create/page.tsxapps/web/src/app/community/[boardCode]/page.tsxapps/web/src/app/layout.tsxapps/web/src/app/login/page.tsxapps/web/src/app/mentor/[id]/page.tsxapps/web/src/app/mentor/chat/[chatId]/page.tsxapps/web/src/app/university/[homeUniversity]/page.tsxapps/web/src/app/university/application/apply/page.tsxapps/web/src/app/university/score/page.tsxapps/web/src/app/university/score/submit/gpa/_lib/schema.tsapps/web/src/app/university/score/submit/gpa/page.tsxapps/web/src/app/university/score/submit/language-test/_lib/schema.tsapps/web/src/app/university/score/submit/language-test/page.tsxapps/web/src/app/university/search/page.tsxapps/web/src/components/layout/GlobalLayout/index.tsxapps/web/src/instrumentation-client.tsapps/web/src/instrumentation.tsapps/web/src/proxy.tsapps/web/src/utils/isServerStateLogin.tsapps/web/tsconfig.json
💤 Files with no reviewable changes (1)
- apps/web/sentry.client.config.ts
| const { boardCode, postId: postIdParam } = await params; | ||
| const postId = Number(postIdParam); |
There was a problem hiding this comment.
1) 숫자 파라미터 검증이 빠져서 NaN이 그대로 내려갈 수 있어요.
- Line 18에서 `Number(postIdParam)`만 수행하고 검증이 없습니다.
- 잘못된 URL 입력 시 `PostPageContent`로 `NaN` 전달되어 500/잘못된 요청으로 이어질 수 있습니다.
수정 예시
import type { Metadata } from "next";
+import { notFound } from "next/navigation";
@@
const PostPage = async ({ params }: PostPageProps) => {
const { boardCode, postId: postIdParam } = await params;
const postId = Number(postIdParam);
+ if (Number.isNaN(postId)) notFound();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { boardCode, postId: postIdParam } = await params; | |
| const postId = Number(postIdParam); | |
| import type { Metadata } from "next"; | |
| import { notFound } from "next/navigation"; | |
| const PostPage = async ({ params }: PostPageProps) => { | |
| const { boardCode, postId: postIdParam } = await params; | |
| const postId = Number(postIdParam); | |
| if (Number.isNaN(postId)) notFound(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/community/`[boardCode]/[postId]/page.tsx around lines 17 -
18, params.postIdParam을 Number로 변환만 하고 검증하지 않아 postId가 NaN으로 PostPageContent에
전달될 수 있으니, params와 postIdParam을 사용해 변환 후 Number.isInteger/!Number.isNaN 검사로 유효성
확인하고 유효하지 않을 경우 PostPageContent로 NaN을 넘기지 말고 적절히 처리(예: next/navigation의
notFound() 호출 또는 400 응답/리다이렉트)하도록 변경하세요; 수정 대상 식별자: params, postIdParam, postId,
PostPageContent.
| const { id } = await params; | ||
| const mentorId = Number(id); |
There was a problem hiding this comment.
1) mentorId도 NaN 방어가 필요합니다.
- Line 17에서 숫자 변환만 하고 유효성 체크가 없습니다.
- 비정상 경로 입력이 들어오면 `MentorDetialContent`로 `NaN`이 전달될 수 있습니다.
수정 예시
import type { Metadata } from "next";
+import { notFound } from "next/navigation";
@@
const MentorDetailPage = async ({ params }: MentorDetailPageProps) => {
const { id } = await params;
const mentorId = Number(id);
+ if (Number.isNaN(mentorId)) notFound();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { id } = await params; | |
| const mentorId = Number(id); | |
| import type { Metadata } from "next"; | |
| import { notFound } from "next/navigation"; | |
| // ... other imports | |
| const MentorDetailPage = async ({ params }: MentorDetailPageProps) => { | |
| const { id } = await params; | |
| const mentorId = Number(id); | |
| if (Number.isNaN(mentorId)) notFound(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/mentor/`[id]/page.tsx around lines 16 - 17, Validate the
parsed mentorId after converting from params: check that Number(id) is a finite
integer (not NaN) before passing it to MentorDetialContent; if validation fails,
handle the bad route by returning or throwing an appropriate response (e.g.,
invoke notFound() or redirect) so MentorDetialContent never receives NaN. Update
the logic around const mentorId = Number(id) to perform this check and
early-return/route-fail when id is invalid.
| dsn: process.env.NEXT_PUBLIC_SENTRY_DSN || process.env.SENTRY_DSN || "", | ||
| environment: process.env.NEXT_PUBLIC_SENTRY_ENVIRONMENT || process.env.SENTRY_ENVIRONMENT || "production", | ||
|
|
||
| sendDefaultPii: true, |
There was a problem hiding this comment.
sendDefaultPii를 기본 활성화하면 개인정보 수집 리스크가 큽니다.
Line 10에서 sendDefaultPii: true가 고정되어 있어, 동의 이전에도 식별 정보가 전송될 수 있습니다. 기본값을 false로 두고 동의 신호가 있을 때만 켜는 방식이 안전합니다.
변경 제안
- sendDefaultPii: true,
+ sendDefaultPii: false,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sendDefaultPii: true, | |
| sendDefaultPii: false, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/instrumentation-client.ts` at line 10, The configuration
currently hardcodes sendDefaultPii: true in instrumentation-client.ts which
risks sending identifiers before user consent; change the default to
sendDefaultPii: false and add a controlled mechanism to enable it only after
explicit consent (e.g., expose a method like enablePiiCollection or accept a
runtime flag in the initialization function used by the app) so the setting is
toggled when the consent signal is received; update any initialization calls
that construct the instrumentation client to respect this default and call the
new enable path after consent.
| Sentry.lazyLoadIntegration("replayIntegration").then((replay) => { | ||
| Sentry.addIntegration( | ||
| replay({ | ||
| maskAllText: true, | ||
| blockAllMedia: true, | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
- Replay 지연 로딩 Promise에 실패 처리가 없어 관측이 조용히 깨질 수 있습니다.
Line 25-32는 .then(...)만 있어 로딩 실패 시 unhandled rejection이 발생할 수 있습니다. .catch(...)를 추가해 오류를 흡수/로깅해 주세요.
변경 제안
- Sentry.lazyLoadIntegration("replayIntegration").then((replay) => {
- Sentry.addIntegration(
- replay({
- maskAllText: true,
- blockAllMedia: true,
- }),
- );
- });
+ Sentry.lazyLoadIntegration("replayIntegration")
+ .then((replay) => {
+ Sentry.addIntegration(
+ replay({
+ maskAllText: true,
+ blockAllMedia: true,
+ }),
+ );
+ })
+ .catch(() => {
+ // replay integration load failure should not break runtime
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Sentry.lazyLoadIntegration("replayIntegration").then((replay) => { | |
| Sentry.addIntegration( | |
| replay({ | |
| maskAllText: true, | |
| blockAllMedia: true, | |
| }), | |
| ); | |
| }); | |
| Sentry.lazyLoadIntegration("replayIntegration") | |
| .then((replay) => { | |
| Sentry.addIntegration( | |
| replay({ | |
| maskAllText: true, | |
| blockAllMedia: true, | |
| }), | |
| ); | |
| }) | |
| .catch(() => { | |
| // replay integration load failure should not break runtime | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/instrumentation-client.ts` around lines 25 - 32, The lazy-load
promise from Sentry.lazyLoadIntegration("replayIntegration") currently only uses
.then(...) and lacks failure handling; update that chain to append a .catch(...)
that absorbs and logs errors (use Sentry.captureException(error) and/or
console.error with contextual message) so failures to load the replay
integration don’t produce unhandled rejections; ensure the .catch references the
same Sentry symbol and includes enough context (e.g., "replayIntegration lazy
load failed") for debugging.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6815eca1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "linkifyjs": "^4.3.2", | ||
| "lucide-react": "^0.479.0", | ||
| "next": "^14.2.35", | ||
| "next": "^16.2.6", |
There was a problem hiding this comment.
Next를 ^16.2.6로 올리면 next dev의 기본 번들러가 Turbopack으로 바뀌는데, 현재 설정은 build만 --webpack으로 고정되어 있어 개발 서버에서는 next.config.mjs의 webpack()(여기서 @svgr/webpack을 등록) 구성이 적용되지 않습니다. 이 저장소는 public/svgs/*.svg를 React 컴포넌트로 광범위하게 import하므로, pnpm dev에서 SVG import가 깨지거나 런타임 렌더링 오류가 발생할 수 있습니다.
Useful? React with 👍 / 👎.
…ack-compat chore: next 16 turbopack 빌드 전환
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
관련 이슈
작업 요약
apps/web의 React/ReactDOM을 19.2.6으로 업그레이드했습니다.next build로 전환했습니다.analyze명령에서만 유지하도록 분리했습니다.주요 변경 사항
React 19 / Next 16 호환성
react,react-dom,next,@next/third-parties,@sentry/nextjs,@next/bundle-analyzer등을 Next 16 / React 19 조합에 맞춰 갱신했습니다.params,searchParams를 Promise 기반으로 정리했습니다.cookies()사용부를 Next 16 async API에 맞춰 정리했습니다.@types/react,@types/react-dom을 갱신했습니다.Turbopack 빌드 전환
apps/web의 기본 build script를next build로 변경해 Next 16 기본 Turbopack build를 사용합니다.next.config.mjs에turbopack.rules를 추가해 SVG를 기존처럼 React 컴포넌트로 import할 수 있게 했습니다.webpack설정을 붙이지 않도록 분리했습니다.@svgr/webpack패키지는 이름은 webpack이지만, Turbopack SVG loader 호환 설정에서도 사용되므로 유지합니다.Bundle analyzer 유지
@next/bundle-analyzer는 webpack plugin 기반이라 Turbopack build에서는 리포트를 만들 수 없습니다.analyzescript는ANALYZE=true next build --webpack으로 유지했습니다.next.config.mjs의 webpack 설정은ANALYZE=true일 때만 활성화되도록 조건부로 분리했습니다.Next 16 마이그레이션 정리
middleware를proxy로 이전했습니다.instrumentation.ts/instrumentation-client.ts구조로 정리했습니다.sentry.client.config.ts는 제거했습니다.FileList가 없는 경우에도 성적 제출 스키마가 안전하게 평가되도록 보완했습니다.favicon.ico를 정상 ICO 파일로 교체했습니다.빌드/분석 동작 정리
pnpm --filter @solid-connect/web buildpnpm --filter @solid-connect/web analyzepnpm --filter @solid-connect/web dev특이 사항
@next/bundle-analyzer의 현재 동작 방식 때문입니다.22.x)과 다르면Unsupported enginewarning이 표시될 수 있습니다.apps/web중심 변경이지만 lockfile 변경은 workspace dependency 해석 결과를 함께 포함합니다.리뷰 요청 사항
params,searchParams)이 누락 없이 반영됐는지 확인해주세요.cookies()async 전환과 서버/클라이언트 boundary 변경이 기존 인증 흐름을 깨지 않는지 봐주세요.proxy전환이 배포 환경에서 문제 없는지 확인해주세요.검증
pnpm --filter @solid-connect/web lint:checkpnpm --filter @solid-connect/web typecheckpnpm --filter @solid-connect/web ci:checkpnpm --filter @solid-connect/web buildNext.js 16.2.6 (Turbopack)경로로 통과 확인pnpm --filter @solid-connect/web analyzenext build --webpack경로로 통과 확인.next/analyze/{client,nodejs,edge}.html생성 확인ci:checkbuild