feat(cli): support opening in browser with dev command#3871
feat(cli): support opening in browser with dev command#3871huseeiin wants to merge 1 commit intonitrojs:mainfrom
Conversation
|
@huseeiin is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA new CLI flag Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 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
📜 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 (2)
package.json(1 hunks)src/cli/commands/dev.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cli/commands/dev.ts (1)
src/runtime/internal/vite/node-runner.mjs (2)
url(289-289)server(170-183)
⏰ 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-rollup (ubuntu-latest)
- GitHub Check: tests-rolldown (windows-latest)
- GitHub Check: tests-rollup (windows-latest)
🔇 Additional comments (4)
src/cli/commands/dev.ts (3)
8-8: LGTM!The import statement is correct for the
openpackage.
19-22: LGTM!The CLI flag definition is clear and follows the existing pattern for command arguments.
68-73: LGTM!Capturing the URL from
server.listen()is necessary for the browser opening feature.package.json (1)
71-71: Theopenpackage version 11.0.0 exists on npm and has no known security vulnerabilities. No action is required.
| }) | ||
| ).url; | ||
|
|
||
| if (args.open && url) open(url); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat src/cli/commands/dev.ts 2>/dev/null | head -100Repository: nitrojs/nitro
Length of output: 2283
🏁 Script executed:
rg "^import.*from.*open" --type ts --type jsRepository: nitrojs/nitro
Length of output: 361
🏁 Script executed:
rg "args\.open|open\(" --type ts --type js -B 3 -A 3Repository: nitrojs/nitro
Length of output: 1332
🏁 Script executed:
rg -A 30 "listen\(" src/dev/server.ts | grep -A 30 "listen"Repository: nitrojs/nitro
Length of output: 907
🏁 Script executed:
cat -n src/dev/server.ts | head -200Repository: nitrojs/nitro
Length of output: 6855
🏁 Script executed:
# Search for IPv6 related issues or comments in the codebase
rg "\[::1\]|IPv6" --type ts --type jsRepository: nitrojs/nitro
Length of output: 188
🏁 Script executed:
# Check if there are any recent commits or PRs about this
git log --all --oneline --grep="IPv6\|localhost\|\[::1\]" | head -20Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look for test files that might show expected URL format
fd ".*test.*" src/ --type f | xargs grep -l "open\|url\|listen" | head -5Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Check the open package type definitions or usage
rg "typeof.*open|open.*Promise" --type tsRepository: nitrojs/nitro
Length of output: 137
🏁 Script executed:
# Check the common args to see what defaults are set
cat src/cli/common.tsRepository: nitrojs/nitro
Length of output: 344
🏁 Script executed:
# Search for devServer configuration in nitro
rg "devServer" src/ --type ts -A 3 | head -40Repository: nitrojs/nitro
Length of output: 1905
🏁 Script executed:
# Check the srvx package to understand how it formats URLs
rg "serve\(" src/dev/server.ts -B 2 -A 10Repository: nitrojs/nitro
Length of output: 406
🏁 Script executed:
# Check what args.port and args.host default to
rg -B 5 -A 5 "listen.*args" src/cli/commands/dev.tsRepository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Search for srvx in package.json to understand the package
cat package.json | grep -A 2 -B 2 "srvx"Repository: nitrojs/nitro
Length of output: 195
🏁 Script executed:
# Look for how server address is determined
rg "localhost|127\.0\.0\.1" src/ --type ts | head -20Repository: nitrojs/nitro
Length of output: 667
🏁 Script executed:
# Check if args.open, args.host, args.port are actually defined as command args
rg -A 30 "args:" src/cli/commands/dev.ts | grep -E "open|host|port" -A 2Repository: nitrojs/nitro
Length of output: 307
🏁 Script executed:
# Search for any bug reports or issues about IPv6
git log --all --oneline | grep -i "ipv6\|open.*url\|browser" | head -10Repository: nitrojs/nitro
Length of output: 157
🏁 Script executed:
# Check how the open package from npm handles IPv6 URLs
npm info open | head -40 2>/dev/null || echo "npm info not available in sandbox"Repository: nitrojs/nitro
Length of output: 933
🏁 Script executed:
# Get more details about the open feature commit
git show 346f20a | head -100Repository: nitrojs/nitro
Length of output: 2854
🏁 Script executed:
git show 346f20a -- src/cli/commands/dev.tsRepository: nitrojs/nitro
Length of output: 2705
🏁 Script executed:
# Check if there are any similar patterns in the codebase where open is used
rg "open\(" src/ --type ts --type js | grep -v "open()" | head -10Repository: nitrojs/nitro
Length of output: 119
Add error handling and await the open() call.
The open() function returns a Promise and must be awaited to properly handle errors. Without await and error handling, an unhandled promise rejection could occur if the browser fails to open.
Apply this diff:
- if (args.open && url) open(url);
+ if (args.open && url) {
+ try {
+ await open(url);
+ } catch (error) {
+ consola.warn("Failed to open browser:", error);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (args.open && url) open(url); | |
| if (args.open && url) { | |
| try { | |
| await open(url); | |
| } catch (error) { | |
| consola.warn("Failed to open browser:", error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/cli/commands/dev.ts around line 75, the call if (args.open && url)
open(url); must be converted to an awaited call with error handling: change it
to await the open(url) inside a try/catch (or Promise.catch) so any rejection is
handled and logged or reported appropriately; ensure the enclosing function is
async or use an IIFE to await, and in the catch block log a concise message
including the error so failures to launch the browser don't cause unhandled
promise rejections.
commit: |
| "jiti": "^2.6.1", | ||
| "ofetch": "^2.0.0-alpha.3", | ||
| "ohash": "^2.0.11", | ||
| "open": "^11.0.0", |
There was a problem hiding this comment.
Ideally we should avoid additional dependencies or bundle them open adds 11 subdeps
There was a problem hiding this comment.
Wondering what vite CLI does use for this feature
There was a problem hiding this comment.
open as well, i'm pretty sure
There was a problem hiding this comment.
side idea: what if we base standalone nitro on vite? meaning like vite-plugin-nitro, but it only does backend and doesn't use index.html. this way we can inherit all of vite's support for --open, https, http2, etc.
it'd be like:
vite-plugin-nitro: plugin for nitro-based frameworks
standalone nitro: a vite-based framework in of itself
There was a problem hiding this comment.
Ideally we should avoid additional dependencies or bundle them open adds 11 subdeps
maybe stupid question: what difference does bundling it make? the dependencies will still be there, just bundled
pi0
changed the title
2 participants
my first feature PR to nitro. might've made mistakes.
i tested and it opens
http://[::1]:3000/in the browser instead ofhttp://localhost:3000. might need changes?🔗 Linked issue
❓ Type of change
📚 Description
📝 Checklist