perf: Update to React 19.2, Next.js canary #679
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates React to 19.2 and Next.js to a canary version, along with many other dependencies. The changes are mostly in package.json files. There are also some necessary code adjustments, such as enabling PPR in Next.js and fixing new linting errors that arose from dependency updates.
My review focuses on a few points to improve code quality and maintainability:
- I've suggested removing some commented-out code.
- I've pointed out a case where a file-level
eslint-disableshould be more scoped. - There's a significant change in a TypeScript type that reduces type safety, which I've flagged and suggested a fix for.
Overall, the dependency updates look good and are a good step forward.
| H extends RouteHandler<R> = RouteHandler<R>, | ||
| > { | ||
| handler: H; | ||
| handler: (...args: unknown[]) => Promise<Response> | Response; |
There was a problem hiding this comment.
The type for handler has been made very generic ((...args: unknown[]) => Promise<Response> | Response), which leads to a loss of type safety for route handlers. This was likely done to resolve a TypeScript error after dependency upgrades, but it makes it harder to correctly type handlers. It would be better to use a more specific type from @hono/zod-openapi to maintain type safety.
| handler: (...args: unknown[]) => Promise<Response> | Response; | |
| handler: RouteHandler<R, EnvVitNode>; |
| // export const generateStaticParams = () => | ||
| // vitNodeConfig.i18n.locales.map(locale => ({ locale: locale.code })); |
There was a problem hiding this comment.
Commented-out code should be removed to keep the codebase clean. If this function is no longer needed, please delete it instead of commenting it out.
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect */ | |||
There was a problem hiding this comment.
A file-level eslint-disable is too broad and can hide other potential issues. It's better to use more specific // eslint-disable-next-line comments for the lines that are causing the linting error. This approach is used in other files in this PR, such as packages/vitnode/src/hooks/use-captcha.ts.
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?
How?