Open
Conversation
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces SSE-based hints delivery with a devtools RPC-based system and in-process HTTP handlers. Client app now registers extended devtools RPC handlers and injects reactive stores for hydration mismatches, lazy-load hints, and HTML-validate reports with buffering, id de-duplication, and tombstone-based clearing; feature flags gate initial fetches. Removed SSE plugins, SSE route/type exports, and SSE server handler; added typed RPC types, a global RPC accessor, new in-memory handlers and router for hydration/lazy-load/html-validate, revised Nitro/devtools integration, and updated tests. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/runtime/types.d.ts (1)
1-67:⚠️ Potential issue | 🔴 CriticalMissing
NitroRuntimeHookstype augmentation causes pipeline failure.The PR removed the
nitropackmodule augmentation that typed the custom Nitro hooks (hints:hydration:mismatch,hints:lazy-load:report,hints:html-validate:report, etc.). This causes TypeScript errors insrc/runtime/core/server/rpc-bridge.tswhere these hooks are used.Re-add the
NitroRuntimeHooksaugmentation or move it to an appropriate location (e.g., alongsiderpc-bridge.ts):declare module 'nitropack' { interface NitroRuntimeHooks { 'hints:hydration:mismatch': (payload: HydrationMismatchPayload) => void 'hints:hydration:cleared': (payload: { id: string[] }) => void 'hints:lazy-load:report': (data: ComponentLazyLoadData) => void 'hints:lazy-load:cleared': (payload: { id: string }) => void 'hints:html-validate:report': (report: HtmlValidateReport) => void 'hints:html-validate:deleted': (id: string) => void } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/types.d.ts` around lines 1 - 67, Re-add the missing TypeScript module augmentation for 'nitropack' to restore the NitroRuntimeHooks interface (so rpc-bridge.ts compiles); declare the hooks 'hints:hydration:mismatch', 'hints:hydration:cleared', 'hints:lazy-load:report', 'hints:lazy-load:cleared', 'hints:html-validate:report', and 'hints:html-validate:deleted' on NitroRuntimeHooks and reference the existing payload types used elsewhere (HydrationMismatchPayload, ComponentLazyLoadData, HtmlValidateReport); place this declaration in src/runtime/types.d.ts (or next to rpc-bridge.ts) and ensure the referenced types are imported/available in that module augmentation.src/module.ts (1)
86-90:⚠️ Potential issue | 🟠 MajorHTML-validate reports will silently fail when devtools is disabled.
The
htmlValidatefeature is enabled independently viaisFeatureEnabled(options, 'htmlValidate')at lines 87-90, but the HTTP endpoint for hints (/__nuxt_hints) is registered only insidesetupDevToolsUI(), which is called conditionally whenoptions.devtoolsis true. The Nitro plugin at line 89 will attempt to POST reports to a non-existent endpoint, resulting in silent 404 failures.Consider either registering the hints router unconditionally when any feature needs it, or making the Nitro plugin check if the endpoint exists before posting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module.ts` around lines 86 - 90, The html-validate Nitro plugin posts to the /__nuxt_hints endpoint but that hints router is only registered inside setupDevToolsUI() when options.devtools is true, causing 404s when htmlValidate is enabled without devtools; fix by ensuring the hints router is registered whenever any feature needs it (e.g., extract the hints router registration from setupDevToolsUI() into a new function and call it when isFeatureEnabled(options, 'htmlValidate') is true before addServerPlugin(resolver.resolve('./runtime/html-validate/nitro.plugin')), or alternatively modify the Nitro plugin (the file resolved by resolver.resolve('./runtime/html-validate/nitro.plugin')) to detect the /__nuxt_hints endpoint (or handle 404s) before attempting to POST; update references to setupDevToolsUI(), isFeatureEnabled, addServerPlugin and the nitro plugin accordingly.
🧹 Nitpick comments (2)
src/runtime/html-validate/api-handlers.ts (1)
31-41: Minimal validation on POST body.The validation only checks that
body.idis a string, butHtmlValidateReportlikely requires additional fields (path,html,results). Malformed payloads will be stored and could cause issues downstream.Consider validating the complete shape or using a schema validator like valibot (already used in
lazy-load/handlers.ts).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/html-validate/api-handlers.ts` around lines 31 - 41, The postHandler currently only verifies body.id; update validation to ensure the full HtmlValidateReport shape is present before pushing to htmlValidateReports and calling onReport: validate that body.path and body.html are strings and that body.results is an array (or use the existing valibot schema pattern from lazy-load/handlers.ts to validate the structure returned by readBody<HtmlValidateReport>(event)); if validation fails, throw createError({ statusCode: 400, ... }) and do not mutate htmlValidateReports or call onReport; keep setResponseStatus(event, 201) only after successful validation.src/runtime/html-validate/nitro.plugin.ts (1)
57-62: Silent error swallowing hides failures.The fetch call silently discards all errors with
.catch(() => {}). If the/__nuxt_hints/html-validateendpoint is unavailable (e.g., devtools disabled) or the request fails, there's no indication in logs.Consider at least logging errors in development:
Proposed improvement
const origin = getRequestURL(event).origin globalThis.fetch(`${origin}/__nuxt_hints/html-validate`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(data), - }).catch(() => {}) + }).catch((err) => { + console.debug('[hints:html-validate] Failed to report validation:', err.message) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/html-validate/nitro.plugin.ts` around lines 57 - 62, The fetch to the '/__nuxt_hints/html-validate' endpoint currently swallows all errors via .catch(() => {}); update the error handling for the globalThis.fetch call (used with getRequestURL(event).origin) to at minimum log the caught error (e.g., console.error or the project logger) in non-production/dev builds so failed requests are visible during development; ensure the catch provides context like "html-validate POST failed" and includes the error object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/app/app.vue`:
- Around line 22-54: The live RPC handlers (registered in
onDevtoolsClientConnected: onHydrationMismatch, onHydrationCleared,
onLazyLoadReport, onLazyLoadCleared, onHtmlValidateReport,
onHtmlValidateDeleted) can reintroduce items when the initial snapshot from the
three get*() calls arrives; fix by either buffering live updates until initial
snapshots are applied or tracking tombstones seen during initial sync: create
per-collection tombstone Sets (e.g., hydrationTombstones, lazyLoadTombstones,
htmlValidateTombstones) and/or an initialSyncDone flag, have each live handler
check the tombstone set and skip adding items whose id is in the tombstone (or
if initialSyncDone is false, enqueue the update), and when each get*().then(...)
merges the snapshot, remove tombstoned ids from the result and flush buffered
updates; reference the onDevtoolsClientConnected registration and the handler
names above to locate where to add tombstone logic or buffering.
In `@src/runtime/core/server/rpc-bridge.ts`:
- Around line 1-6: Add missing type declarations by creating a .d.ts file (e.g.,
src/runtime/types.d.ts) that (1) declares the global var __nuxtHintsRpcBroadcast
typed as HintsClientFunctions | undefined to satisfy uses of
globalThis.__nuxtHintsRpcBroadcast (reference HintsClientFunctions from
src/runtime/core/rpc-types) and (2) re-augment the NitroRuntimeHooks interface
to include the missing hook names removed from the project so the Nitro hook
usages compile; ensure the file uses declare global { var
__nuxtHintsRpcBroadcast: HintsClientFunctions | undefined } and the appropriate
module augmentation for NitroRuntimeHooks.
- Around line 13-14: The hook "hints:hydration:cleared" handler is passing
payload.id (singular) to getRpcBroadcast()?.onHydrationCleared which expects
ids: string[]; update the hook payload and call so the handler provides an
array: change the hook signature/type to use ids: string[] (instead of id) and
call getRpcBroadcast()?.onHydrationCleared(payload.ids); alternatively, if
payload must remain singular, wrap it into an array when calling
onHydrationCleared (e.g., [payload.id]); update the nitroApp.hooks.hook handler
and any related type/interface that defines the hook payload to reflect ids:
string[].
- Around line 8-31: The exported default function registering Nitro hooks (hook
names: 'hints:hydration:mismatch', 'hints:hydration:cleared',
'hints:lazy-load:report', 'hints:lazy-load:cleared',
'hints:html-validate:report', 'hints:html-validate:deleted') is dead because
getRpcBroadcast() is never populated (globalThis.__nuxtHintsRpcBroadcast) and
the codebase uses direct callbacks via setHydrationNotify, setLazyLoadNotify,
setHtmlValidateNotify that call rpc.broadcast; either remove this bridge
entirely, or rewire it by (a) ensuring devtools.ts assigns
globalThis.__nuxtHintsRpcBroadcast so getRpcBroadcast() returns the broadcast
implementation, and (b) updating the handlers that currently call
setHydrationNotify/setLazyLoadNotify/setHtmlValidateNotify to also trigger the
corresponding Nitro hooks (e.g.,
nitroApp.hooks.callHook('hints:hydration:mismatch', ...) or emit equivalent) so
the bridge receives events—pick one approach and apply it consistently (delete
unused default export and its hook registrations if removing, or add the global
assignment in devtools.ts and the hook emissions in the handler functions if
reconnecting).
In `@src/runtime/html-validate/api-handlers.ts`:
- Around line 21-27: clearHtmlValidateReport currently calls onDeleted?.(id)
unconditionally, causing notifications for IDs that weren't found; change the
function so it only invokes onDeleted when a matching report was actually
removed — i.e., check the result of htmlValidateReports.findIndex (index !== -1)
and, inside that branch after splice, call onDeleted?.(id) (do not call it when
no report was found).
In `@src/runtime/hydration/handlers.ts`:
- Around line 43-47: When the 20-item ring buffer drops the oldest entry in
hydrationMismatches, also notify clients so they can remove the evicted mismatch
from their local state (clients currently only remove items via
onHydrationCleared and will otherwise drift from getHydrationMismatches()).
Change the eviction logic in the block that currently does
hydrationMismatches.push(payload) / hydrationMismatches.shift() to capture the
shifted item (const evicted = hydrationMismatches.shift()) and call the client
notification callback (e.g., onHydrationCleared?.(evicted) or the appropriate
signature your system uses) immediately after eviction; keep onMismatch(payload)
as-is for the new mismatch notification.
- Around line 61-69: The assertPayload function currently treats null as an
object and dereferences fields, causing a TypeError; change the initial guard to
explicitly reject null (e.g., check body === null or !body) before accessing
properties so null bodies fail the assertion and trigger a 400; update the guard
in assertPayload (used by postHandler) to mirror the defensive pattern used in
deleteHandler by ensuring body is truthy and an object before checking
htmlPreHydration, htmlPostHydration, componentName, and fileLocation.
In `@src/runtime/lazy-load/handlers.ts`:
- Around line 23-29: POST /lazy-load must be idempotent by id: in the
postHandler, do not always push into lazyLoadData — locate an existing item in
lazyLoadData by item.id and either update/replace it or skip adding if
identical; use findIndex(item => item.id === id) and set lazyLoadData[index] =
newItem when found, otherwise push. Also make deletion/clearing remove all
duplicates: update deleteHandler and clearLazyLoadHint to remove every entry
with the given id (e.g., lazyLoadData = lazyLoadData.filter(item => item.id !==
id)) so a duplicate doesn't reappear on the next GET. Ensure you reference and
modify the lazyLoadData array and the functions postHandler, deleteHandler, and
clearLazyLoadHint accordingly.
---
Outside diff comments:
In `@src/module.ts`:
- Around line 86-90: The html-validate Nitro plugin posts to the /__nuxt_hints
endpoint but that hints router is only registered inside setupDevToolsUI() when
options.devtools is true, causing 404s when htmlValidate is enabled without
devtools; fix by ensuring the hints router is registered whenever any feature
needs it (e.g., extract the hints router registration from setupDevToolsUI()
into a new function and call it when isFeatureEnabled(options, 'htmlValidate')
is true before
addServerPlugin(resolver.resolve('./runtime/html-validate/nitro.plugin')), or
alternatively modify the Nitro plugin (the file resolved by
resolver.resolve('./runtime/html-validate/nitro.plugin')) to detect the
/__nuxt_hints endpoint (or handle 404s) before attempting to POST; update
references to setupDevToolsUI(), isFeatureEnabled, addServerPlugin and the nitro
plugin accordingly.
In `@src/runtime/types.d.ts`:
- Around line 1-67: Re-add the missing TypeScript module augmentation for
'nitropack' to restore the NitroRuntimeHooks interface (so rpc-bridge.ts
compiles); declare the hooks 'hints:hydration:mismatch',
'hints:hydration:cleared', 'hints:lazy-load:report', 'hints:lazy-load:cleared',
'hints:html-validate:report', and 'hints:html-validate:deleted' on
NitroRuntimeHooks and reference the existing payload types used elsewhere
(HydrationMismatchPayload, ComponentLazyLoadData, HtmlValidateReport); place
this declaration in src/runtime/types.d.ts (or next to rpc-bridge.ts) and ensure
the referenced types are imported/available in that module augmentation.
---
Nitpick comments:
In `@src/runtime/html-validate/api-handlers.ts`:
- Around line 31-41: The postHandler currently only verifies body.id; update
validation to ensure the full HtmlValidateReport shape is present before pushing
to htmlValidateReports and calling onReport: validate that body.path and
body.html are strings and that body.results is an array (or use the existing
valibot schema pattern from lazy-load/handlers.ts to validate the structure
returned by readBody<HtmlValidateReport>(event)); if validation fails, throw
createError({ statusCode: 400, ... }) and do not mutate htmlValidateReports or
call onReport; keep setResponseStatus(event, 201) only after successful
validation.
In `@src/runtime/html-validate/nitro.plugin.ts`:
- Around line 57-62: The fetch to the '/__nuxt_hints/html-validate' endpoint
currently swallows all errors via .catch(() => {}); update the error handling
for the globalThis.fetch call (used with getRequestURL(event).origin) to at
minimum log the caught error (e.g., console.error or the project logger) in
non-production/dev builds so failed requests are visible during development;
ensure the catch provides context like "html-validate POST failed" and includes
the error object.
🪄 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
Run ID: dbaa79ce-c8c4-4d7b-9bf0-5214df7c94f7
📒 Files selected for processing (27)
client/app/app.vueclient/app/plugins/0.sse.tsclient/app/plugins/html-validate.tsclient/app/plugins/hydration.tsclient/app/plugins/lazy-load.tsclient/app/utils/routes.tssrc/devtools-handlers.tssrc/devtools.tssrc/module.tssrc/runtime/core/rpc-types.tssrc/runtime/core/server/rpc-bridge.tssrc/runtime/core/server/sse.tssrc/runtime/core/server/types.tssrc/runtime/html-validate/api-handlers.tssrc/runtime/html-validate/handlers/delete.tssrc/runtime/html-validate/handlers/get.tssrc/runtime/html-validate/handlers/nitro-handlers.plugin.tssrc/runtime/html-validate/nitro.plugin.tssrc/runtime/html-validate/storage.tssrc/runtime/hydration/handlers.tssrc/runtime/hydration/nitro.plugin.tssrc/runtime/hydration/types.tssrc/runtime/hydration/utils.tssrc/runtime/lazy-load/handlers.tssrc/runtime/lazy-load/nitro.plugin.tssrc/runtime/types.d.tstest/unit/core/sse.test.ts
💤 Files with no reviewable changes (14)
- src/runtime/html-validate/storage.ts
- client/app/utils/routes.ts
- src/runtime/core/server/sse.ts
- src/runtime/html-validate/handlers/get.ts
- client/app/plugins/0.sse.ts
- src/runtime/html-validate/handlers/nitro-handlers.plugin.ts
- src/runtime/html-validate/handlers/delete.ts
- test/unit/core/sse.test.ts
- client/app/plugins/lazy-load.ts
- client/app/plugins/hydration.ts
- src/runtime/hydration/types.ts
- client/app/plugins/html-validate.ts
- src/runtime/hydration/nitro.plugin.ts
- src/runtime/lazy-load/nitro.plugin.ts
Comment on lines
+1
to
+6
| import type { NitroApp } from 'nitropack/types' | ||
| import type { HintsClientFunctions } from '../rpc-types' | ||
|
|
||
| function getRpcBroadcast(): HintsClientFunctions | undefined { | ||
| return globalThis.__nuxtHintsRpcBroadcast | ||
| } |
There was a problem hiding this comment.
TypeScript errors: missing type declarations.
The pipeline is failing due to multiple TypeScript errors:
globalThis.__nuxtHintsRpcBroadcastneeds a type declaration- The Nitro hook names are not recognized because
NitroRuntimeHooksaugmentation was removed
Add type declarations to fix:
Proposed fix for globalThis typing
Add to a .d.ts file (e.g., src/runtime/types.d.ts or a new file):
import type { HintsClientFunctions } from './core/rpc-types'
declare global {
// eslint-disable-next-line no-var
var __nuxtHintsRpcBroadcast: HintsClientFunctions | undefined
}🧰 Tools
🪛 GitHub Actions: ci
[error] 5-5: TypeScript (tsc) error TS7017: Element implicitly has an 'any' type because type 'typeof globalThis' has no index signature.
🪛 GitHub Check: ci
[failure] 5-5:
Element implicitly has an 'any' type because type 'typeof globalThis' has no index signature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/core/server/rpc-bridge.ts` around lines 1 - 6, Add missing type
declarations by creating a .d.ts file (e.g., src/runtime/types.d.ts) that (1)
declares the global var __nuxtHintsRpcBroadcast typed as HintsClientFunctions |
undefined to satisfy uses of globalThis.__nuxtHintsRpcBroadcast (reference
HintsClientFunctions from src/runtime/core/rpc-types) and (2) re-augment the
NitroRuntimeHooks interface to include the missing hook names removed from the
project so the Nitro hook usages compile; ensure the file uses declare global {
var __nuxtHintsRpcBroadcast: HintsClientFunctions | undefined } and the
appropriate module augmentation for NitroRuntimeHooks.
Comment on lines
+13
to
+14
| nitroApp.hooks.hook('hints:hydration:cleared', (payload) => { | ||
| getRpcBroadcast()?.onHydrationCleared(payload.id) |
There was a problem hiding this comment.
Type mismatch: payload.id vs ids array.
The hints:hydration:cleared hook handler passes payload.id (singular), but HintsClientFunctions.onHydrationCleared expects ids: string[] (array) based on the interface in rpc-types.ts.
Proposed fix
nitroApp.hooks.hook('hints:hydration:cleared', (payload) => {
- getRpcBroadcast()?.onHydrationCleared(payload.id)
+ getRpcBroadcast()?.onHydrationCleared(payload.ids)
})Also update the hook type to use ids: string[] instead of id.
📝 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
| nitroApp.hooks.hook('hints:hydration:cleared', (payload) => { | |
| getRpcBroadcast()?.onHydrationCleared(payload.id) | |
| nitroApp.hooks.hook('hints:hydration:cleared', (payload) => { | |
| getRpcBroadcast()?.onHydrationCleared(payload.ids) | |
| }) |
🧰 Tools
🪛 GitHub Check: ci
[failure] 13-13:
Parameter 'payload' implicitly has an 'any' type.
[failure] 13-13:
Argument of type '"hints:hydration:cleared"' is not assignable to parameter of type 'HookKeys'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/core/server/rpc-bridge.ts` around lines 13 - 14, The hook
"hints:hydration:cleared" handler is passing payload.id (singular) to
getRpcBroadcast()?.onHydrationCleared which expects ids: string[]; update the
hook payload and call so the handler provides an array: change the hook
signature/type to use ids: string[] (instead of id) and call
getRpcBroadcast()?.onHydrationCleared(payload.ids); alternatively, if payload
must remain singular, wrap it into an array when calling onHydrationCleared
(e.g., [payload.id]); update the nitroApp.hooks.hook handler and any related
type/interface that defines the hook payload to reflect ids: string[].
Comment on lines
+21
to
+27
| export function clearHtmlValidateReport(id: string) { | ||
| const index = htmlValidateReports.findIndex(r => r.id === id) | ||
| if (index !== -1) { | ||
| htmlValidateReports.splice(index, 1) | ||
| } | ||
| onDeleted?.(id) | ||
| } |
There was a problem hiding this comment.
onDeleted callback invoked even when report doesn't exist.
The clearHtmlValidateReport function calls onDeleted?.(id) unconditionally, even when no matching report was found. This can cause spurious RPC broadcasts to clients for non-existent IDs.
Proposed fix
export function clearHtmlValidateReport(id: string) {
const index = htmlValidateReports.findIndex(r => r.id === id)
if (index !== -1) {
htmlValidateReports.splice(index, 1)
+ onDeleted?.(id)
}
- onDeleted?.(id)
}📝 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
| export function clearHtmlValidateReport(id: string) { | |
| const index = htmlValidateReports.findIndex(r => r.id === id) | |
| if (index !== -1) { | |
| htmlValidateReports.splice(index, 1) | |
| } | |
| onDeleted?.(id) | |
| } | |
| export function clearHtmlValidateReport(id: string) { | |
| const index = htmlValidateReports.findIndex(r => r.id === id) | |
| if (index !== -1) { | |
| htmlValidateReports.splice(index, 1) | |
| onDeleted?.(id) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/html-validate/api-handlers.ts` around lines 21 - 27,
clearHtmlValidateReport currently calls onDeleted?.(id) unconditionally, causing
notifications for IDs that weren't found; change the function so it only invokes
onDeleted when a matching report was actually removed — i.e., check the result
of htmlValidateReports.findIndex (index !== -1) and, inside that branch after
splice, call onDeleted?.(id) (do not call it when no report was found).
This was referenced Apr 5, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
client/app/composables/features.ts (1)
11-14: Unify disconnected-state defaults across both feature helpers.Line 13 and Line 26 currently disagree on fallback behavior. This can produce contradictory gating (
useEnabledHintsFeatures()vsuseHintsFeature()).♻️ Proposed fix
import type { FeaturesName } from '../../../src/runtime/core/types' import { useDevtoolsClient } from '@nuxt/devtools-kit/iframe-client' +const DEFAULT_DEVTOOLS_FEATURES: Record<FeaturesName, boolean> = { + hydration: true, + lazyLoad: true, + webVitals: false, + thirdPartyScripts: false, + htmlValidate: true, +} + export function useHintsConfig() { const hostNuxt = useHostNuxt() return hostNuxt.hints.config } export function useEnabledHintsFeatures(): Record<FeaturesName, boolean> { const client = useDevtoolsClient().value if (!client?.host?.nuxt) { - return { hydration: true, lazyLoad: true, webVitals: false, thirdPartyScripts: false, htmlValidate: true } + return DEFAULT_DEVTOOLS_FEATURES } const config = useHintsConfig() return Object.fromEntries<boolean>( @@ export function useHintsFeature(feature: FeaturesName): boolean { const client = useDevtoolsClient().value - if (!client?.host?.nuxt) return true + if (!client?.host?.nuxt) return DEFAULT_DEVTOOLS_FEATURES[feature] const config = useHintsConfig() return typeof config.features[feature] === 'object' ? config.features[feature].devtools !== false : Boolean(config.features[feature]) }Also applies to: 25-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/app/composables/features.ts` around lines 11 - 14, The two feature helper functions return different fallback defaults when the devtools client is disconnected; update both useHintsFeature() and useEnabledHintsFeatures() to return the same unified disconnected-state defaults (e.g. hydration: true, lazyLoad: true, webVitals: false, thirdPartyScripts: false, htmlValidate: true) so gating behavior is consistent across helpers; locate the conditional that checks client?.host?.nuxt in each function and replace the differing return objects with the single canonical default object.src/runtime/hydration/handlers.ts (1)
11-19: RPC notification includes all requested IDs, even those not found.
clearHydrationMismatchesbroadcastsonHydrationCleared(ids)with all input IDs, even if some weren't in the array. Consider filtering to only the actually-removed IDs for consistency with other handlers.Proposed fix to notify only removed IDs
export function clearHydrationMismatches(ids: string[]) { + const removed: string[] = [] for (const id of ids) { const index = hydrationMismatches.findIndex(m => m.id === id) if (index !== -1) { hydrationMismatches.splice(index, 1) + removed.push(id) } } - getRPC()?.onHydrationCleared(ids) + if (removed.length > 0) { + getRPC()?.onHydrationCleared(removed) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/hydration/handlers.ts` around lines 11 - 19, clearHydrationMismatches currently calls getRPC()?.onHydrationCleared(ids) with every requested id even if none were removed; update clearHydrationMismatches to build an array (e.g., removedIds) of only the ids that were actually found and spliced from hydrationMismatches (use the existing loop over ids and push when index !== -1), and then call getRPC()?.onHydrationCleared(removedIds) only if removedIds.length > 0; reference function clearHydrationMismatches, the hydrationMismatches array, and getRPC()?.onHydrationCleared when applying the change.src/runtime/lazy-load/handlers.ts (1)
35-41: POST skips update when entry exists; consider upsert semantics.When an entry with the same
idalready exists, the code silently skips (no update, no RPC notification) but still returns 201. If the client sends updated data for an existing id, those changes are lost. Consider whether upsert behavior is intended:Proposed upsert implementation
const index = lazyLoadData.findIndex(item => item.id === parsed.id) if (index === -1) { lazyLoadData.push(parsed) - getRPC()?.onLazyLoadReport(parsed) + setResponseStatus(event, 201) + } + else { + lazyLoadData[index] = parsed + setResponseStatus(event, 200) } - setResponseStatus(event, 201) + getRPC()?.onLazyLoadReport(parsed) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/lazy-load/handlers.ts` around lines 35 - 41, The POST handler currently ignores incoming data when an entry with the same id exists (it only pushes when not found) but still returns 201; change it to perform an upsert: locate the existing entry in lazyLoadData by id (using the existing findIndex logic), if index === -1 push parsed, call getRPC()?.onLazyLoadReport(parsed) and return 201; if found replace lazyLoadData[index] = parsed (or merge fields if partial updates are expected), call getRPC()?.onLazyLoadReport(parsed) and return 200 to indicate an update; ensure you still call setResponseStatus(event, ...) with the correct status and use the same parsed/getRPC identifiers shown in the diff.src/runtime/html-validate/api-handlers.ts (1)
21-31: Consider stricter validation for required fields.The validation only checks
body.idis a string, butHtmlValidateReportrequireshtml,results, andpathfields. Invalid payloads could be stored and cause issues in the client.Proposed stricter validation
export const postHandler = defineEventHandler(async (event) => { const body = await readBody<HtmlValidateReport>(event) - if (!body || typeof body.id !== 'string') { + if ( + !body + || typeof body.id !== 'string' + || typeof body.html !== 'string' + || typeof body.path !== 'string' + || !Array.isArray(body.results) + ) { throw createError({ statusCode: 400, statusMessage: 'Invalid payload' }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/html-validate/api-handlers.ts` around lines 21 - 31, postHandler currently only checks that body.id is a string which allows incomplete HtmlValidateReport objects to be stored; update the validation after readBody<HtmlValidateReport>(event) to assert that body.id, body.html, and body.path are non-empty strings and that body.results is an array (and optionally check array element shape or length), and if any check fails throw createError({ statusCode: 400, statusMessage: 'Invalid payload' }); only push to htmlValidateReports and call getRPC()?.onHtmlValidateReport(body) when the full shape is validated. Use the existing function names (postHandler, readBody, htmlValidateReports, getRPC()?.onHtmlValidateReport) so the change is easy to locate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/core/rpc.ts`:
- Line 5: The file src/runtime/core/rpc.ts is missing a newline at EOF (file
currently ends with a trailing "}" token); add a single newline character at the
end of the file so the file ends with an empty line to satisfy the
`@stylistic/eol-last` ESLint rule.
In `@src/runtime/html-validate/api-handlers.ts`:
- Around line 33-40: The handler deleteHandler currently throws a 404 when
event.context.params?.id is missing or not a string; change this to throw a 400
Bad Request instead. Locate deleteHandler in api-handlers.ts, and update the
error thrown in the typeof id !== 'string' branch to createError({ statusCode:
400, statusMessage: 'Bad Request' }) so invalid/missing id parameters return the
correct HTTP status.
---
Nitpick comments:
In `@client/app/composables/features.ts`:
- Around line 11-14: The two feature helper functions return different fallback
defaults when the devtools client is disconnected; update both useHintsFeature()
and useEnabledHintsFeatures() to return the same unified disconnected-state
defaults (e.g. hydration: true, lazyLoad: true, webVitals: false,
thirdPartyScripts: false, htmlValidate: true) so gating behavior is consistent
across helpers; locate the conditional that checks client?.host?.nuxt in each
function and replace the differing return objects with the single canonical
default object.
In `@src/runtime/html-validate/api-handlers.ts`:
- Around line 21-31: postHandler currently only checks that body.id is a string
which allows incomplete HtmlValidateReport objects to be stored; update the
validation after readBody<HtmlValidateReport>(event) to assert that body.id,
body.html, and body.path are non-empty strings and that body.results is an array
(and optionally check array element shape or length), and if any check fails
throw createError({ statusCode: 400, statusMessage: 'Invalid payload' }); only
push to htmlValidateReports and call getRPC()?.onHtmlValidateReport(body) when
the full shape is validated. Use the existing function names (postHandler,
readBody, htmlValidateReports, getRPC()?.onHtmlValidateReport) so the change is
easy to locate.
In `@src/runtime/hydration/handlers.ts`:
- Around line 11-19: clearHydrationMismatches currently calls
getRPC()?.onHydrationCleared(ids) with every requested id even if none were
removed; update clearHydrationMismatches to build an array (e.g., removedIds) of
only the ids that were actually found and spliced from hydrationMismatches (use
the existing loop over ids and push when index !== -1), and then call
getRPC()?.onHydrationCleared(removedIds) only if removedIds.length > 0;
reference function clearHydrationMismatches, the hydrationMismatches array, and
getRPC()?.onHydrationCleared when applying the change.
In `@src/runtime/lazy-load/handlers.ts`:
- Around line 35-41: The POST handler currently ignores incoming data when an
entry with the same id exists (it only pushes when not found) but still returns
201; change it to perform an upsert: locate the existing entry in lazyLoadData
by id (using the existing findIndex logic), if index === -1 push parsed, call
getRPC()?.onLazyLoadReport(parsed) and return 201; if found replace
lazyLoadData[index] = parsed (or merge fields if partial updates are expected),
call getRPC()?.onLazyLoadReport(parsed) and return 200 to indicate an update;
ensure you still call setResponseStatus(event, ...) with the correct status and
use the same parsed/getRPC identifiers shown in the diff.
🪄 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
Run ID: 617177d1-2205-416f-9546-4fdaf93bbb4c
📒 Files selected for processing (10)
client/app/app.vueclient/app/composables/features.tsclient/app/composables/host.tssrc/devtools-handlers.tssrc/devtools.tssrc/runtime/core/rpc.tssrc/runtime/html-validate/api-handlers.tssrc/runtime/hydration/handlers.tssrc/runtime/lazy-load/handlers.tssrc/runtime/types.d.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/devtools-handlers.ts
- src/runtime/types.d.ts
- client/app/app.vue
Comment on lines
+33
to
+40
| export const deleteHandler = defineEventHandler(async (event) => { | ||
| const id = event.context.params?.id | ||
| if (typeof id !== 'string') { | ||
| throw createError({ statusCode: 404, statusMessage: 'Not Found' }) | ||
| } | ||
| clearHtmlValidateReport(id) | ||
| setResponseStatus(event, 204) | ||
| }) |
There was a problem hiding this comment.
Use 400 for invalid parameter type, not 404.
When id is not a string (missing or wrong type), this is a bad request, not a "not found" scenario. HTTP 400 is more semantically correct.
Proposed fix
export const deleteHandler = defineEventHandler(async (event) => {
const id = event.context.params?.id
if (typeof id !== 'string') {
- throw createError({ statusCode: 404, statusMessage: 'Not Found' })
+ throw createError({ statusCode: 400, statusMessage: 'Missing or invalid ID parameter' })
}
clearHtmlValidateReport(id)
setResponseStatus(event, 204)
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/html-validate/api-handlers.ts` around lines 33 - 40, The handler
deleteHandler currently throws a 404 when event.context.params?.id is missing or
not a string; change this to throw a 400 Bad Request instead. Locate
deleteHandler in api-handlers.ts, and update the error thrown in the typeof id
!== 'string' branch to createError({ statusCode: 400, statusMessage: 'Bad
Request' }) so invalid/missing id parameters return the correct HTTP status.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/nitro.d.ts`:
- Line 7: The file src/runtime/nitro.d.ts ends with a closing brace '}' but is
missing a trailing newline which fails the `@stylistic/eol-last` lint rule; fix it
by adding a single newline character at the end of the file (after the final
'}') so the file ends with an EOF newline.
In `@test/unit/html-validate/nitro-plugin.test.ts`:
- Around line 70-75: The test's beforeEach stubs the global fetch with
vi.stubGlobal('fetch', vi.fn()) but never reverts it; add an afterEach that
unstubs the global to avoid leaking into other tests — e.g. add afterEach(() =>
{ vi.unstubGlobal('fetch') }) or afterEach(() => { vi.unstubAllGlobals() })
alongside the existing cleanup of htmlValidateReports and rpcMock in the same
test scope.
🪄 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
Run ID: bd31283e-8968-40a1-afb4-be9d7561fa39
📒 Files selected for processing (5)
src/runtime/core/rpc.tssrc/runtime/html-validate/api-handlers.tssrc/runtime/html-validate/nitro.plugin.tssrc/runtime/nitro.d.tstest/unit/html-validate/nitro-plugin.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/runtime/core/rpc.ts
- src/runtime/html-validate/nitro.plugin.ts
- src/runtime/html-validate/api-handlers.ts
Comment on lines
+70
to
+75
| beforeEach(() => { | ||
| htmlValidateReports.length = 0 | ||
| rpcMock.onHtmlValidateReport.mockClear() | ||
| rpcMock.onHtmlValidateDeleted.mockClear() | ||
| vi.stubGlobal('fetch', vi.fn()) | ||
| }) |
There was a problem hiding this comment.
Unstub global fetch after each test to prevent cross-test leakage.
On Line 74, vi.stubGlobal('fetch', vi.fn()) is never reverted. This can bleed into other test files and hide regressions.
🔧 Suggested fix
-import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
@@
describe('html-validate nitro plugin', () => {
beforeEach(() => {
htmlValidateReports.length = 0
rpcMock.onHtmlValidateReport.mockClear()
rpcMock.onHtmlValidateDeleted.mockClear()
vi.stubGlobal('fetch', vi.fn())
})
+
+ afterEach(() => {
+ vi.unstubAllGlobals()
+ })📝 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
| beforeEach(() => { | |
| htmlValidateReports.length = 0 | |
| rpcMock.onHtmlValidateReport.mockClear() | |
| rpcMock.onHtmlValidateDeleted.mockClear() | |
| vi.stubGlobal('fetch', vi.fn()) | |
| }) | |
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | |
| describe('html-validate nitro plugin', () => { | |
| beforeEach(() => { | |
| htmlValidateReports.length = 0 | |
| rpcMock.onHtmlValidateReport.mockClear() | |
| rpcMock.onHtmlValidateDeleted.mockClear() | |
| vi.stubGlobal('fetch', vi.fn()) | |
| }) | |
| afterEach(() => { | |
| vi.unstubAllGlobals() | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/unit/html-validate/nitro-plugin.test.ts` around lines 70 - 75, The
test's beforeEach stubs the global fetch with vi.stubGlobal('fetch', vi.fn())
but never reverts it; add an afterEach that unstubs the global to avoid leaking
into other tests — e.g. add afterEach(() => { vi.unstubGlobal('fetch') }) or
afterEach(() => { vi.unstubAllGlobals() }) alongside the existing cleanup of
htmlValidateReports and rpcMock in the same test scope.
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.
This PRs move SSE to devtools RPC so we stick close to devtool's API. (and also to test devtools v4 with vite-devtools)