refactor: apply vercel-react-best-practices review#5
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR applies “vercel-react-best-practices” recommendations by moving article search/filtering to server-side routing, aligning microCMS fetching with Next.js caching, and trimming client-only dependencies.
Changes:
- Refactor microCMS data fetching to use Next.js Data Cache + React
cache()with revalidation and cache tags. - Replace client-side SWR/axios-based search with a dedicated
/searchApp Router page usingsearchParams. - Reduce client bundle and dependencies by removing SWR/axios/simplebar/zod usage and adjusting analytics loading.
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/index.ts | Extends Article with createdAt / publishedAt. |
| src/libs/microcms/client.ts | Adds caching/revalidate/tag behavior for list/detail fetches. |
| src/features/home/components/useArticleSearch.ts | Removes client-side SWR hook (deleted). |
| src/features/home/components/TopPage.tsx | Converts to server-driven search/category navigation via links and form action. |
| src/features/home/components/ArticleCard.tsx | Removes memo, tweaks CTA label. |
| src/features/common/components/AnalyticsProvider.tsx | Adds client-only analytics wrapper via dynamic import. |
| src/features/articles/utils/parser.tsx | Removes comments only; logic unchanged. |
| src/features/articles/components/Code.tsx | Removes client-only SimpleBar usage; uses native overflow container. |
| src/features/articles/components/Code.module.css | Adds native scrollbar styling. |
| src/features/articles/components/ArticleHeader.tsx | Updates date label text. |
| src/app/search/page.tsx | Adds new server-rendered search page using searchParams. |
| src/app/page.tsx | Adds revalidate and updates metadata description. |
| src/app/layout.tsx | Moves analytics to provider; adjusts external link handling. |
| src/app/not-found.tsx | Updates copy text. |
| src/app/loading.tsx | Updates copy text. |
| src/app/error/page.tsx | Updates copy text. |
| src/app/articles/[id]/page.tsx | Adds revalidate and metadata copy tweak. |
| src/app/api/list/route.ts | Removes API endpoint used by SWR search (deleted). |
| src/app/[others]/page.tsx | Updates copy text. |
| package.json | Removes unused deps (axios/swr/simplebar/zod/etc.). |
| package-lock.json | Lockfile updated accordingly. |
| README.md | Updates project description/setup/docs. |
| .gitignore | Adds ignores for .codex/ and skills-lock.json. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| overview: string | ||
| category: string | ||
| createdDate: string | ||
| createdAt: string | ||
| publishedAt: string |
There was a problem hiding this comment.
Article 型が詳細用の detail や createdAt / publishedAt を必須にしていますが、一覧取得では fields でそれらを含めない指定になっています（例: Home/Search では id,title,overview,... のみ）。microCMS の fields 指定で実際に返るフィールドが絞られる場合、ランタイムではこれらが undefined になり得るので、(1) 一覧用の型（例: ArticleSummary）を別に作る/Pick を使う、または (2) これらのプロパティを optional にして型を実態に合わせる、のどちらかに寄せた方が安全です。
| export const getArticleList = cache(async (fieldNames?: string, keyword?: string) => { | ||
| const normalizedKeyword = keyword?.trim() | ||
|
|
||
| try { | ||
| return await client.getList<Article>({ |
There was a problem hiding this comment.
getArticleList が fields 引数で取得フィールドを可変にしている一方で、返却型を常に client.getList<Article> に固定しているため、呼び出し側が「Article の全プロパティがある」前提で書けてしまいます。fields の指定と整合するように、一覧用の型を導入するか、getArticleList<T>() のようにジェネリクスで返却型を受け取れる形にすると型安全になります。
| export const getArticleList = cache(async (fieldNames?: string, keyword?: string) => { | |
| const normalizedKeyword = keyword?.trim() | |
| try { | |
| return await client.getList<Article>({ | |
| export const getArticleList = cache(async <T = Article>(fieldNames?: string, keyword?: string) => { | |
| const normalizedKeyword = keyword?.trim() | |
| try { | |
| return await client.getList<T>({ |
| <a | ||
| href={item.url} | ||
| target="_blank" | ||
| rel="noreferrer" |
There was a problem hiding this comment.
外部リンクを target="_blank" で開く場合、rel は noopener を含めないと window.opener 経由の reverse tabnabbing リスクがあります。noreferrer だけでもブラウザによっては抑止されますが、明示的に rel="noopener noreferrer" にしておくのが安全です。
| rel="noreferrer" | |
| rel="noopener noreferrer" |
apply vercel-react-best-practices review