Conversation
…file server utils + get endpoint, create profile on first login
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds user profiles and likes across client and server: new profile page at /profile/[handle] with edit/save and likes display; LikeCard component with optimistic like toggles and auth-modal integration; composables for profile likes; server API routes for profile GET/PUT and profile likes; AtProto utilities for identity resolution, profile and likes retrieval; OAuth scope expanded to include profile access and auth callback now fetches/creates an npmx profile; new lexicon schemas, shared types/schemas and session shape extended to include profile data. Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
lexicons/blue/microcosm/links/getBacklinksCount.json-12-15 (1)
12-15:⚠️ Potential issue | 🟡 MinorClarify
subjectconstraints: schema and description currently disagree.
format: "at-uri"conflicts with the description claiming DID/URI are also valid. Please align one source of truth to avoid client-side integration bugs.app/composables/atproto/useProfileLikes.ts-11-12 (1)
11-12:⚠️ Potential issue | 🟡 MinorUse a getter function for reactive key support.
The key is evaluated once when the composable is initialised. If
handleis reactive and changes, the key won't update, potentially causing stale data issues.Based on learnings about reactive keys in Nuxt 3, use a getter function to ensure the key reacts to handle changes:
♻️ Proposed fix
const asyncData = useLazyAsyncData( - `profile:${toValue(handle)}:likes`, + () => `profile:${toValue(handle)}:likes`, async (_nuxtApp, { signal }) => {server/api/social/profile/[identifier]/likes.get.ts-3-9 (1)
3-9:⚠️ Potential issue | 🟡 MinorAdd validation for the
identifierparameter.If
identifieris missing, passing an empty string togetMiniDocwill causeensureValidAtIdentifierto throw, resulting in an unhelpful 500 error. Validate the parameter and return a proper 400 response.Additionally,
PackageLikesUtilsappears to be missing an import (verify if auto-imported).🛡️ Proposed fix with validation
import { IdentityUtils } from '#server/utils/atproto/utils/identity' +import { PackageLikesUtils } from '#server/utils/atproto/utils/likes' export default defineEventHandler(async event => { const identifier = getRouterParam(event, 'identifier') + if (!identifier) { + throw createError({ + statusCode: 400, + message: 'Missing identifier parameter', + }) + } + const utils = new IdentityUtils() - const minidoc = await utils.getMiniDoc(identifier || '') + const minidoc = await utils.getMiniDoc(identifier) const likesUtil = new PackageLikesUtils() return likesUtil.getUserLikes(minidoc) })server/api/auth/atproto.get.ts-287-316 (1)
287-316:⚠️ Potential issue | 🟡 MinorAdd type annotation and error handling for
getNpmxProfile.The function has a few issues:
profileResponse.json()returnsany— no type validation is performed on the fetched profile.- Network errors during fetch are unhandled and will bubble up, causing the OAuth flow to fail silently.
- Return type is implicitly
any.Consider adding explicit typing and defensive error handling:
🛡️ Suggested improvements
-async function getNpmxProfile(handle: string, authSession: OAuthSession) { +async function getNpmxProfile(handle: string, authSession: OAuthSession): Promise<NPMXProfile> { const client = new Client(authSession) // get existing npmx profile OR create a new one const profileUri = `at://${client.did}/dev.npmx.actor.profile/self` - const profileResponse = await fetch( - `https://${SLINGSHOT_HOST}/xrpc/blue.microcosm.repo.getRecordByUri?at_uri=${profileUri}`, - { headers: { 'User-Agent': 'npmx' } }, - ) + let profileResponse: Response + try { + profileResponse = await fetch( + `https://${SLINGSHOT_HOST}/xrpc/blue.microcosm.repo.getRecordByUri?at_uri=${profileUri}`, + { headers: { 'User-Agent': 'npmx' } }, + ) + } catch { + // Network error - create default profile + const profile: NPMXProfile = { + website: '', + displayName: handle, + description: '', + } + await client.createRecord({ $type: 'dev.npmx.actor.profile', ...profile }, 'self') + return profile + } if (profileResponse.ok) { - const profile = await profileResponse.json() + const profile = (await profileResponse.json()) as NPMXProfile return profile } else {You'll also need to import the
NPMXProfiletype from#shared/types/social.app/pages/profile/[handle]/index.vue-176-182 (1)
176-182:⚠️ Potential issue | 🟡 MinorInvalid
:titlebinding -Likesis an undefined variable.The binding
:title="Likes"attempts to reference a variableLikeswhich doesn't exist. This should be a string literal.🐛 Proposed fix
<h2 class="font-mono text-2xl sm:text-3xl font-medium min-w-0 break-words" - :title="Likes" + title="Likes" dir="ltr" >app/pages/profile/[handle]/index.vue-14-16 (1)
14-16:⚠️ Potential issue | 🟡 MinorRoute typing and unused variable issues.
Static analysis indicates: (1) the route path argument is not in
RouteNamedMap, (2)routeris declared but never used, and (3)handleproperty doesn't exist on the route params type. Consider using a generic approach or removing unused code.♻️ Proposed fix
-const route = useRoute('/profile/[handle]') -const router = useRouter() -const handle = computed(() => route.params.handle) +const route = useRoute() +const handle = computed(() => route.params.handle as string)app/pages/profile/[handle]/index.vue-189-195 (1)
189-195:⚠️ Potential issue | 🟡 MinorPotential null access on
likesData.likes.recordswithout guard.When
statusis neither'pending'nor'error',likesData.likes.recordsis accessed but may still be undefined. The innerv-ifon line 191 is redundant with the outerv-else-ifon line 189. Additionally, add akeyattribute for thev-fordirective.♻️ Proposed fix with proper key
- <div v-else-if="likesData.likes.records" class="grid grid-cols-1 lg:grid-cols-2 gap-4"> + <div v-else-if="likesData?.likes?.records" class="grid grid-cols-1 lg:grid-cols-2 gap-4"> <PackageLikeCard - v-if="likesData.likes.records" v-for="like in likesData.likes.records" + :key="like.value.subjectRef" :packageUrl="like.value.subjectRef" /> </div>app/pages/profile/[handle]/index.vue-2-6 (1)
2-6:⚠️ Potential issue | 🟡 MinorRemove unused imports and type declaration.
Static analysis confirms these are unused:
debounce,normalizeSearchParam, andLikesResulttype. Remove to keep the code clean.🧹 Proposed fix
<script setup lang="ts"> -import { debounce } from 'perfect-debounce' -import { updateProfile as updateProfileUtil } from '~/utils/atproto/profile' -import { normalizeSearchParam } from '#shared/utils/url' - -type LikesResult = { - records: { - value: { - subjectRef: string - } - }[] -} +import { updateProfile as updateProfileUtil } from '~/utils/atproto/profile'
🧹 Nitpick comments (5)
lexicons/blue/microcosm/repo/get-record-by-uri.ts (2)
35-38: Tighten output type contract forcidif it is always returned.Line 37 currently makes
cidoptional, which weakens downstream type guarantees for consumers of this query. If the endpoint always resolves a concrete record version, makecidrequired in the schema.Suggested change
schema: object({ properties: { uri: required(string({ format: 'at-uri', description: 'at-uri for this record' })), - cid: string({ format: 'cid', description: 'CID for this exact version of the record' }), + cid: required( + string({ format: 'cid', description: 'CID for this exact version of the record' }), + ), value: required(unknown({ description: 'the record itself' })), }, }),As per coding guidelines, "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
11-44: Reduce schema drift risk between TS and JSON lexicon definitions.This document mirrors
lexicons/blue/microcosm/repo/getRecordByUri.jsonalmost verbatim. Consider a single source of truth (generate one from the other) to avoid silent divergence over time.app/components/Header/AuthModal.client.vue (1)
84-93: Prefer the global focus-visible style for buttons in this project.Please remove inline
focus-visible:*utility classes on these button elements and rely on the shared global button/select focus rule.Based on learnings: "In the npmx.dev project, ensure that focus-visible styling for button and select elements is implemented globally in app/assets/main.css ... Do not apply per-element inline utility classes ... Rely on the global rule for consistency and maintainability."
app/composables/atproto/useProfileLikes.ts (1)
1-7: Consider derivingLikesResultfrom server/shared types.The inline type definition works, but if the server's
getUserLikesreturn type is exported, consider importing it to ensure consistency and avoid type drift.server/api/social/profile/[identifier]/index.get.ts (1)
10-12: Consider returning 404 when profile is not found.
profileUtil.getProfile(identifier)may returnundefinedif the profile doesn't exist. Currently, this returnsundefinedto the client, which may be ambiguous. A 404 response would be clearer.♻️ Suggested fix
const profileUtil = new ProfileUtils() const profile = await profileUtil.getProfile(identifier) + if (!profile) { + throw createError({ + status: 404, + message: 'Profile not found', + }) + } return profile
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
app/components/Header/AuthModal.client.vueapp/components/Package/LikeCard.vueapp/composables/atproto/useProfileLikes.tsapp/pages/profile/[handle]/index.vueapp/utils/atproto/profile.tslexicons/blue/microcosm/identity/resolveMiniDoc.jsonlexicons/blue/microcosm/links/getBacklinks.jsonlexicons/blue/microcosm/links/getBacklinksCount.jsonlexicons/blue/microcosm/links/getManyToManyCounts.jsonlexicons/blue/microcosm/repo/get-record-by-uri.tslexicons/blue/microcosm/repo/getRecordByUri.jsonlexicons/dev/npmx/actor/profile.jsonserver/api/auth/atproto.get.tsserver/api/social/profile/[identifier]/index.get.tsserver/api/social/profile/[identifier]/index.put.tsserver/api/social/profile/[identifier]/likes.get.tsserver/utils/atproto/oauth.tsserver/utils/atproto/utils/identity.tsserver/utils/atproto/utils/likes.tsserver/utils/atproto/utils/profile.tsshared/schemas/social.tsshared/types/social.tsshared/types/userSession.tsshared/utils/constants.ts
| <NuxtLink | ||
| :to="{ name: 'profile-handle', params: { handle: user.handle } }" | ||
| :prefetch-on="prefetch ? 'visibility' : 'interaction'" | ||
| > | ||
| <button | ||
| class="w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50" | ||
| @click="authModal.close()" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the entire file to understand the component structure
wc -l app/components/Header/AuthModal.client.vueRepository: npmx-dev/npmx.dev
Length of output: 107
🏁 Script executed:
# Read the file to see the full context, focusing on the lines around 79-85
cat -n app/components/Header/AuthModal.client.vue | head -120 | tail -50Repository: npmx-dev/npmx.dev
Length of output: 2297
🏁 Script executed:
# Search for prefetch definition in the file
rg -n "prefetch" app/components/Header/AuthModal.client.vueRepository: npmx-dev/npmx.dev
Length of output: 128
🏁 Script executed:
# Search for NuxtLink and button elements to understand the structure
rg -n "NuxtLink|<button" app/components/Header/AuthModal.client.vue -B2 -A8Repository: npmx-dev/npmx.dev
Length of output: 1126
🏁 Script executed:
# Read the script section to check if prefetch is defined
cat -n app/components/Header/AuthModal.client.vue | head -70Repository: npmx-dev/npmx.dev
Length of output: 2672
🏁 Script executed:
# Search for all variable/prop definitions
rg -n "prefetch|defineProps|const prefetch|ref\(|computed\(" app/components/Header/AuthModal.client.vueRepository: npmx-dev/npmx.dev
Length of output: 128
🏁 Script executed:
# Check if there are other uses of prefetch-on in the codebase to understand the pattern
rg -n "prefetch-on" app/ --type vueRepository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Check other NuxtLink usages to see if they apply classes and click handlers
rg -n "NuxtLink" app/components/ -A3 | head -60Repository: npmx-dev/npmx.dev
Length of output: 4126
Remove undefined variable and fix invalid interactive element nesting.
The prefetch variable is not defined in the component (lines 1–63 of the script section), causing a type-check failure. Additionally, nesting a <button> inside <NuxtLink> violates HTML semantics for interactive elements. Apply styles and event handlers directly to NuxtLink instead:
Proposed fix
- <NuxtLink
- :to="{ name: 'profile-handle', params: { handle: user.handle } }"
- :prefetch-on="prefetch ? 'visibility' : 'interaction'"
- >
- <button
- class="w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50"
- `@click`="authModal.close()"
- >
- Profile
- </button>
- </NuxtLink>
+ <NuxtLink
+ :to="{ name: 'profile-handle', params: { handle: user.handle } }"
+ class="block w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50"
+ `@click`="authModal.close()"
+ >
+ Profile
+ </NuxtLink>📝 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.
| <NuxtLink | |
| :to="{ name: 'profile-handle', params: { handle: user.handle } }" | |
| :prefetch-on="prefetch ? 'visibility' : 'interaction'" | |
| > | |
| <button | |
| class="w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50" | |
| @click="authModal.close()" | |
| <NuxtLink | |
| :to="{ name: 'profile-handle', params: { handle: user.handle } }" | |
| class="block w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50" | |
| `@click`="authModal.close()" | |
| > | |
| Profile | |
| </NuxtLink> |
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 81-81:
Property 'prefetch' does not exist on type '{ user: Simplify<SerializeObject<{ did: string; handle: string; pds: string; avatar?: string | undefined; relogin?: boolean | undefined; }> | null> | undefined; ... 721 more ...; $npmApi: (url: string, options?: NitroFetchOptions<...> | undefined, ttl?: number | undefined) => Promise<...>; }'.
| import { Client } from '@atproto/lex' | ||
| import { ensureValidAtIdentifier } from '@atproto/syntax' | ||
| import * as blue from '#shared/types/lexicons/blue' | ||
|
|
||
| const HEADERS = { 'User-Agent': 'npmx' } | ||
|
|
||
| // Aggersive cache on identity since that doesn't change a ton | ||
| const CACHE_MAX_AGE_IDENTITY = CACHE_MAX_AGE_ONE_HOUR * 6 | ||
|
|
||
| const CACHE_KEY_IDENTITY = (identity: string) => `identity:${identity}` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full identity.ts file to understand its actual imports and content
wc -l server/utils/atproto/utils/identity.tsRepository: npmx-dev/npmx.dev
Length of output: 102
🏁 Script executed:
# Check for nuxt.config files
git ls-files | grep -E "(nuxt\.config|\.nuxtrc)" | head -5Repository: npmx-dev/npmx.dev
Length of output: 103
🏁 Script executed:
# Check how other server utils import these commonly used symbols
rg -A 5 "import.*getCacheAdapter|import.*CACHE_MAX_AGE_ONE_HOUR|import.*SLINGSHOT_HOST" server/utils --type ts | head -30Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
cat -n server/utils/atproto/utils/identity.tsRepository: npmx-dev/npmx.dev
Length of output: 1681
🏁 Script executed:
# Check nuxt.config.ts for auto-import configuration
cat nuxt.config.tsRepository: npmx-dev/npmx.dev
Length of output: 10471
🏁 Script executed:
# Find where these symbols are defined
echo "=== Searching for CACHE_MAX_AGE_ONE_HOUR ==="
rg "CACHE_MAX_AGE_ONE_HOUR" --type ts --type tsx -B 2 | head -20
echo ""
echo "=== Searching for getCacheAdapter definition ==="
rg "export.*getCacheAdapter|function getCacheAdapter" --type ts --type tsx | head -10
echo ""
echo "=== Searching for CacheAdapter type ==="
rg "export.*type CacheAdapter|export.*interface CacheAdapter|class CacheAdapter" --type ts --type tsx | head -10
echo ""
echo "=== Searching for SLINGSHOT_HOST ==="
rg "SLINGSHOT_HOST" --type ts --type tsx -B 2 | head -20Repository: npmx-dev/npmx.dev
Length of output: 363
🏁 Script executed:
# Try broader search without type restrictions
echo "=== Searching for CACHE_MAX_AGE_ONE_HOUR ==="
rg "CACHE_MAX_AGE_ONE_HOUR" -B 2 | head -30
echo ""
echo "=== Searching for getCacheAdapter ==="
rg "getCacheAdapter" -B 2 | head -30
echo ""
echo "=== Searching for CacheAdapter ==="
rg "CacheAdapter" -B 2 | head -30
echo ""
echo "=== Searching for SLINGSHOT_HOST ==="
rg "SLINGSHOT_HOST" -B 2 | head -30Repository: npmx-dev/npmx.dev
Length of output: 7186
🏁 Script executed:
# Check imports in files that use getCacheAdapter to see the pattern
echo "=== Checking oauth-session-store imports ==="
head -20 server/utils/atproto/oauth-session-store.ts
echo ""
echo "=== Checking oauth-state-store imports ==="
head -20 server/utils/atproto/oauth-state-store.ts
echo ""
echo "=== Checking profile.ts imports ==="
head -20 server/utils/atproto/utils/profile.tsRepository: npmx-dev/npmx.dev
Length of output: 2463
🏁 Script executed:
# Get full import sections for these files
echo "=== Full imports from oauth-session-store.ts ==="
sed -n '1,30p' server/utils/atproto/oauth-session-store.ts
echo ""
echo "=== Full imports from oauth-state-store.ts ==="
sed -n '1,30p' server/utils/atproto/oauth-state-store.ts
echo ""
echo "=== Full imports from profile.ts ==="
sed -n '1,40p' server/utils/atproto/utils/profile.tsRepository: npmx-dev/npmx.dev
Length of output: 3781
🏁 Script executed:
# Check if these symbols are auto-imported via Nitro config
echo "=== Checking for auto-imports in nuxt.config ==="
grep -A 20 "imports:" nuxt.config.ts
echo ""
echo "=== Checking for Nitro auto-imports in nuxt.config ==="
grep -A 30 "nitro:" nuxt.config.ts | head -40
echo ""
echo "=== Looking for .nitro.ts or nitro config files ==="
git ls-files | grep -i nitro
echo ""
echo "=== Checking server utils directory structure ==="
find server/utils -name "*.ts" -type f | head -15Repository: npmx-dev/npmx.dev
Length of output: 2038
🏁 Script executed:
# Check if there's a tsconfig or other config that enables auto-imports
echo "=== Checking for global auto-import configuration ==="
cat tsconfig.json 2>/dev/null || echo "No tsconfig.json at root"
echo ""
echo "=== Looking for .nuxtignore or auto-import config ==="
git ls-files | grep -E "(tsconfig|auto-import|\.nuxt)" | head -10
echo ""
echo "=== Check if oauth-session-store actually compiles/works ==="
# This file uses symbols without imports - let's see if it's actually used
rg "oauth-session-store" --type ts | head -5
echo ""
echo "=== Let's check if there's global type augmentation ==="
rg "declare global" --type ts -A 5 | head -30Repository: npmx-dev/npmx.dev
Length of output: 1076
🏁 Script executed:
# Check if Nitro has built-in auto-imports for server utils
echo "=== Checking package.json for Nitro config ==="
grep -A 20 "nitro" package.json 2>/dev/null || echo "No package.json or nitro config"
echo ""
echo "=== Check if there's a .nuxtrc configuration ==="
cat .nuxtrc
echo ""
echo "=== Let's verify oauth-session-store actually exists and is used ==="
head -50 server/utils/atproto/oauth-session-store.ts | grep -E "^import|CACHE_MAX_AGE|getCacheAdapter|CacheAdapter"
echo ""
echo "=== Check if these constants are re-exported somewhere accessible ==="
rg "export.*CACHE_MAX_AGE_ONE_HOUR|export.*getCacheAdapter|export.*CacheAdapter" server/ | head -10Repository: npmx-dev/npmx.dev
Length of output: 1002
🏁 Script executed:
# Check if there's a global server declarations file
echo "=== Looking for server declarations/augmentation files ==="
find server -name "*.d.ts" -o -name "tsconfig.json" | xargs grep -l "auto-import\|declare global" 2>/dev/null
echo ""
echo "=== Check app.config.ts or defineNuxtConfig for auto-imports ==="
git ls-files | xargs grep -l "defineNuxtConfig\|autoImports" 2>/dev/null | head -5
echo ""
echo "=== Let's look at .nuxt generated tsconfig if mentioned ==="
# The tsconfig.json references .nuxt files, which are generated
# Check if the actual identity.ts file has errors or just missing imports in the diff
echo "=== Verify: Does identity.ts actually have those imports? ==="
cat server/utils/atproto/utils/identity.ts | grep "^import"
echo ""
echo "=== Check: Are CACHE_MAX_AGE_ONE_DAY and related constants in a shared import path? ==="
rg "export const CACHE_MAX_AGE_ONE_DAY" shared/Repository: npmx-dev/npmx.dev
Length of output: 656
🏁 Script executed:
# Final check: Does oauth-session-store actually have these imports that I might have missed?
echo "=== Full oauth-session-store.ts file ==="
cat server/utils/atproto/oauth-session-store.ts
echo ""
echo "=== Full oauth-state-store.ts file ==="
cat server/utils/atproto/oauth-state-store.tsRepository: npmx-dev/npmx.dev
Length of output: 2315
🏁 Script executed:
# Check Nitro documentation or check if these are globally available in the server context
echo "=== Looking for Nitro presets or config that might enable auto-imports ==="
rg "nitro|Nitro" nuxt.config.ts -A 3 -B 1
echo ""
echo "=== Check if there's any server plugin that globalizes these ==="
git ls-files server/plugins/ 2>/dev/null || echo "No server/plugins"
echo ""
echo "=== Check if these are in any .d.ts declaration files ==="
git ls-files "**/*.d.ts" | xargs cat 2>/dev/null | head -50
echo ""
echo "=== Final verification: Are these really unimported or is there a Nitro magic? ==="
# Look for Nitro version in package-lock or package.json
git ls-files | xargs grep -h "\"nuxt\":|\"nitro\":" 2>/dev/null | head -5Repository: npmx-dev/npmx.dev
Length of output: 1890
🏁 Script executed:
# Check if there are server middleware or plugins that might declare these globally
echo "=== Checking server/plugins for global declarations ==="
cat server/plugins/oauth-client.ts | head -50
echo ""
echo "=== Checking if there's a server-specific tsconfig ==="
find . -path "./.nuxt" -prune -o -name "tsconfig.*.json" -type f -print | xargs cat 2>/dev/null | grep -A 20 "compilerOptions" | head -40
echo ""
echo "=== Check if these are Nitro auto-imports (documented in Nitro docs) ==="
# Nitro uses `#server`, `#shared` paths - let's check if that provides auto-imports
rg "#server|#shared" server/utils/atproto/ | head -10Repository: npmx-dev/npmx.dev
Length of output: 1753
Add missing imports and fix typo on line 7.
The file references CACHE_MAX_AGE_ONE_HOUR, getCacheAdapter, SLINGSHOT_HOST, and CacheAdapter without explicit imports. These will cause compilation errors. Add:
import { CACHE_MAX_AGE_ONE_HOUR, SLINGSHOT_HOST } from '#shared/utils/constants'
import { getCacheAdapter } from '#server/utils/cache/adapter'
import type { CacheAdapter } from '#server/utils/cache/shared'
Also, line 7: "Aggersive" → "Aggressive".
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
app/pages/profile/[handle]/index.vue (2)
4-5:⚠️ Potential issue | 🔴 CriticalFix dynamic route usage and normalise
handleto a string.Line 4 uses an invalid
useRouteargument for this page setup, and Line 5 leaveshandleas a non-normalised union type. This is causing current type-check failures and unsafe downstream usage.Proposed fix
-const route = useRoute('/profile/[handle]') -const handle = computed(() => route.params.handle) +const route = useRoute() +const handle = computed<string>(() => { + const raw = route.params.handle + if (Array.isArray(raw)) { + return raw[0] ?? '' + } + return raw ?? '' +})Nuxt 4.3.1 documentation: valid `useRoute()` usage in file-based routes and recommended handling for dynamic params typed as `string | string[] | undefined`.
7-13:⚠️ Potential issue | 🔴 CriticalAlign
useFetchtyping with the actual profile payload shape.The current annotation and
defaultpayload shape do not match the endpoint contract, and this cascades into theprofile.valuetype errors later in the file.Proposed fix
-const { data: profile }: { data?: NPMXProfile } = useFetch( +const { data: profile } = useFetch<NPMXProfile>( () => `/api/social/profile/${handle.value}`, { - default: () => ({ profile: { displayName: handle.value } }), + default: () => ({ displayName: handle.value, description: '', website: '' }), server: false, }, )Nuxt `useFetch` typing docs: generic usage for `data` ref and how `default` must conform to the declared response type.app/components/Package/LikeCard.vue (1)
103-103:⚠️ Potential issue | 🔴 CriticalCall
format()with a single argument only.Line 103 passes an unsupported second argument, which currently fails type-check.
Proposed fix
- compactNumberFormatter.format(likesData?.totalLikes ?? 0, { decimals: 1 }) + compactNumberFormatter.format(likesData?.totalLikes ?? 0)What is the method signature of `Intl.NumberFormat.prototype.format` and does it accept any options argument at call time?
🧹 Nitpick comments (2)
app/pages/profile/[handle]/index.vue (1)
121-121: Remove per-buttonfocus-visibleutility classes.These buttons should rely on the global button focus-visible rule instead of inline
focus-visible:outline-accent/70.Based on learnings: "focus-visible styling for button and select elements is implemented globally in app/assets/main.css ... Do not apply per-element inline utility classes like focus-visible:outline-accent/70 on these elements."
Also applies to: 128-128, 149-149
app/components/Package/LikeCard.vue (1)
96-97: Use UnoCSS icon colon syntax for Lucide classes.Prefer
i-lucide:heart-minus/i-lucide:heart-plusinstead of dash syntax in this project.Based on learnings: "In Vue components that use UnoCSS with the preset-icons collection, prefer colon-syntax for icons (e.g., i-carbon:checkmark) over the dash-separated form."
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/components/Header/AuthModal.client.vueapp/components/Package/LikeCard.vueapp/pages/profile/[handle]/index.vuei18n/locales/en.jsoni18n/schema.jsonlunaria/files/en-GB.jsonlunaria/files/en-US.jsonserver/api/auth/atproto.get.tsserver/api/social/profile/[identifier]/likes.get.tsserver/utils/atproto/oauth.tsserver/utils/atproto/utils/profile.ts
✅ Files skipped from review due to trivial changes (1)
- i18n/schema.json
🚧 Files skipped from review as they are similar to previous changes (3)
- app/components/Header/AuthModal.client.vue
- server/utils/atproto/utils/profile.ts
- server/utils/atproto/oauth.ts
| <NuxtLink :to="packageRoute(name)"> | ||
| <BaseCard class="group font-mono flex justify-between"> | ||
| {{ name }} | ||
| <div class="flex items-center gap-4 justify-between"> | ||
| <ClientOnly> | ||
| <TooltipApp | ||
| :text="likesData?.userHasLiked ? $t('package.likes.unlike') : $t('package.likes.like')" | ||
| position="bottom" | ||
| > | ||
| <button | ||
| @click.prevent="likeAction" | ||
| type="button" | ||
| :title=" | ||
| likesData?.userHasLiked ? $t('package.likes.unlike') : $t('package.likes.like') | ||
| " | ||
| class="inline-flex items-center gap-1.5 font-mono text-sm text-fg hover:text-fg-muted transition-colors duration-200" | ||
| :aria-label=" | ||
| likesData?.userHasLiked ? $t('package.likes.unlike') : $t('package.likes.like') | ||
| " | ||
| > | ||
| <span | ||
| :class=" | ||
| likesData?.userHasLiked | ||
| ? 'i-lucide-heart-minus text-red-500' | ||
| : 'i-lucide-heart-plus' | ||
| " | ||
| class="w-4 h-4" | ||
| aria-hidden="true" | ||
| /> | ||
| <span>{{ | ||
| compactNumberFormatter.format(likesData?.totalLikes ?? 0, { decimals: 1 }) | ||
| }}</span> | ||
| </button> | ||
| </TooltipApp> |
There was a problem hiding this comment.
Avoid nesting a <button> inside NuxtLink.
This creates invalid interactive nesting and can break keyboard/screen-reader behaviour. Split the clickable card link and like button into sibling interactive elements.
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 103-103:
Expected 1 arguments, but got 2.
| const npmxProfile = await getNpmxProfile(profile.handle, result.session) | ||
|
|
||
| await session.update({ public: profile }) | ||
| await session.update({ | ||
| public: profile, | ||
| profile: npmxProfile, | ||
| }) |
There was a problem hiding this comment.
Edge case: fallback handle value may propagate to profile creation.
When getMiniProfile fails to resolve the identity (line 248), it returns handle: 'Not available'. This value is then passed to getNpmxProfile, which would create a profile record with displayName: 'Not available' — a poor user experience that persists in their PDS.
Consider using authSession.did as a more meaningful fallback, or skipping profile creation when the handle cannot be resolved:
🛡️ Proposed defensive check
const profile = await getMiniProfile(result.session)
+ if (profile.handle === 'Not available') {
+ throw createError({
+ statusCode: 502,
+ message: 'Unable to resolve user identity. Please try again.',
+ })
+ }
const npmxProfile = await getNpmxProfile(profile.handle, result.session)📝 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.
| const npmxProfile = await getNpmxProfile(profile.handle, result.session) | |
| await session.update({ public: profile }) | |
| await session.update({ | |
| public: profile, | |
| profile: npmxProfile, | |
| }) | |
| const profile = await getMiniProfile(result.session) | |
| if (profile.handle === 'Not available') { | |
| throw createError({ | |
| statusCode: 502, | |
| message: 'Unable to resolve user identity. Please try again.', | |
| }) | |
| } | |
| const npmxProfile = await getNpmxProfile(profile.handle, result.session) | |
| await session.update({ | |
| public: profile, | |
| profile: npmxProfile, | |
| }) |
| async function getNpmxProfile(handle: string, authSession: OAuthSession) { | ||
| const client = new Client(authSession) | ||
|
|
||
| // get existing npmx profile OR create a new one | ||
| const profileUri = `at://${client.did}/dev.npmx.actor.profile/self` | ||
|
|
||
| // TODO: update with safe client rpc, see `getMiniProfile` response variable | ||
| const profileResponse = await fetch( | ||
| `https://${SLINGSHOT_HOST}/xrpc/blue.microcosm.repo.getRecordByUri?at_uri=${profileUri}`, | ||
| { headers: { 'User-Agent': 'npmx' } }, | ||
| ) | ||
|
|
||
| if (profileResponse.ok) { | ||
| const profile = await profileResponse.json() | ||
| return profile | ||
| } else { | ||
| const profile = { | ||
| website: '', | ||
| displayName: handle, | ||
| description: '', | ||
| } | ||
|
|
||
| await client.createRecord( | ||
| { | ||
| $type: 'dev.npmx.actor.profile', | ||
| ...profile, | ||
| }, | ||
| 'self', | ||
| ) | ||
|
|
||
| return profile | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit return type and wrap createRecord in error handling.
The function lacks type safety and consistent error handling:
- No explicit return type — the function returns
anyfrom JSON parsing or an inline object createRecord(lines 309-315) can throw but has no error handling, unlike the pattern used ingetMiniProfile
♻️ Proposed refactor for type safety and error handling
-async function getNpmxProfile(handle: string, authSession: OAuthSession) {
+type NpmxProfileData = {
+ website: string
+ displayName: string
+ description: string
+}
+
+async function getNpmxProfile(handle: string, authSession: OAuthSession): Promise<NpmxProfileData> {
const client = new Client(authSession)
// get existing npmx profile OR create a new one
const profileUri = `at://${client.did}/dev.npmx.actor.profile/self`
// ... fetch logic ...
if (profileResponse.ok) {
- const profile = await profileResponse.json()
- return profile
+ const data = await profileResponse.json()
+ // Consider validating against the lexicon schema here
+ return {
+ website: data.value?.website ?? '',
+ displayName: data.value?.displayName ?? handle,
+ description: data.value?.description ?? '',
+ }
} else {
const profile = {
website: '',
displayName: handle,
description: '',
}
- await client.createRecord(
- {
- $type: 'dev.npmx.actor.profile',
- ...profile,
- },
- 'self',
- )
+ try {
+ await client.createRecord(
+ {
+ $type: 'dev.npmx.actor.profile',
+ ...profile,
+ },
+ 'self',
+ )
+ } catch {
+ // Profile creation failed, continue with default values
+ // User can update their profile later
+ }
return profile
}
}As per coding guidelines: "Ensure you write strictly type-safe code" and "Use error handling patterns consistently".
| const likesUtil = new PackageLikesUtils() | ||
|
|
||
| return likesUtil.getUserLikes(minidoc) |
There was a problem hiding this comment.
Import PackageLikesUtils before instantiation.
PackageLikesUtils is used on Line 14 but never imported, which will break this route at runtime/type-check time.
Proposed fix
import { IdentityUtils } from '#server/utils/atproto/utils/identity'
+import { PackageLikesUtils } from '#server/utils/atproto/utils/likes'
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
app/pages/profile/[handle]/index.vue (1)
169-173:⚠️ Potential issue | 🟠 MajorGuard
likesDatabefore dereferencing likes records.
likesData.likes.recordsis read without an undefined guard in the branch condition and list rendering, which matches the current type-check failures on Lines 169-172.Suggested fix
- <div v-else-if="likesData.likes.records" class="grid grid-cols-1 lg:grid-cols-2 gap-4"> + <div v-else-if="likesData?.likes?.records" class="grid grid-cols-1 lg:grid-cols-2 gap-4"> <PackageLikeCard - v-if="likesData.likes.records" v-for="like in likesData.likes.records" + :key="like.value.subjectRef" :packageUrl="like.value.subjectRef" /> </div>server/api/auth/atproto.get.ts (2)
299-315:⚠️ Potential issue | 🟠 MajorNormalise
getNpmxProfileoutput and make profile creation non-fatal.On Line 300, the success path returns the raw XRPC payload, while Lines 303-317 return a flat profile object. This creates inconsistent session data on Lines 88-91. Also, a
createRecordfailure on Line 309 currently aborts callback completion.♻️ Proposed fix
+type NpmxProfileData = { + website: string + displayName: string + description: string +} + -async function getNpmxProfile(handle: string, authSession: OAuthSession) { +async function getNpmxProfile(handle: string, authSession: OAuthSession): Promise<NpmxProfileData> { const client = new Client(authSession) - - // get existing npmx profile OR create a new one const profileUri = `at://${client.did}/dev.npmx.actor.profile/self` - - // TODO: update with safe client rpc, see `getMiniProfile` response variable const profileResponse = await fetch( `https://${SLINGSHOT_HOST}/xrpc/blue.microcosm.repo.getRecordByUri?at_uri=${encodeURIComponent(profileUri)}`, { headers: { 'User-Agent': 'npmx' }, signal: AbortSignal.timeout(5_000) }, ) if (profileResponse.ok) { - const profile = await profileResponse.json() - return profile + const data = (await profileResponse.json()) as { value?: Partial<NpmxProfileData> } + return { + website: data.value?.website ?? '', + displayName: data.value?.displayName ?? handle, + description: data.value?.description ?? '', + } } else { const profile = { website: '', displayName: handle, description: '', } - - await client.createRecord( - { - $type: 'dev.npmx.actor.profile', - ...profile, - }, - 'self', - ) + try { + await client.createRecord( + { + $type: 'dev.npmx.actor.profile', + ...profile, + }, + 'self', + ) + } catch { + // Keep auth flow resilient; user can edit profile later. + } return profile } }#!/bin/bash set -euo pipefail echo "Inspect getNpmxProfile implementation:" sed -n '287,319p' server/api/auth/atproto.get.ts echo echo "Find profile field consumers expecting flat shape:" rg -nP --type=ts 'profile\.(displayName|description|website)|session\.update\(\{[^}]*profile:' echo echo "Locate getRecordByUri typing/schema references:" rg -nP 'getRecordByUri|blue\.microcosm\.repo|getRecordByUri'As per coding guidelines, "Ensure you write strictly type-safe code" and "Use error handling patterns consistently".
85-87:⚠️ Potential issue | 🟡 MinorGuard against persisting
'Not available'as a display name.If
getMiniProfilefalls back to'Not available', Line 86 passes that value into profile creation, which can persist poor profile data.🛡️ Proposed guard
const state = decodeOAuthState(event, result.state) const profile = await getMiniProfile(result.session) - const npmxProfile = await getNpmxProfile(profile.handle, result.session) + const handleForProfile = profile.handle === 'Not available' ? result.session.did : profile.handle + const npmxProfile = await getNpmxProfile(handleForProfile, result.session)
🧹 Nitpick comments (2)
app/components/Header/AuthModal.client.vue (1)
88-93: Use the project-wide focus-visible style for the logout button.Line 89 adds per-element
focus-visible:*utilities on a<button>, which duplicates the global button focus rule and should be removed for consistency.Based on learnings: In the npmx.dev project, focus-visible styling for
buttonandselectis defined globally inapp/assets/main.css, and per-element inline focus-visible utilities should not be added for those elements.app/pages/profile/[handle]/index.vue (1)
116-126: Use global button focus styling instead of per-element utilities.These button classes add
focus-visible:outline-accent/70, but this project applies button focus-visible styles globally.Based on learnings: focus-visible styling for
buttonandselectshould come from the globalapp/assets/main.cssrule, not inline per-element utility classes.Also applies to: 143-147
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/components/Header/AuthModal.client.vueapp/components/Package/LikeCard.vueapp/pages/profile/[handle]/index.vueknip.tsserver/api/auth/atproto.get.tsserver/utils/atproto/utils/identity.tsserver/utils/atproto/utils/profile.tsshared/schemas/social.tstest/nuxt/a11y.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- shared/schemas/social.ts
- server/utils/atproto/utils/identity.ts
- app/components/Package/LikeCard.vue
app/pages/profile/[handle]/index.vue
Outdated
| <button | ||
| @click="isEditing = false" | ||
| class="hidden sm:inline-flex link-subtle font-mono text-sm items-center gap-2 px-2 py-1.5 hover:bg-bg-subtle focus-visible:outline-accent/70 rounded" | ||
| > | ||
| {{ $t('common.cancel') }} | ||
| </button> | ||
| <button | ||
| @click.prevent="updateProfile" | ||
| :disabled="isUpdateProfileActionPending" | ||
| class="hidden sm:inline-flex link-subtle font-mono text-sm items-center gap-2 px-2 py-1.5 hover:bg-bg-subtle focus-visible:outline-accent/70 rounded" | ||
| > | ||
| {{ $t('common.save') }} | ||
| </button> |
There was a problem hiding this comment.
Profile editing controls are hidden on small screens.
Lines 116-128 and Line 143-147 use hidden sm:inline-flex, which hides Edit/Cancel/Save on mobile widths and blocks the edit flow there.
Suggested fix
- class="hidden sm:inline-flex link-subtle font-mono text-sm items-center gap-2 px-2 py-1.5 hover:bg-bg-subtle focus-visible:outline-accent/70 rounded"
+ class="inline-flex link-subtle font-mono text-sm items-center gap-2 px-2 py-1.5 hover:bg-bg-subtle focus-visible:outline-accent/70 rounded"Apply to the Edit, Cancel, and Save buttons, or provide equivalent mobile-visible controls.
Also applies to: 143-147
| const response = await fetch(resolveUrl, { | ||
| headers: { 'User-Agent': 'npmx' }, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to examine lines 30-32 and 63-74
cat -n server/utils/atproto/utils/profile.ts | sed -n '20,85p'Repository: npmx-dev/npmx.dev
Length of output: 2621
🏁 Script executed:
# Check for existing timeout patterns in the codebase
rg -n 'AbortSignal\.timeout|signal:.*timeout' --type=ts -C 2 | head -50Repository: npmx-dev/npmx.dev
Length of output: 820
🏁 Script executed:
# Verify the encoding pattern used elsewhere in the codebase
rg -n 'encodeURIComponent' --type=ts | head -20Repository: npmx-dev/npmx.dev
Length of output: 2663
Harden Slingshot fetches: add timeouts and distinguish missing profile from upstream failure.
Lines 30–32 and 63–68 lack timeout signals, and lines 69–74 silently swallow non-OK responses without distinguishing a missing profile (404) from upstream failures. The first fetch (line 33) correctly throws on error, but the second does not—violating consistent error handling. Timeouts and encoding patterns are already established elsewhere in the codebase (e.g., server/api/auth/atproto.get.ts line 296 applies both fixes to the same Slingshot endpoint).
🛠️ Proposed fix
const response = await fetch(resolveUrl, {
headers: { 'User-Agent': 'npmx' },
+ signal: AbortSignal.timeout(5_000),
})
if (!response.ok) {
throw createError({
status: response.status,
message: `Failed to resolve MiniDoc for ${handle}`,
})
}
@@
const profileUri = `at://${miniDoc.did}/dev.npmx.actor.profile/self`
const response = await fetch(
- `https://${SLINGSHOT_HOST}/xrpc/blue.microcosm.repo.getRecordByUri?at_uri=${profileUri}`,
+ `https://${SLINGSHOT_HOST}/xrpc/blue.microcosm.repo.getRecordByUri?at_uri=${encodeURIComponent(profileUri)}`,
{
headers: { 'User-Agent': 'npmx' },
+ signal: AbortSignal.timeout(5_000),
},
)
if (response.ok) {
const { value } = (await response.json()) as { value: NPMXProfile }
profile = value
await this.cache.set(profileKey, profile, CACHE_MAX_AGE)
+ } else if (response.status !== 404) {
+ throw createError({
+ status: response.status,
+ message: `Failed to fetch profile for DID ${miniDoc.did}`,
+ })
}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/pages/profile/[handle]/index.vue (1)
170-175:⚠️ Potential issue | 🟡 MinorMissing
:keyattribute inv-forloop.Vue requires a unique
:keybinding forv-forto efficiently track and update list items. ThesubjectRefappears to be a suitable unique identifier.🔧 Proposed fix
<div v-else-if="likesData?.likes?.records" class="grid grid-cols-1 lg:grid-cols-2 gap-4"> <PackageLikeCard v-for="like in likesData.likes.records" + :key="like.value.subjectRef" :packageUrl="like.value.subjectRef" /> </div>
| watchEffect(() => { | ||
| if (isEditing.value) { | ||
| if (profile) { | ||
| displayNameInput.value = profile.value.displayName | ||
| descriptionInput.value = profile.value.description | ||
| websiteInput.value = profile.value.website | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Guard should check profile.value, not profile.
profile is a Ref object which is always truthy. The condition should check profile.value to properly guard against null/undefined profile data before accessing its properties.
🐛 Proposed fix
watchEffect(() => {
if (isEditing.value) {
- if (profile) {
+ if (profile.value) {
displayNameInput.value = profile.value.displayName
descriptionInput.value = profile.value.description
websiteInput.value = profile.value.website
}
}
})📝 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.
| watchEffect(() => { | |
| if (isEditing.value) { | |
| if (profile) { | |
| displayNameInput.value = profile.value.displayName | |
| descriptionInput.value = profile.value.description | |
| websiteInput.value = profile.value.website | |
| } | |
| } | |
| }) | |
| watchEffect(() => { | |
| if (isEditing.value) { | |
| if (profile.value) { | |
| displayNameInput.value = profile.value.displayName | |
| descriptionInput.value = profile.value.description | |
| websiteInput.value = profile.value.website | |
| } | |
| } | |
| }) |
Co-authored-by: Brandon Hurrington <brandon.o.hurrington@gmail.com>
Co-authored-by: Brandon Hurrington <brandon.o.hurrington@gmail.com>
3 participants
Implements
dev.npmx.actor.profilelexicon and/profile/[handle]page that displays the record.When the user logs in via oauth, it will automatically create a profile record in their PDS with their domain as the display name. A user can edit their profile, which also includes the optional description and website fields. Users can also like and unlike packages from profile pages.
Fixes #616
Fixes #923
npmxprofilefull.mp4