fix: address vite doctor diagnostics#2261
Conversation
|
@onmax is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
d0c0c3c to
47e0744
Compare
|
Warning Review limit reached
More reviews will be available in 11 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR updates three interconnected systems. On the client, Vue component watchers are deferred to post-flush timing for scroll and load-more handlers, composables are refactored to use ISO string serialization (useDateRange), synchronous reactive calls (useSponsors), and safer head registration (useCanonical), and element attributes are added for button types and responsive image sizing. On the server, team authorization and member-checking utilities accept H3Event to enable event-scoped runtime config and fetch operations, and three API handlers now pass the event through to these authorization functions for proper environment-aware configuration access. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/team.ts (1)
9-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
loginparameter should be normalized insideisCoreTeamMember.The comparison on line 17 lowercases
member.loginbut compares tologinas-is. Since this function is exported, callers could pass mixed-case logins directly (e.g.,"UserName"), causing the comparison to fail.Currently
isAuthorizedAdminpre-normalizes, but the exported contract is unclear.🛡️ Proposed fix to normalize inside the function
export async function isCoreTeamMember(event: H3Event, login: string): Promise<boolean> { + const normalizedLogin = login.toLowerCase() const coreMembers = await getCoreMembers(event) if (!coreMembers || !Array.isArray(coreMembers)) { throw createError({ statusCode: 500, statusMessage: 'Failed to fetch core team members.' }) } - return coreMembers.some(member => member.login.toLowerCase() === login) + return coreMembers.some(member => member.login.toLowerCase() === normalizedLogin) }🤖 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 `@server/utils/team.ts` around lines 9 - 18, The isCoreTeamMember function compares member.login.toLowerCase() to the incoming login without normalizing the login param; update isCoreTeamMember to normalize the incoming login (e.g., trim() and toLowerCase()) before the comparison so callers can pass mixed-case values; locate isCoreTeamMember and ensure you normalize the login variable prior to returning coreMembers.some(...) (getCoreMembers remains unchanged and keep the existing createError behavior).
🧹 Nitpick comments (1)
server/utils/team.ts (1)
4-7: 💤 Low valueStatic cache key means
eventis only used on first invocation.
getCoreMembersacceptseventforevent.$fetch, but the cache key is static ('core-members'). After the first call within the 7-day cache window, subsequent calls return the cached result without invoking the function—effectively ignoring theeventparameter.For fetching immutable team data this may be acceptable, but if request-specific behavior (e.g., headers forwarding) is expected on every call, this pattern won't provide it.
🤖 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 `@server/utils/team.ts` around lines 4 - 7, getCoreMembers is using a static cache key ('core-members') so the event parameter (and any per-request headers) is only used the first time; subsequent calls within the cache window will return the cached result and ignore event.$fetch. Fix by making the cache key depend on the request when needed (e.g., change getKey to accept event and return a key that includes any request-specific bits such as an authorization header or host, e.g. getKey: (event) => `core-members:${event?.node?.req?.headers['authorization']||''}`), or if the data is truly immutable remove the event parameter from getCoreMembers to avoid confusion; update the cachedFunction invocation accordingly so event.$fetch is invoked on every call when per-request behavior is required.
🤖 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 `@app/composables/useDateRange.ts`:
- Around line 13-18: createDefaultDateRange() currently returns raw timestamps
which are narrower than the normalized ranges created by setDateRange(); update
createDefaultDateRange to compute the end as endOfDay(new Date()) and the start
as startOfDay(subDays(end, 30)) (then toISOString both) so the initial "Last 30
days" matches the same startOfDay/endOfDay normalization used by setDateRange().
---
Outside diff comments:
In `@server/utils/team.ts`:
- Around line 9-18: The isCoreTeamMember function compares
member.login.toLowerCase() to the incoming login without normalizing the login
param; update isCoreTeamMember to normalize the incoming login (e.g., trim() and
toLowerCase()) before the comparison so callers can pass mixed-case values;
locate isCoreTeamMember and ensure you normalize the login variable prior to
returning coreMembers.some(...) (getCoreMembers remains unchanged and keep the
existing createError behavior).
---
Nitpick comments:
In `@server/utils/team.ts`:
- Around line 4-7: getCoreMembers is using a static cache key ('core-members')
so the event parameter (and any per-request headers) is only used the first
time; subsequent calls within the cache window will return the cached result and
ignore event.$fetch. Fix by making the cache key depend on the request when
needed (e.g., change getKey to accept event and return a key that includes any
request-specific bits such as an authorization header or host, e.g. getKey:
(event) => `core-members:${event?.node?.req?.headers['authorization']||''}`), or
if the data is truly immutable remove the event parameter from getCoreMembers to
avoid confusion; update the cachedFunction invocation accordingly so
event.$fetch is invoked on every call when per-request behavior is required.
🪄 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: ee0e771a-bb50-4308-a521-4889b0ff3b91
📒 Files selected for processing (12)
app/components/AdminDashboard.vueapp/components/chat/ChatPanel.vueapp/composables/useCanonical.tsapp/composables/useDateRange.tsapp/composables/useSponsors.tsapp/pages/deploy/index.vueapp/pages/modules/index.vueapp/pages/video-courses.vueserver/api/admin/mcp-install.get.tsserver/api/auth/github.get.tsserver/api/v1/teams/index.get.tsserver/utils/team.ts
A form-associated <button> defaults to type="submit", not type="button". The suggestion tiles trigger a chat action, so declaring type="button" keeps them inert if this prompt area is ever wrapped by a form. Docs: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/button#type
Nuxt useState is serialized through the payload, so the state value should stay JSON-safe. Store ISO strings there and expose Date objects through a computed wrapper so consumers keep the same API without relying on Date instances surviving hydration. The default range also uses the same startOfDay/endOfDay normalization as setDateRange. That keeps the first "Last 30 days" window aligned with picker and preset updates instead of using raw timestamps. Docs: https://nuxt.com/docs/4.x/getting-started/state-management Docs: https://nuxt.com/docs/api/composables/use-state
Nuxt documents useFetch as a composable that should be called directly in setup, plugins, or route middleware so it can register payload state for hydration. Calling useFetch and useAsyncData directly in this composable keeps the Nuxt data primitives visible instead of hiding them behind a generic Promise.all boundary. The call sites can still include useSponsors() in their existing Promise.all arrays; Promise.all accepts plain values, and the composable keeps returning the same sponsors helpers. Docs: https://nuxt.com/docs/4.x/api/composables/use-fetch
Nuxt server-route docs say request headers and request context are not forwarded by default for server-side fetches, and event.$fetch should be used when that context should follow the internal request. The same server docs recommend passing event to useRuntimeConfig in server routes so runtime environment overrides are resolved against the active request. This keeps the admin/team helpers on request-aware server primitives instead of ambient global fetch/config access. Docs: https://nuxt.com/docs/4.x/directory-structure/server#forwarding-context-headers Docs: https://nuxt.com/docs/4.x/directory-structure/server#runtime-config
Nuxt recommends useHeadSafe when head data is derived from input because it restricts rendered head fields to a safe attribute whitelist. The canonical composable builds links from route and optional content-derived markdown paths, and useHeadSafe still allows the rel, href, type, and related link attributes this helper emits. Docs: https://nuxt.com/docs/4.x/api/composables/use-head-safe
Vue runs normal watcher callbacks before the owner component DOM has updated. These watchers read or mutate DOM state after reactive values change, so flush="post" schedules them after Vue applies the render update. Docs: https://vuejs.org/guide/essentials/watchers.html#callback-flush-timing
Nuxt Image renders native img attributes and documents that alt should always be provided. Deployment logos carry information, so the logo image gets an explicit label. Nuxt Image also generates responsive candidates from the sizes option. The video-course logos are fixed-size images, so the sizes value now matches the actual sponsor/non-sponsor rendered width instead of forcing a wider candidate for every course. Docs: https://image.nuxt.com/usage/nuxt-img#alt Docs: https://image.nuxt.com/usage/nuxt-img#sizes
3fa9adb to
75a4e85
Compare
This is an experiment from running the Vite Doctor CLI against nuxt.com. The project rules are documented at https://vite-doctor.onmax.me/nuxt/rules.
I ran the CLI on origin/main and on this draft branch so maintainers can compare the before/after output below. More context for each change is in the individual commits.
Before: Vite Doctor on origin/main (0 blockers, 4 errors, 33 warnings, 50 info)
After: Vite Doctor on this draft branch (0 blockers, 0 errors, 24 warnings, 50 info)
PR made with codex