fix(next): remove runtime import form 'next/server'

[ymc9]

Summary by CodeRabbit

• New Features

  • Added a nested CLI task "db" with "push" (completed) and "seed" (pending) subtasks.

• Database

  • PostgreSQL multi-schema option enabled/checked in configuration.

• Chores

  • Internal server request handling adjusted for standard Web Response behavior.
  • Dependency bump for internal tooling.

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Replaced NextResponse usage with standard Web Response in the Next app route handler (signature changed and query extraction added), updated package dependency ts-japi in packages/server/package.json, and added nested CLI database tasks plus PostgreSQL multi-schema flag in TODO.md.

Changes

Cohort / File(s)	Summary
Project documentation TODO.md	Added nested CLI subtask db with children push (completed) and seed (incomplete); marked PostgreSQL "Multi-schema" as checked.
Next app route handler packages/server/src/adapter/next/app-route-handler.ts	Replaced NextResponse usages with standard Web Response; updated exported handler return type from Promise<NextResponse> to Promise<Response>; removed NextResponse import, kept NextRequest as a type import; added URL parsing to extract query parameters; converted all response/error paths to Response.json.
Server package manifest packages/server/package.json	Bumped dependency ts-japi from ^1.12.0 to ^1.12.1.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Areas needing extra attention:
  • packages/server/src/adapter/next/app-route-handler.ts: verify behavior parity after switching to Web Response, ensure headers/status codes and streaming behaviors are preserved, and validate query extraction across edge/runtime environments.
  • Error handling paths: confirm consistent error shapes and logging.
  • packages/server/package.json: confirm dependency bump has no transitive impact.

Possibly related PRs

• feat(server): migrate next.js server adapter #335 — Modifies the same Next app-route-handler; related to replacing NextResponse with Web Response and signature changes.
• feat(server): migrate nuxt adapter #342 — Also updates the app route handler and error/response handling; overlapping changes likely.

Poem

🐇 I hopped through code at break of day,
Swapped responses for the Web's clear way.
A TODO note, a DB task penned,
A tiny bump — dependencies mend.
Hooray — the rabbit hops again! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title mentions removing runtime import from 'next/server', but the actual changes show replacing NextResponse with standard Web Response and updating a dependency version, which is only partially related to the stated title.	Update the PR title to better reflect the main changes: consider 'fix(next): replace NextResponse with standard Web Response' or align the code changes with the title's stated objective.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/next-server

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @ymc9's task —— View job

---

Pull Request Review

I'll review this PR focusing on code quality, best practices, potential bugs, performance, security, and test coverage.

Todo List:

• Read and analyze the changed files
• Review ESM compatibility changes
• Check for potential bugs or issues
• Evaluate performance implications
• Assess security concerns
• Review test coverage needs
• Provide comprehensive feedback

Analysis of Changes

Files Changed:

1. packages/server/src/adapter/next/app-route-handler.ts - Added .js extension to Next.js import
2. TODO.md - Updated task completion status (unrelated to ESM fix)

The core change is on line 2 of app-route-handler.ts:

- import { NextRequest, NextResponse } from 'next/server';
+ import { NextRequest, NextResponse } from 'next/server.js';

Review Feedback

✅ Code Quality & Best Practices

• Good: The change follows ES module import conventions by explicitly specifying the .js extension
• Context: The package.json shows "type": "module" and TypeScript config uses "moduleResolution": "bundler", making this change appropriate for ESM compatibility
• Improvement: This aligns with Node.js ESM requirements for explicit file extensions

⚠️ Consistency Issues

• Issue Found: Inconsistent import patterns in the codebase
• Location: packages/server/src/adapter/next/index.ts:4

  import type { NextRequest } from 'next/server'; // Missing .js extension

• Recommendation: Update this import for consistency:

  import type { NextRequest } from 'next/server.js';

✅ Security Concerns

• Assessment: No security implications
• Reasoning: This is a purely syntactic change to import statements with no functional modifications

✅ Performance Considerations

• Assessment: Minimal to no performance impact
• Details: ESM imports with explicit extensions can slightly improve module resolution performance by avoiding ambiguous lookups

⚠️ Potential Bugs

• Low Risk: The change itself is safe, but the inconsistency could lead to:
  • Build issues in stricter ESM environments
  • Developer confusion about import patterns
  • Potential future bundling inconsistencies

✅ Test Coverage

• Current Status: Existing test file (test/adapter/next.test.ts) doesn't directly import from next/server, so no test updates needed
• Assessment: The change is syntactic only and shouldn't affect functionality, so current test coverage remains adequate

Recommendations

1. Fix Consistency: Update the remaining import in packages/server/src/adapter/next/index.ts:4 to use .js extension:

   import type { NextRequest } from 'next/server.js';

2. Verify Build: Ensure the build process works correctly with both CommonJS and ESM outputs (package.json shows dual export targets)

3. Consider Automation: Add a linting rule to enforce explicit extensions for external module imports if this pattern should be consistent across the codebase

Summary

This is a solid improvement for ESM compatibility. The primary concern is the inconsistency with one remaining import that should also be updated. The change itself is low-risk and aligns well with modern ES module best practices.

Severity: Minor inconsistency issue
Recommendation: Approve after fixing the consistency issue

---

[Copilot]

Pull Request Overview

This PR attempts to improve ESM compatibility by adding a .js extension to a Next.js import statement. However, this change introduces an incorrect import path that will likely cause runtime errors.

• Modified import statement in app-route-handler.ts from next/server to next/server.js
• Updated TODO.md with task tracking progress

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/server/src/adapter/next/app-route-handler.ts	Modified Next.js import to include .js extension
TODO.md	Task tracking updates for CLI and database features (non-functional changes)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

TODO.md (1)

6-8: Consider fixing markdown indentation for consistency.

The static analysis tool flagged inconsistent indentation levels. For better consistency, consider using 2-space indentation for nested list items instead of 4-space, and 4-space for sub-sub items instead of 8-space.

Based on static analysis

Also applies to: 112-112

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2df77b9 and cc78ef6.

📒 Files selected for processing (2)

• TODO.md (2 hunks)
• packages/server/src/adapter/next/app-route-handler.ts (1 hunks)

🧰 Additional context used

🪛 markdownlint-cli2 (0.18.1)

TODO.md

6-6: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

---

7-7: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)

---

8-8: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)

---

112-112: Unordered list indentation
Expected: 4; Actual: 8

(MD007, ul-indent)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: claude-review
• GitHub Check: build-test (20.x, sqlite)
• GitHub Check: build-test (20.x, postgresql)

🔇 Additional comments (2)

TODO.md (2)

6-8: LGTM! CLI database tasks added to tracking.

The nested task structure clearly tracks the database-related CLI commands. Good progress on completing the push command.

---

112-112: Multi-schema support completion noted.

[claude]

Claude finished @ymc9's task —— View job

---

Pull Request Review

I'll review this PR focusing on code quality, best practices, potential bugs, performance, security, and test coverage.

Todo List:

• Read and analyze the changed files
• Review ESM compatibility changes
• Check for potential bugs or issues
• Evaluate performance implications
• Assess security concerns
• Review test coverage needs
• Provide comprehensive feedback

Analysis of Changes

Files Changed:

1. packages/server/src/adapter/next/app-route-handler.ts - Major refactor removed .js extension and NextResponse
2. packages/server/package.json - Updated ts-japi dependency version
3. TODO.md - Updated task completion status (unrelated to core fix)

The latest commit (fca389c) actually reverses the original ESM fix and makes significant improvements:

• Removed the problematic .js extension from next/server import
• Switched from NextResponse to native Response.json() API
• Changed import from value import to type-only import: import type { NextRequest }

Review Feedback

✅ Code Quality & Best Practices

• Excellent: The refactor to use native Response.json() instead of NextResponse is a significant improvement
• Good: Using type-only imports (import type) reduces bundle size and improves tree-shaking
• Context: The package.json shows "type": "module" and proper dual export targets, so standard imports work well

✅ Bug Fixes

• Fixed Issue: The original .js extension approach was problematic as identified by other reviewers
• Solution: Correctly reverted to standard next/server import pattern which is the recommended approach
• Improvement: Native Response.json() eliminates dependency on Next.js-specific response handling

✅ Consistency

• Current State: Both files now consistently use import type { NextRequest } from 'next/server'
• Location: Both app-route-handler.ts:2 and index.ts:4 follow the same pattern
• Assessment: Import patterns are now consistent across the Next.js adapter

✅ Performance Considerations

• Native Web APIs: Using Response.json() instead of NextResponse.json() can have slight performance benefits
• Tree Shaking: Type-only imports improve bundle optimization
• Module Resolution: Standard imports avoid potential ESM/CJS resolution conflicts

✅ Security Assessment

• No Security Implications: Changes are purely API surface modifications
• Error Handling: Maintains proper error responses and status codes
• Input Validation: Preserves existing request parameter validation

✅ Test Coverage

• Current Tests: Existing test file (test/adapter/next.test.ts) focuses on the Next.js pages API handler
• Assessment: Tests should continue to pass as the functional behavior remains the same
• Note: The refactor to Response.json() maintains the same API contract

🎯 Architecture Improvements

• Web Standards Alignment: Using native Response.json() aligns with modern web standards
• Reduced Dependencies: Less reliance on Next.js-specific APIs improves portability
• ESM Compatibility: Standard imports work well with the package's ESM configuration

Summary

This PR has evolved significantly and now represents a substantial improvement:

1. Fixed the ESM Issue: Removed the problematic .js extension that would have caused runtime errors
2. Modernized APIs: Switched to native Response.json() from Next.js-specific NextResponse
3. Improved Consistency: Standardized import patterns across Next.js adapter files
4. Enhanced Performance: Type-only imports and native APIs provide better optimization

The latest changes address the concerns raised by previous reviewers and implement a much better solution than the original .js extension approach.

Recommendation: ✅ Approve - This is a solid improvement that fixes ESM compatibility issues while modernizing the codebase.

---