refactor(dev): use internal listener#1382
Open
danielroe wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe development server no longer relies on listhen for listener types, argument parsing, or server startup. It adds local HTTP/HTTPS listening, certificate loading and generation, cached binary resolution with consent handling, and Cloudflare quick tunnels. The Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
commit: |
danielroe
force-pushed
the
refactor/listhen
branch
from
July 24, 2026 23:50
fdc793f to
1b97a6c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@packages/nuxi/src/commands/dev.ts`:
- Around line 221-238: The httpsOptions validityDays parsing must reject
non-numeric values instead of passing NaN to certificate generation. Update the
validityDays handling in the HTTPS options construction to validate the numeric
conversion and use the existing undefined/invalid-input behavior when conversion
is not finite, while preserving valid numeric values.
- Around line 245-259: Update the returned configuration in the dev command
around httpsOptions so overrides.https is undefined or otherwise falsy when
HTTPS is disabled, rather than always returning the possibly empty object.
Preserve the existing HTTPS options when httpsEnabled is true, or equivalently
pass the enabled-gated value through the resolveDevServerDefaults path used by
`#loadNuxtInstance`.
In `@packages/nuxi/src/dev/binaries.ts`:
- Around line 29-42: Update resolveTool and the download/extraction flow used by
downloadBinary so installations are written to a unique temporary path rather
than destination, then atomically rename the completed file into destination
using renameSync. Ensure archive extraction completes in a per-process temporary
location before the final rename, and clean up temporary artifacts on failure so
concurrent installs cannot leave a corrupted cache entry.
- Around line 100-107: Update downloadBinary to call fetch with an explicit
timeout signal, using the project’s established timeout mechanism or AbortSignal
timeout support. Keep the existing response validation, retry behavior, and
error fallback unchanged, including propagation of timeout failures through the
current catch flow.
In `@packages/nuxi/src/dev/cert.ts`:
- Around line 45-64: Update generateCertificate so the cache freshness threshold
incorporates options.validityDays rather than relying solely on the fixed
CERT_MAX_AGE_MS value. Bound the effective cache age by the configured
certificate validity, preserving the existing certificate generation flow and
optionally documenting that this also shortens mkcert certificate caching when
validityDays is set.
In `@packages/nuxi/src/dev/listen.ts`:
- Around line 78-81: Update the server error handling in the Promise around
server.listen, replacing the one-time error listener with a persistent listener
so later server-level errors remain handled throughout the dev session. Preserve
the existing Promise resolution and rejection behavior for the initial listen
operation.
In `@packages/nuxi/src/dev/tunnel.ts`:
- Line 27: Attach a child.on('error', ...) handler immediately after spawn in
the tunnel startup flow, using the existing tunnel error-handling behavior to
report the failure and disable or clean up the tunnel without allowing an
uncaught exception to terminate nuxi dev. Keep the normal stdout/stderr and
process-exit handling unchanged.
🪄 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: 3a94f6bc-9bb3-48f6-9c0d-83ee01d161c1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
knip.jsonpackages/nuxi/package.jsonpackages/nuxi/src/commands/dev.tspackages/nuxi/src/dev/binaries.tspackages/nuxi/src/dev/cert.tspackages/nuxi/src/dev/index.tspackages/nuxi/src/dev/listen.tspackages/nuxi/src/dev/pool.tspackages/nuxi/src/dev/tunnel.tspackages/nuxi/src/dev/utils.tspackages/nuxt-cli/package.jsonpackages/nuxt-cli/test/bench/dev.bench.ts
Comment on lines
+45
to
+64
| const CERT_MAX_AGE_MS = 27 * 24 * 60 * 60 * 1000 | ||
|
|
||
| async function generateCertificate(options: HTTPSOptions): Promise<ResolvedCertificate> { | ||
| const domains = options.domains?.length ? options.domains : ['localhost', '127.0.0.1', '::1'] | ||
| const hash = createHash('sha256').update(domains.join(',')).digest('hex').slice(0, 8) | ||
| const dir = getCacheDir('certs', hash) | ||
| const certPath = join(dir, 'cert.pem') | ||
| const keyPath = join(dir, 'key.pem') | ||
|
|
||
| const fresh = existsSync(certPath) | ||
| && existsSync(keyPath) | ||
| && Date.now() - statSync(certPath).mtimeMs < CERT_MAX_AGE_MS | ||
|
|
||
| if (!fresh) { | ||
| const generated = await generateWithMkcert(certPath, keyPath, domains) | ||
| || generateWithOpenssl(certPath, keyPath, domains, options.validityDays) | ||
| if (!generated) { | ||
| throw new Error('Could not generate a development certificate. Install `mkcert` (https://github.com/FiloSottile/mkcert) or provide `--https.cert` and `--https.key`.') | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Hardcoded 27-day cache TTL ignores --https.validityDays; can serve an expired self-signed cert.
CERT_MAX_AGE_MS is fixed at 27 days and only checks the cert file's mtime, but validityDays (used by the openssl fallback, Line 126) is user-configurable and can be set below 27. If someone runs with --https.validityDays 10, then reruns nuxt dev between day 10 and day 27, this code treats the cache as "fresh" and serves a certificate that has actually already expired, breaking HTTPS in the browser.
🔧 Suggested fix: bound the cache window by the configured validity
- const fresh = existsSync(certPath)
- && existsSync(keyPath)
- && Date.now() - statSync(certPath).mtimeMs < CERT_MAX_AGE_MS
+ const maxAgeMs = Math.min(CERT_MAX_AGE_MS, (options.validityDays ?? Infinity) * 24 * 60 * 60 * 1000)
+ const fresh = existsSync(certPath)
+ && existsSync(keyPath)
+ && Date.now() - statSync(certPath).mtimeMs < maxAgeMsNote: this also shortens the cache lifetime for mkcert-issued certs when validityDays happens to be set alongside --https, even though mkcert ignores that option — an acceptable trade-off, but worth a comment if you go this route.
📝 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.
Suggested change
| const CERT_MAX_AGE_MS = 27 * 24 * 60 * 60 * 1000 | |
| async function generateCertificate(options: HTTPSOptions): Promise<ResolvedCertificate> { | |
| const domains = options.domains?.length ? options.domains : ['localhost', '127.0.0.1', '::1'] | |
| const hash = createHash('sha256').update(domains.join(',')).digest('hex').slice(0, 8) | |
| const dir = getCacheDir('certs', hash) | |
| const certPath = join(dir, 'cert.pem') | |
| const keyPath = join(dir, 'key.pem') | |
| const fresh = existsSync(certPath) | |
| && existsSync(keyPath) | |
| && Date.now() - statSync(certPath).mtimeMs < CERT_MAX_AGE_MS | |
| if (!fresh) { | |
| const generated = await generateWithMkcert(certPath, keyPath, domains) | |
| || generateWithOpenssl(certPath, keyPath, domains, options.validityDays) | |
| if (!generated) { | |
| throw new Error('Could not generate a development certificate. Install `mkcert` (https://github.com/FiloSottile/mkcert) or provide `--https.cert` and `--https.key`.') | |
| } | |
| } | |
| const CERT_MAX_AGE_MS = 27 * 24 * 60 * 60 * 1000 | |
| async function generateCertificate(options: HTTPSOptions): Promise<ResolvedCertificate> { | |
| const domains = options.domains?.length ? options.domains : ['localhost', '127.0.0.1', '::1'] | |
| const hash = createHash('sha256').update(domains.join(',')).digest('hex').slice(0, 8) | |
| const dir = getCacheDir('certs', hash) | |
| const certPath = join(dir, 'cert.pem') | |
| const keyPath = join(dir, 'key.pem') | |
| const maxAgeMs = Math.min(CERT_MAX_AGE_MS, (options.validityDays ?? Infinity) * 24 * 60 * 60 * 1000) | |
| const fresh = existsSync(certPath) | |
| && existsSync(keyPath) | |
| && Date.now() - statSync(certPath).mtimeMs < maxAgeMs | |
| if (!fresh) { | |
| const generated = await generateWithMkcert(certPath, keyPath, domains) | |
| || generateWithOpenssl(certPath, keyPath, domains, options.validityDays) | |
| if (!generated) { | |
| throw new Error('Could not generate a development certificate. Install `mkcert` (https://github.com/FiloSottile/mkcert) or provide `--https.cert` and `--https.key`.') | |
| } | |
| } |
🤖 Prompt for 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.
In `@packages/nuxi/src/dev/cert.ts` around lines 45 - 64, Update
generateCertificate so the cache freshness threshold incorporates
options.validityDays rather than relying solely on the fixed CERT_MAX_AGE_MS
value. Bound the effective cache age by the configured certificate validity,
preserving the existing certificate generation flow and optionally documenting
that this also shortens mkcert certificate caching when validityDays is set.
danielroe
force-pushed
the
refactor/listhen
branch
from
July 25, 2026 00:21
1b97a6c to
8351819
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
📚 Description
this follows in nitro's footsteps by replacing
listhen, which is a pretty hefty dependency, with our own listener. (this is also what vite, nitro, etc. do)we only used a small part of that, which is feasible to maintain ourselves within nuxt/cli. the key benefit is about 3.5MB of unneeded dependencies we can lose.
Note
we'll likely migrate to
srvxlater on, and possibly even upstream some of these changes into a new listhen v2, if appropriate(this won't make much difference in nuxt v3/4 because nitropack pulls them in as it has its own dependency on
listhen, but in nuxt v5, it'll feel great!)using
mkcertnot only removes a dependency onnode-forge, but generates a locally-trusted certificate, which should be strictly better....a quick feature comparison
get-port-please--host/--public--host= all interfaces--open--clipboardtinyclip)--qr(auto when public, centered)uqr)--publicURL--httpswith own cert/key/pfx--httpswith no certnode-forge(untrusted)mkcert(PATH or one-time download);opensslself-signed fallback--tunneluntuncloudflareddirectly; ToS consent now persisted in user.nuxtrchttp-shutdownserver.closeAllConnections()--https.validityDaysopensslfallback (mkcert fixes its own validity)--name,--baseURLCLI flagsbaseURLstill resolved fromnuxt.config)mkcertoropenssl(error with hint otherwise)