fix(cli): respect devServer config#3881
Conversation
|
@kricsleo is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe dev server startup in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cli/commands/dev.ts (1)
19-19: Minor: Remove trailing space in description.The description has a trailing space after "hostname".
- host: { type: "string", description: "specify hostname " }, + host: { type: "string", description: "specify hostname" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cli/commands/dev.ts(1 hunks)
⏰ 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: tests-rolldown (windows-latest)
- GitHub Check: tests-rollup (windows-latest)
- GitHub Check: tests-rollup (ubuntu-latest)
| port: args.port || nitro.options.devServer.port, | ||
| hostname: args.host || nitro.options.devServer.hostname, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check types and definitions relevant to the review comment
# Read the relevant sections of dev.ts to understand types
echo "=== Lines 18-20 (args definition) ==="
sed -n '18,20p' src/cli/commands/dev.ts
echo -e "\n=== Lines 60-70 (full context) ==="
sed -n '60,70p' src/cli/commands/dev.ts
# Search for listen() method signature
echo -e "\n=== Searching for listen() method ==="
rg -A 10 'listen\s*\(' --type=ts src/cli/commands/dev.ts | head -20
# Find DevServer types
echo -e "\n=== Searching for DevServer type definitions ==="
rg -B 2 -A 8 'interface.*DevServer|type.*DevServer' --type=ts | head -40Repository: nitrojs/nitro
Length of output: 2003
🏁 Script executed:
#!/bin/bash
# Check for trailing spaces in line 19
echo "=== Checking line 19 for trailing spaces ==="
sed -n '19p' src/cli/commands/dev.ts | cat -A
# Get broader context of args definition
echo -e "\n=== Lines 15-25 for complete args definition ==="
sed -n '15,25p' src/cli/commands/dev.tsRepository: nitrojs/nitro
Length of output: 547
🏁 Script executed:
#!/bin/bash
# Search for NitroDevServer class definition
echo "=== Finding NitroDevServer class ==="
rg -n "class NitroDevServer" --type=ts
echo -e "\n=== Searching for listen method in NitroDevServer ==="
rg -B 2 -A 15 "listen\s*\(" --type=ts | grep -A 15 -B 2 "NitroDevServer"
# Check if there's a types file for devServer options
echo -e "\n=== Searching for devServer type/interface definitions ==="
rg -B 2 -A 10 "devServer\s*:" --type=ts | head -50Repository: nitrojs/nitro
Length of output: 3242
🏁 Script executed:
#!/bin/bash
# Search more broadly for NitroDevServer
echo "=== Searching NitroDevServer in all files ==="
find . -name "*.ts" -type f -exec grep -l "NitroDevServer" {} \;
echo -e "\n=== Check imports in dev.ts to find NitroDevServer source ==="
head -30 src/cli/commands/dev.ts | grep -E "import|from"Repository: nitrojs/nitro
Length of output: 518
🏁 Script executed:
#!/bin/bash
# Read NitroDevServer class and find listen method
echo "=== NitroDevServer class and listen method ==="
sed -n '29,150p' src/dev/server.tsRepository: nitrojs/nitro
Length of output: 3398
🏁 Script executed:
#!/bin/bash
# Check imports to understand what 'serve' is and find ServerOptions type
echo "=== Imports in dev/server.ts ==="
head -50 src/dev/server.ts | grep -E "import|from"
echo -e "\n=== Search for ServerOptions type definition ==="
rg -n "interface ServerOptions|type ServerOptions" --type=tsRepository: nitrojs/nitro
Length of output: 830
🏁 Script executed:
#!/bin/bash
# Check srvx package to understand ServerOptions expectations
# First, see if there's a package.json or package-lock.json info
echo "=== Check srvx in package.json or dependencies ==="
grep -A 5 "srvx" package.json 2>/dev/null || echo "Not found in package.json"
# Check if there are any examples in the codebase showing how listen is called
echo -e "\n=== Search for other listen() calls with arguments ==="
rg -B 2 -A 3 "server\.listen\(" --type=ts | head -40
# Check if there's a TypeScript strict mode or type checking config
echo -e "\n=== TypeScript configuration ==="
cat tsconfig.json 2>/dev/null | grep -E "strict|port|number" || echo "tsconfig.json not readable"Repository: nitrojs/nitro
Length of output: 2259
Fix trailing space and address type mismatch in devServer config fallback.
Line 19 has a trailing space in the description: "specify hostname " — remove it.
The implementation's fallback logic is sound, but there's a type concern: args.port and args.host are strings (line 18-19), while nitro.options.devServer.port is a number. Passing string | number for port may cause issues with the serve() function. Additionally, using || means an empty string "" or "0" from the CLI would unexpectedly fall back to the config value. Consider:
- Converting
args.portto a number if provided, or - Using nullish coalescing (
??) instead of||to only fall back when undefined/null
port: args.port ? Number(args.port) : nitro.options.devServer.port,
hostname: args.host ?? nitro.options.devServer.hostname,
🤖 Prompt for AI Agents
In src/cli/commands/dev.ts around lines 64-65, remove the trailing space in the
description string ("specify hostname "), and fix type/mismatch and falsy-value
fallback: convert args.port from string to a number when provided (e.g., use
Number/parseInt and if the result is NaN fall back to
nitro.options.devServer.port) and use nullish coalescing (??) for hostname so an
empty string from CLI does not incorrectly trigger the fallback (i.e., port:
args.port provided ? Number(args.port) : nitro.options.devServer.port; hostname:
args.host ?? nitro.options.devServer.hostname).
commit: |
2 participants
🔗 Linked issue
resolves #3323
❓ Type of change
📚 Description
📝 Checklist