Repository navigation
refactor(nitro): drop the dead !ctx operand in the v3 response hook - #796
Merged
Merged
Conversation
The compound guard two lines above already returns when ctx is undefined, so the !ctx check in the log guard is unreachable and ctx?.log can read ctx.log directly.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
Contributor
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Contributor
|
Thank you for following the naming conventions! 🙏 |
@evlog/cli
evlog
@evlog/nuxthub
@evlog/signals
@evlog/telemetry
commit: |
This branch was successfully deployed
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
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.
Summary
In the nitro-v3
responsehook (packages/evlog/src/nitro-v3/plugin.ts), the guardalready returns when
ctxis undefined: withctxundefined,!ctx?._evlogShouldEmitis true, so control never reaches the second guard. The!ctxoperand is therefore dead, and once it is gone the?.onctx?.logis dead with it (TypeScript narrowsctxpast the compound guard, confirmed bypnpm run typecheck). The same shape already exists in theerrorhook two functions below, which readsctx.logafterif (!ctx?._evlogShouldEmit) return.Behavior is preserved. The nitro-v3 integration suite (
test/nitro-v3/nitro-v3.test.ts) is the matching test: it drives theresponsehook through h3 v2's real request driver, including the filtered-out and already-emitted branches this guard serves.Checks
pnpm --filter evlog exec vitest run test/nitro-v3/: 5 files, 29 tests passed with the edit.pnpm run lint,pnpm run typecheck,pnpm run test: all exit 0 with the edit (96 files, 2,014 tests). Typecheck is the load-bearing one here: it provesctxis narrowed so the non-optionalctx.logand the laterctx._evlogStartTimeread insideemitSuccessResponsestay valid.AGENTS.md).Found by the repo-health sweep at revision
54dcc504; confirmed by the workflow's verifier and reproduced in the sandbox.Note: this touches
plugin.tslines 246-247, just above the region PR #795 refactors; if #795 merges first this rebases cleanly, and vice versa.ELI5
One safety check in the v3 request logger asked the same question twice in a row, and the second ask could never fail because the first had already handled it. Removing the repeated check makes the code read straight through; the automated tests confirm nothing changed at runtime.