Skip to content

fix: try to resolve server entry also from server/index - #4478

Open
lazerg wants to merge 3 commits into
nitrojs:mainfrom
lazerg:fix/server-entry-index
Open

fix: try to resolve server entry also from server/index#4478
lazerg wants to merge 3 commits into
nitrojs:mainfrom
lazerg:fix/server-entry-index

Conversation

@lazerg

@lazerg lazerg commented Jul 23, 2026

Copy link
Copy Markdown

🔗 Linked issue

Resolves #4455

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

server.ts in the project root is auto-detected as the server entry, and #4313 extended that to a server dir, but server/index.ts still isn't picked up. This adds index to the resolved suffixes so the folder form (server/index.ts) is detected the same way.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@lazerg
lazerg requested a review from pi0 as a code owner July 23, 2026 22:24
@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

@lazerg is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Server entry auto-detection now considers both server.ts and server/index.ts. New unit tests create temporary projects and verify that configuration loading detects each entry path.

Changes

Server entry detection

Layer / File(s) Summary
Expand server entry resolution
src/config/resolvers/paths.ts
Server entry resolution considers empty and /index suffixes for ./server.
Test server entry detection
test/unit/server-entry.test.ts
Temporary-project tests verify detection of root server.ts and nested server/index.ts entries, with cleanup after each test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code and tests implement #4455 by resolving server/index.ts as an auto-detected server entry.
Out of Scope Changes check ✅ Passed The changes stay focused on server entry resolution and the corresponding unit test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title follows conventional commits with a clear fix: prefix and accurately describes the server entry resolution change.
Description check ✅ Passed The description is directly related to the change and explains the new server/index.ts detection plus test updates.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@test/unit/server-entry.test.ts`:
- Line 3: Update the path assertions in the server entry tests to normalize the
resolved handler path before comparison, using the existing pathe utilities or
an equivalent separator-independent matcher. Apply this to the assertions around
both the referenced import and the additional lines 43–49, while preserving the
expected path values.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a68822b1-4281-4e99-99a5-a5e5b8bd52d5

📥 Commits

Reviewing files that changed from the base of the PR and between 77b77ff and 0b4383a.

📒 Files selected for processing (2)
  • src/config/resolvers/paths.ts
  • test/unit/server-entry.test.ts

Comment thread test/unit/server-entry.test.ts Outdated
@pi0 pi0 changed the title fix: try to resolve server entry also from index file fix: try to resolve server entry also from server/index Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Possibility of recognizing server/index.ts as a server entry?

2 participants