-
-
Notifications
You must be signed in to change notification settings - Fork 0
Upgrade Next 16 #357
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
Upgrade Next 16 #357
Conversation
- Adjust `tsconfig.json` for improved formatting and compatibility (`jsx`, `paths`, etc.). - Modify `package.json` scripts to use Webpack in dev and build commands.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBump Node.js to 24, update Docker base image, upgrade Next.js/MDX to 16, switch MDX plugin references to string identifiers, change dev scripts, adjust pnpm workspace hoist/quoting, update tsconfig and ESLint config, extend .dockerignore, simplify several client components, and revise lint workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Next as Next.js (dev/build)
participant MDX as @next/mdx
participant Resolver as Plugin Resolver
Dev->>Next: run dev/build (webpack/next)
Note right of Next: Runtime now Node 24.x (Dockerfile/.node-version)
Next->>MDX: load .mdx files
MDX->>Resolver: request configured plugins (string IDs)
Resolver-->>MDX: resolve plugins by name -> plugin instances
MDX-->>Next: compiled components
Next-->>Dev: serve/build output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
✅ Lint passedShow output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
9-10: Clarify dev script configuration.The script changes appear counterintuitive:
devnow uses--webpackexplicitlydev:turboruns plainnext dev(which in Next.js 15+ would default to Turbopack in dev mode)This naming may confuse developers expecting
devto use the default bundler. Consider either:
- Swapping the script names to match their actual bundler
- Adding comments to clarify the intentional configuration
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.node-version(1 hunks)Dockerfile(1 hunks)next.config.ts(1 hunks)package.json(4 hunks)pnpm-workspace.yaml(1 hunks)tsconfig.json(3 hunks)
🔇 Additional comments (8)
tsconfig.json (2)
4-8: LGTM on formatting changes.The multiline formatting for
lib,paths, andexcludearrays improves readability without changing functionality.Also applies to: 27-30, 42-44
39-40: LGTM on adding development types.Including
.next/dev/types/**/*.tsprovides better TypeScript support during development.pnpm-workspace.yaml (2)
3-4: LGTM on dependency updates.Quoting
sharpandunrs-resolverimproves YAML consistency. The removal of@tailwindcss/oxidealigns with the dependencies in package.json.
12-14: LGTM on OpenTelemetry hoisting fix.Adding
import-in-the-middleandrequire-in-the-middleto public hoist patterns addresses known instrumentation compatibility issues with Next.js/Turbopack builds.package.json (1)
28-28: LGTM on Next.js 16 upgrade.The coordinated upgrade of
@next/mdx,next, andeslint-config-nextto version 16.0.0 is consistent with the PR objective.Also applies to: 39-39, 59-59
Dockerfile (1)
1-1: node:24-alpine image is available on Docker Hub.Verification confirms the node:24-alpine image exists in multiple stable variants, including the base tag and versions with specific Alpine Linux versions (3.20, 3.21, 3.22). Test your project's dependencies against this image version in your environment to ensure compatibility.
next.config.ts (1)
14-19: Code is correct — string identifiers are required in @next/mdx 16.@next/mdx v16.0.0 requires plugins to be passed as serializable module identifiers (strings) — either "name" or ["name", options] — not direct JS function imports. The configuration in
next.config.tsalready follows this exact format for all plugins (remark and rehype), so no changes are needed..node-version (1)
1-1: Node.js 24.10.0 is a "Current" release, not LTS—verify production readiness.Node.js v24.10.0 was released on October 8, 2025. However, v24 is a "Current" release line, and for production you should prefer an Active LTS release. Confirm this version is appropriate for your production environment before committing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/not-found.tsx (1)
8-8: Remove redundant transition classes and template literal syntax.Since the opacity is now hardcoded to
opacity-100and never changes, thetransition-opacity duration-1000classes serve no purpose. Additionally, the template literal syntax is unnecessary for a static string.Apply this diff to simplify the code:
- <div className={`opacity-100 transition-opacity duration-1000`}> + <div className="opacity-100">Or remove the opacity class entirely if it's already the default:
- <div className={`opacity-100 transition-opacity duration-1000`}> + <div>.dockerignore (1)
169-191: Consider consolidating duplicate ignore patterns.Several patterns added here duplicate existing entries earlier in the file:
**/node_modules(line 187) overlaps withnode_modules/(line 43)**/.env(line 170) overlaps with.env(line 77)**/npm-debug.log(line 188) overlaps withnpm-debug.log*(line 5)- Coverage patterns at lines 24 and 140
While the
**/prefix makes patterns recursive, the earlier patterns already achieve similar coverage. This redundancy doesn't break functionality but increases maintenance overhead.Consider removing the duplicate patterns or consolidating them into a single section. For example:
-node_modules/ +**/node_modulesAnd removing line 187, or keeping the existing patterns and removing the new duplicates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.dockerignore(1 hunks)src/app/not-found.tsx(1 hunks)tsconfig.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tsconfig.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
eslint.config.mjs (3)
9-10: Document or resolve the type suppressions.Two separate type suppressions suggest underlying compatibility issues between the ESLint config packages:
- Line 9:
ts-expect-errorwith unclear reasoning- Line 14: Disabled
no-unsafe-assignmentruleWhile these may be necessary workarounds for package type mismatches, the comment "I have no idea what the problem here is" indicates technical debt that should be addressed.
Consider:
- Investigating the actual type mismatch and documenting why the suppression is needed
- Checking if there are updated
@typespackages that resolve these issues- Updating the comment to explain the specific type incompatibility
Also applies to: 14-15
16-24: Verify whether the globalIgnores are redundant.The
globalIgnores()call specifies paths that are noted as "Default ignores of eslint-config-next". Since you're spreading...nextVitals(fromeslint-config-next/core-web-vitals), these ignores might already be included and could be redundant.Verify whether these can be removed:
#!/bin/bash # Check what ignores are already provided by eslint-config-next echo "=== Inspecting eslint-config-next/core-web-vitals default config ===" node -e " const config = require('eslint-config-next/core-web-vitals'); console.log('Config structure:', JSON.stringify(config, null, 2).substring(0, 500)); if (Array.isArray(config)) { const ignoresConfig = config.find(c => c.ignores); console.log('\nIgnores found:', ignoresConfig); } "
8-10: Imports are valid — remove or document the @ts-expect-error.eslint/config and eslint-config-next/core-web-vitals import successfully and npx eslint --print-config loads the config (ESLint 9.38.0). Remove the line "// @ts-expect-error I have no idea what the problem here is" (eslint.config.mjs, lines 8–10); either fix the underlying type error or replace it with a narrowly scoped suppression and a short comment explaining why it’s necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
eslint.config.mjs(1 hunks)src/app/components/NavLink.tsx(1 hunks)src/app/components/client-date.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- src/app/components/client-date.tsx
🔇 Additional comments (1)
src/app/components/NavLink.tsx (1)
11-12: LGTM! Excellent simplification.Removing the
useState/useEffectpattern in favor of direct synchronous computation is the right approach here. SinceusePathname()returns the current value immediately, there's no need for state management or effects to track the path.
✅ Lint passedShow output |
Summary by CodeRabbit
Chores
Refactor