fix: resolve multiple UI, UX, and a11y issues#2201
fix: resolve multiple UI, UX, and a11y issues#2201thealxlabs wants to merge 22 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
📝 WalkthroughWalkthroughThis pull request adds a new package dependents feature, improves code viewing capabilities, and refines UI components and styling across the application. Changes include a new dependents page and server endpoint for discovering packages that depend on a given package, word-wrap functionality for the code viewer with a UI toggle, npm alias parsing in the dependencies component, licence change detection on the package page, footer restructuring into a responsive grid layout, enhanced touch device support for copy buttons, and improved search pagination. Additionally, numerous i18n translation keys are added to support these new features. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 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 Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
app/components/CollapsibleSection.vue (1)
109-109: Remove inlinefocus-visibleutility from this button class.Use the project’s global button focus-visible styling instead of per-element utility classes to keep focus behaviour consistent.
Suggested fix
- class="text-start focus-visible:outline-accent/70 rounded" + class="text-start rounded"Based on learnings: in this repo, button/select focus-visible styling is handled globally in
app/assets/main.css, and inlinefocus-visible:outline-accent/70should not be added on individual buttons.app/components/Compare/FacetBarChart.vue (1)
348-354: Remove non-essential template comments in this block.The comments at Line 348 and Line 353 describe obvious markup and can be dropped to keep the template lean.
♻️ Suggested cleanup
- <!-- Title skeleton --> <div class="flex flex-col gap-2 justify-center items-center mb-4" aria-hidden="true"> <SkeletonInline class="h-4 w-20 rounded" /> <SkeletonInline class="h-3 w-36 rounded" /> </div> - <!-- Bar skeletons with varying widths for visual realism --> <div class="flex flex-col gap-2 px-2" aria-hidden="true">As per coding guidelines, "Add comments only to explain complex logic or non-obvious implementations".
app/components/Code/Viewer.vue (1)
159-172: Dead CSS selector:.word-wrap-activeis never applied.The class
word-wrap-activeis never added to any element in the template. Line 117 adds overflow classes directly, and line 119 addsword-wrapto.code-lines, not.code-content. The selector.code-content.word-wrap-activewill never match.The
:has(.word-wrap)selector on line 160 and.code-lines.word-wrapon line 167 both work correctly, making lines 159-165 partially redundant.🧹 Proposed fix to remove dead selector
-.code-content.word-wrap-active :deep(.line), .code-content:has(.word-wrap) :deep(.line) { white-space: pre-wrap; overflow-wrap: break-word; max-height: none; overflow: visible; }app/pages/package/[[org]]/[name]/dependents.vue (1)
66-78: Consider usingscrollMarginfromdefinePageMetainstead of manual scroll.The page defines
scrollMargin: 200indefinePageMeta(line 4), butprevPageandnextPageusewindow.scrollTo({ top: 0 }). If thescrollMarginis intended to affect pagination scrolling, consider using it consistently. Otherwise, the current approach is fine for a simple scroll-to-top behaviour.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb889fd3-6312-4b3a-ad6e-fe5b5a537079
📒 Files selected for processing (17)
app/components/AppFooter.vueapp/components/AppHeader.vueapp/components/Button/Base.vueapp/components/Code/Viewer.vueapp/components/CollapsibleSection.vueapp/components/Compare/FacetBarChart.vueapp/components/CopyToClipboardButton.vueapp/components/Package/Dependencies.vueapp/components/Package/Header.vueapp/pages/blog/index.vueapp/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vueapp/pages/package/[[org]]/[name].vueapp/pages/package/[[org]]/[name]/dependents.vueapp/pages/search.vuei18n/locales/en.jsonserver/api/registry/dependents/[...pkg].get.tsserver/utils/readme.ts
| <button | ||
| type="button" | ||
| class="text-start focus-visible:outline-accent/70 rounded" | ||
| :aria-label="`${isOpen ? 'Collapse' : 'Expand'} ${title}`" | ||
| @click="toggle" |
There was a problem hiding this comment.
Expose disclosure semantics on the title toggle button.
This button also controls the collapsible content, but it currently omits the standard aria-expanded and aria-controls relationship/state on this control.
Suggested fix
<button
type="button"
class="text-start focus-visible:outline-accent/70 rounded"
- :aria-label="`${isOpen ? 'Collapse' : 'Expand'} ${title}`"
+ :aria-expanded="isOpen"
+ :aria-controls="contentId"
+ :aria-label="ariaLabel"
`@click`="toggle"
>📝 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.
| <button | |
| type="button" | |
| class="text-start focus-visible:outline-accent/70 rounded" | |
| :aria-label="`${isOpen ? 'Collapse' : 'Expand'} ${title}`" | |
| @click="toggle" | |
| <button | |
| type="button" | |
| class="text-start focus-visible:outline-accent/70 rounded" | |
| :aria-expanded="isOpen" | |
| :aria-controls="contentId" | |
| :aria-label="ariaLabel" | |
| `@click`="toggle" |
| // Detect license changes between current version and latest | ||
| const licenseChanged = computed(() => { | ||
| const currentLicense = displayVersion.value?.license | ||
| const latestLicense = latestVersion.value?.license ?? pkg.value?.license | ||
| if (!currentLicense || !latestLicense) return false | ||
| // Normalize: compare string representations | ||
| const normalize = (l: unknown): string => | ||
| typeof l === 'string' ? l : (l as { type?: string })?.type ?? '' | ||
| return normalize(currentLicense) !== normalize(latestLicense) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find SlimVersion type definition
ast-grep --pattern 'interface SlimVersion {
$$$
}'
# Also check PackumentVersion for comparison
ast-grep --pattern 'interface PackumentVersion {
$$$
}'Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for SlimVersion type definition
rg "interface SlimVersion|type SlimVersion" -A 10
# Also search for PackumentVersion
rg "interface PackumentVersion|type PackumentVersion" -A 10Repository: npmx-dev/npmx.dev
Length of output: 1674
🏁 Script executed:
#!/bin/bash
# Check the vue file for displayVersion and latestVersion declarations
head -350 app/pages/package/[[org]]/[name].vue | tail -50Repository: npmx-dev/npmx.dev
Length of output: 1556
🏁 Script executed:
#!/bin/bash
# Search for where SlimVersion is imported or used in the codebase
rg "SlimVersion" -B 2 -A 2Repository: npmx-dev/npmx.dev
Length of output: 4706
🏁 Script executed:
#!/bin/bash
# Read the vue file around lines 300-350 to see variable declarations and type annotations
sed -n '280,340p' app/pages/package/[[org]]/[name].vueRepository: npmx-dev/npmx.dev
Length of output: 1893
🏁 Script executed:
#!/bin/bash
# Find where displayVersion and latestVersion are typed in the vue file
rg "displayVersion|latestVersion" app/pages/package/[[org]]/[name].vue -B 2 -A 2 | head -80Repository: npmx-dev/npmx.dev
Length of output: 2561
🏁 Script executed:
#!/bin/bash
# Check the type of pkg in the vue file
rg "const pkg|interface.*Pkg|type.*Pkg" app/pages/package/[[org]]/[name].vue -A 3Repository: npmx-dev/npmx.dev
Length of output: 178
🏁 Script executed:
#!/bin/bash
# Check what SlimPackumentVersion includes (to understand SlimVersion better)
rg "interface SlimPackumentVersion|type SlimPackumentVersion" shared/types/npm-registry.ts -A 15Repository: npmx-dev/npmx.dev
Length of output: 580
🏁 Script executed:
#!/bin/bash
# Find where pkg is defined in the page component - look for computed or ref declarations
rg "const pkg|ref<|computed<" app/pages/package/[[org]]/[name].vue | head -20Repository: npmx-dev/npmx.dev
Length of output: 175
🏁 Script executed:
#!/bin/bash
# Get more context around pkg definition
sed -n '1,100p' app/pages/package/[[org]]/[name].vue | grep -A 5 "const pkg"Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check what PackageResponse or the pkg's interface actually is
rg "interface PackageResponse|type PackageResponse" shared/types -A 20Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for useAsyncData or useFetch that assigns pkg
rg "useAsyncData|useFetch" app/pages/package/[[org]]/[name].vue -A 3 | head -40Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for where pkg.value.requestedVersion is defined - check the API route
rg "requestedVersion" shared/types -B 5 -A 5Repository: npmx-dev/npmx.dev
Length of output: 853
🏁 Script executed:
#!/bin/bash
# Look at the entire start of the script section to find pkg definition
head -200 app/pages/package/[[org]]/[name].vue | grep -A 20 "const pkg\|definePageMeta\|useAsyncData"Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Find the complete Packument type definition
rg "export type Packument|export interface Packument" shared/types/npm-registry.ts -A 30Repository: npmx-dev/npmx.dev
Length of output: 1468
🏁 Script executed:
#!/bin/bash
# Check if there's a response type for the package page endpoint
rg "requestedVersion" app/ -B 10 -A 2 | head -50Repository: npmx-dev/npmx.dev
Length of output: 3265
🏁 Script executed:
#!/bin/bash
# Verify PackumentVersionWithoutAttestations has license property
rg "interface PackumentVersionWithoutAttestations|type PackumentVersionWithoutAttestations" shared/types/npm-registry.ts -A 30Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the actual computed return type by looking at type inference
# See if there are any explicit type annotations on these computeds
sed -n '200,350p' app/pages/package/[[org]]/[name].vue | grep -B 3 -A 8 "const displayVersion\|const latestVersion"Repository: npmx-dev/npmx.dev
Length of output: 892
🏁 Script executed:
#!/bin/bash
# Run TypeScript to check for actual compilation errors on this file
# First, find tsconfig
find . -name "tsconfig.json" -o -name "tsconfig.*.json" | head -5Repository: npmx-dev/npmx.dev
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Let's check if there are TypeScript errors in this file
# Look at the package.json to understand the build setup
cat package.json | grep -A 20 "scripts"Repository: npmx-dev/npmx.dev
Length of output: 3031
🏁 Script executed:
#!/bin/bash
# Check if the vue file has explicit type annotations for these computed properties
sed -n '200,250p' app/pages/package/[[org]]/[name].vue | cat -nRepository: npmx-dev/npmx.dev
Length of output: 2508
🏁 Script executed:
#!/bin/bash
# Most importantly, let's trace the exact types:
# displayVersion comes from pkg.value?.requestedVersion
# requestedVersion is SlimPackumentVersion | null
# latestVersion comes from pkg.value.versions[latestTag]
# versions is Record<string, SlimVersion>
# So let's verify if SlimPackumentVersion has license
rg "interface SlimPackumentVersion|type SlimPackumentVersion" shared/types/npm-registry.ts -B 5 -A 3Repository: npmx-dev/npmx.dev
Length of output: 297
🏁 Script executed:
#!/bin/bash
# Check what PackumentVersionWithoutAttestations includes
rg "PackumentVersionWithoutAttestations" shared/types/npm-registry.ts -B 10 -A 15 | head -80Repository: npmx-dev/npmx.dev
Length of output: 1525
🏁 Script executed:
#!/bin/bash
# Confirm that PackumentVersionWithoutAttestations has license by checking `@npm/types`
# First let's see what the actual node_modules has
find node_modules/@npm/types -name "*.d.ts" 2>/dev/null | head -5Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Let's check if there's a way to run the type checker on this specific file
# Look for the vue-tsc command that's in the test:types script
which vue-tsc || echo "vue-tsc not in PATH, checking package.json"Repository: npmx-dev/npmx.dev
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Let me check the exact structure of what's available on these types
# Create a simple test to understand the type chain
cat > /tmp/test-types.ts << 'EOF'
import type { SlimVersion, SlimPackumentVersion, Packument } from './shared/types/npm-registry'
// Check what properties are available
type SVProps = keyof SlimVersion
type SPVProps = keyof SlimPackumentVersion
// This would error if license doesn't exist
// type TestSV = SlimVersion['license']
// type TestSPV = SlimPackumentVersion['license']
EOF
cat /tmp/test-types.tsRepository: npmx-dev/npmx.dev
Length of output: 408
🏁 Script executed:
#!/bin/bash
# Let's trace PackumentVersionWithoutAttestations from `@npm/types`
# It's imported from `@npm/types` package, let's check what properties it should have
# according to npm's type definition
# Search for any local overrides or extensions
rg "PackumentVersionWithoutAttestations" shared/types/ -A 5 -B 5Repository: npmx-dev/npmx.dev
Length of output: 1713
🏁 Script executed:
#!/bin/bash
# Let me trace the actual type of pkg to confirm it has license
# usePackage composable should return Packument-like type
rg "export.*usePackage|function usePackage" app/composables -A 20 | head -50Repository: npmx-dev/npmx.dev
Length of output: 3394
🏁 Script executed:
#!/bin/bash
# Check what usePackage actually returns
rg "usePackage" app/composables/npm/usePackage.ts -A 30 | head -80Repository: npmx-dev/npmx.dev
Length of output: 789
🏁 Script executed:
#!/bin/bash
# Now let's check transformPackument to see how it converts types
rg "transformPackument" app/composables/npm/usePackage.ts -B 5 -A 50Repository: npmx-dev/npmx.dev
Length of output: 2719
🏁 Script executed:
#!/bin/bash
# Search for transformPackument definition
rg "export.*transformPackument|function transformPackument" app/ -A 30Repository: npmx-dev/npmx.dev
Length of output: 2336
🏁 Script executed:
#!/bin/bash
# Check SlimPackument type definition
rg "interface SlimPackument|type SlimPackument" shared/types/npm-registry.ts -B 5 -A 30Repository: npmx-dev/npmx.dev
Length of output: 2031
🏁 Script executed:
#!/bin/bash
# Let me check the full transformPackument to see what SlimPackument structure is
rg "SlimPackument" app/composables/npm/usePackage.ts -B 10 -A 10Repository: npmx-dev/npmx.dev
Length of output: 1742
🏁 Script executed:
#!/bin/bash
# Check if SlimPackument is defined in types
rg "SlimPackument" shared/types -B 2 -A 20Repository: npmx-dev/npmx.dev
Length of output: 4082
🏁 Script executed:
#!/bin/bash
# Let's check if there are any actual TypeScript errors reported when building
# Look for the actual `@npm/types` to understand what PackumentVersion includes
rg "license" node_modules/@npm/types/index.d.ts 2>/dev/null | head -20Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Simpler approach: check if `@npm/types` is installed and what it exports
ls -la node_modules/@npm/types/ 2>/dev/null | head -20Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Let's look for any test files that might show the expected types or behaviors
rg "displayVersion.*license|latestVersion.*license" test/ app/ --type ts --type vueRepository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Check if there are any existing TypeScript errors or type definitions for this component
# Look at the vue file more carefully for any explicit type annotations
sed -n '1,50p' app/pages/package/[[org]]/[name].vueRepository: npmx-dev/npmx.dev
Length of output: 1500
🏁 Script executed:
#!/bin/bash
# Check if there are any TypeScript errors reported for this file
# Look for any type checking configuration
cat tsconfig.json | grep -A 10 '"compilerOptions"'Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for any existing TypeScript errors or suppressions near the licenseChanged computed
rg "licenseChanged" app/pages/package/[[org]]/[name].vue -B 5 -A 15Repository: npmx-dev/npmx.dev
Length of output: 2043
🏁 Script executed:
#!/bin/bash
# Let me check the template more carefully to see the actual usage of these properties
# and confirm this is actually being accessed
rg "latestVersion\?\.license|displayVersion\?\.license" app/pages/package/[[org]]/[name].vueRepository: npmx-dev/npmx.dev
Length of output: 298
Type error: license property does not exist on SlimVersion.
The code accesses latestVersion.value?.license (both in the computed property at line 331 and in the template), but latestVersion is typed as SlimVersion | null. SlimVersion explicitly picks only 'version', 'deprecated', and 'tags' from SlimPackumentVersion to reduce payload size, so accessing .license violates type safety and will cause a TypeScript compilation failure.
The normalize function can also be extracted outside the computed since it has no dependency on the parent scope.
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 327-327:
Property 'license' does not exist on type 'SlimVersion'.
🪛 GitHub Check: 🤖 Autofix code
[warning] 330-330: eslint-plugin-unicorn(consistent-function-scoping)
Function normalize does not capture any variables from its parent scope
| <dd class="font-mono text-sm text-fg flex items-center gap-2 flex-wrap"> | ||
| <LicenseDisplay v-if="displayVersion?.license ?? pkg.license" :license="displayVersion?.license ?? pkg.license" /> | ||
| <span v-else>{{ $t('package.license.none') }}</span> | ||
| <TooltipApp | ||
| v-if="licenseChanged" | ||
| :text="$t('package.license.changed', { latest: latestVersion?.license ?? pkg.license })" | ||
| position="bottom" | ||
| > | ||
| <span class="inline-flex items-center gap-1 px-1.5 py-0.5 text-2xs font-sans rounded bg-amber-500/15 text-amber-700 dark:text-amber-400 border border-amber-500/30 cursor-help"> | ||
| <span class="i-lucide:triangle-alert w-3 h-3" aria-hidden="true" /> | ||
| {{ $t('package.license.changed_badge') }} | ||
| </span> | ||
| </TooltipApp> | ||
| </dd> |
There was a problem hiding this comment.
Type mismatch on :license prop.
The LicenseDisplay component's :license prop receives displayVersion?.license ?? pkg.license, but:
displayVersionisSlimVersion | nullwhich may not havelicense- The expression evaluates to
string | undefined, but the prop likely expectsstring
This aligns with the type errors flagged at lines 327 and 617.
🔧 Proposed fix with type narrowing
<dd class="font-mono text-sm text-fg flex items-center gap-2 flex-wrap">
- <LicenseDisplay v-if="displayVersion?.license ?? pkg.license" :license="displayVersion?.license ?? pkg.license" />
+ <LicenseDisplay
+ v-if="(displayVersion as PackumentVersion | null)?.license ?? pkg.license"
+ :license="((displayVersion as PackumentVersion | null)?.license ?? pkg.license) as string"
+ />
<span v-else>{{ $t('package.license.none') }}</span>
<TooltipApp
v-if="licenseChanged"
- :text="$t('package.license.changed', { latest: latestVersion?.license ?? pkg.license })"
+ :text="$t('package.license.changed', { latest: (latestVersion as PackumentVersion | null)?.license ?? pkg.license })"
position="bottom"
>Alternatively, consider adding license to the SlimVersion type if it's always present in practice.
📝 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.
| <dd class="font-mono text-sm text-fg flex items-center gap-2 flex-wrap"> | |
| <LicenseDisplay v-if="displayVersion?.license ?? pkg.license" :license="displayVersion?.license ?? pkg.license" /> | |
| <span v-else>{{ $t('package.license.none') }}</span> | |
| <TooltipApp | |
| v-if="licenseChanged" | |
| :text="$t('package.license.changed', { latest: latestVersion?.license ?? pkg.license })" | |
| position="bottom" | |
| > | |
| <span class="inline-flex items-center gap-1 px-1.5 py-0.5 text-2xs font-sans rounded bg-amber-500/15 text-amber-700 dark:text-amber-400 border border-amber-500/30 cursor-help"> | |
| <span class="i-lucide:triangle-alert w-3 h-3" aria-hidden="true" /> | |
| {{ $t('package.license.changed_badge') }} | |
| </span> | |
| </TooltipApp> | |
| </dd> | |
| <dd class="font-mono text-sm text-fg flex items-center gap-2 flex-wrap"> | |
| <LicenseDisplay | |
| v-if="(displayVersion as PackumentVersion | null)?.license ?? pkg.license" | |
| :license="((displayVersion as PackumentVersion | null)?.license ?? pkg.license) as string" | |
| /> | |
| <span v-else>{{ $t('package.license.none') }}</span> | |
| <TooltipApp | |
| v-if="licenseChanged" | |
| :text="$t('package.license.changed', { latest: (latestVersion as PackumentVersion | null)?.license ?? pkg.license })" | |
| position="bottom" | |
| > | |
| <span class="inline-flex items-center gap-1 px-1.5 py-0.5 text-2xs font-sans rounded bg-amber-500/15 text-amber-700 dark:text-amber-400 border border-amber-500/30 cursor-help"> | |
| <span class="i-lucide:triangle-alert w-3 h-3" aria-hidden="true" /> | |
| {{ $t('package.license.changed_badge') }} | |
| </span> | |
| </TooltipApp> | |
| </dd> |
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 617-617:
Property 'license' does not exist on type 'SlimVersion'.
[failure] 613-613:
Type 'string | undefined' is not assignable to type 'string'.
| <PackageHeader | ||
| :pkg="pkg ?? null" | ||
| :resolved-version="resolvedVersion" | ||
| :display-version="displayVersion" | ||
| :latest-version="latestVersion" | ||
| :version-url-pattern="versionUrlPattern" | ||
| page="dependents" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check displayVersion computation in main package page
rg -n "displayVersion.*computed" app/pages/package/Repository: npmx-dev/npmx.dev
Length of output: 267
🏁 Script executed:
# First, view the displayVersion computation in dependents.vue (lines 20-30)
head -n 30 app/pages/package/[[org]]/[name]/dependents.vue | tail -n 15Repository: npmx-dev/npmx.dev
Length of output: 513
🏁 Script executed:
# Find and examine PackageHeader component definition
fd -name "PackageHeader*" -t fRepository: npmx-dev/npmx.dev
Length of output: 291
🏁 Script executed:
# Check the type definitions for SlimVersion and PackumentVersion
rg -n "type (SlimVersion|PackumentVersion)" --type ts --type tsx --type vueRepository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# View the PackageHeader prop definition to see what type is expected
fd -name "PackageHeader*" -t f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -n 100 {}'Repository: npmx-dev/npmx.dev
Length of output: 291
🏁 Script executed:
# Find PackageHeader component
find . -name "*PackageHeader*" -type fRepository: npmx-dev/npmx.dev
Length of output: 104
🏁 Script executed:
# Search for PackageHeader component definition in common locations
rg "export.*PackageHeader" --type vueRepository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Find where PackageHeader is defined by searching for its definition
rg "defineProps.*display-version" --type vueRepository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Check the types in the pkg object
rg "type.*Package" --type ts | head -20Repository: npmx-dev/npmx.dev
Length of output: 2011
🏁 Script executed:
# Search for PackageHeader component more broadly
rg "PackageHeader" --max-count 5Repository: npmx-dev/npmx.dev
Length of output: 1036
🏁 Script executed:
# Find the component file
find . -type f -name "*.vue" | xargs rg "PackageHeader" | head -10Repository: npmx-dev/npmx.dev
Length of output: 785
🏁 Script executed:
# Look for PackageHeader in components
rg "export.*default" | grep -i "packageheader"Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
# Search in components directory
find ./app/components -type f | xargs rg "PackageHeader" 2>/dev/null | head -5Repository: npmx-dev/npmx.dev
Length of output: 239
🏁 Script executed:
# View the PackageHeader component (Package/Header.vue)
cat app/components/Package/Header.vueRepository: npmx-dev/npmx.dev
Length of output: 12283
🏁 Script executed:
# Also check what types are imported in dependents.vue
head -n 50 app/pages/package/[[org]]/[name]/dependents.vueRepository: npmx-dev/npmx.dev
Length of output: 1228
🏁 Script executed:
# Find type definitions for SlimVersion, PackumentVersion, and SlimPackument
rg "export type (SlimVersion|PackumentVersion|SlimPackument)" --max-count 10Repository: npmx-dev/npmx.dev
Length of output: 266
🏁 Script executed:
# Search for type definitions without 'export'
rg "type SlimVersion|type PackumentVersion|type SlimPackument" --max-count 10Repository: npmx-dev/npmx.dev
Length of output: 266
🏁 Script executed:
# Look for interface definitions
rg "interface (SlimVersion|PackumentVersion|SlimPackument)" --max-count 10Repository: npmx-dev/npmx.dev
Length of output: 231
🏁 Script executed:
# Check what the pkg type has for versions property
rg "versions.*:" | grep -i "slim\|packument" | head -10Repository: npmx-dev/npmx.dev
Length of output: 830
🏁 Script executed:
# View the full type definitions for SlimPackument and the versions property
sed -n '/export interface SlimPackument/,/^}/p' shared/types/npm-registry.tsRepository: npmx-dev/npmx.dev
Length of output: 1472
🏁 Script executed:
# View the full Packument type definition
sed -n '/export interface Packument/,/^}/p' shared/types/npm-registry.tsRepository: npmx-dev/npmx.dev
Length of output: 312
🏁 Script executed:
# Also check SlimPackumentVersion vs PackumentVersion
sed -n '/export type SlimPackumentVersion/,/^}/p' shared/types/npm-registry.tsRepository: npmx-dev/npmx.dev
Length of output: 156
🏁 Script executed:
# Verify SlimVersion structure to confirm it's a subset
rg -A 5 "export type SlimVersion" shared/types/npm-registry.tsRepository: npmx-dev/npmx.dev
Length of output: 218
🏁 Script executed:
# Check if there are any existing usages of SlimVersion with PackageHeader elsewhere
rg "displayVersion.*SlimVersion" --max-count 5Repository: npmx-dev/npmx.dev
Length of output: 43
Type mismatch: displayVersion is SlimVersion but PackageHeader expects PackumentVersion.
In dependents.vue, the computation pkg.value.versions[resolvedVersion.value] ?? null returns SlimVersion, which is only a subset of SlimPackumentVersion. Since SlimPackumentVersion extends PackumentVersion, but SlimVersion is just a pick of certain properties (version, deprecated, tags, plus optional hasProvenance and trustLevel), it cannot satisfy the :display-version prop type.
The main package page avoids this by using pkg.value?.requestedVersion ?? null, which returns SlimPackumentVersion | null and is assignable to PackumentVersion. Consider using the same approach for consistency.
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 93-93:
Type 'SlimVersion | null' is not assignable to type 'PackumentVersion | null | undefined'.
| @@ -0,0 +1,87 @@ | |||
| import { CACHE_MAX_AGE_FIVE_MINUTES, NPM_REGISTRY } from '#shared/utils/constants' | |||
There was a problem hiding this comment.
Remove unused import NPM_REGISTRY.
Static analysis correctly identifies that NPM_REGISTRY is imported but never used. The endpoint uses NPM_SEARCH_BASE instead.
🔧 Proposed fix
-import { CACHE_MAX_AGE_FIVE_MINUTES, NPM_REGISTRY } from '#shared/utils/constants'
+import { CACHE_MAX_AGE_FIVE_MINUTES } from '#shared/utils/constants'📝 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.
| import { CACHE_MAX_AGE_FIVE_MINUTES, NPM_REGISTRY } from '#shared/utils/constants' | |
| import { CACHE_MAX_AGE_FIVE_MINUTES } from '#shared/utils/constants' |
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 1-1:
'NPM_REGISTRY' is declared but its value is never read.
🪛 GitHub Check: 🤖 Autofix code
[failure] 1-1: eslint(no-unused-vars)
Identifier 'NPM_REGISTRY' is imported but never used.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
graphieros
left a comment
graphieros
left a comment
There was a problem hiding this comment.
Thank you for your PR!
Please consider proposing one PR per issue.
Also, new features should be covered by tests.
We recommend you check out our contribution guide.
thealxlabs
changed the title
graphieros
left a comment
graphieros
left a comment
There was a problem hiding this comment.
As stated previously, this PR requires to be split per issue to be considered.
Kindly close this PR and propose one PR per issue, this will help maintainers to review your contributions. 🙏
2 participants
🔗 Linked issue
🧭 Context
Fixed and improved a bunch of long-standing UI issues found while going through the issue tracker — layout bugs, broken links, mobile fixes, and a new dependents page.
19 issues closed across two sessions of work.
📚 Description
Went through the open issue list and tackled everything that was clearly broken or had an obvious fix. Here's what changed:
Layout & visual bugs
z-1from the nav element that was causing the Connect dropdown to clip behind the package sub-header ("Connect" dropdown menu is hidden behind a div (on browser) #2191)grid-template-rowsmismatch — 5 row definitions for 4 template areas (Huge empty space around "Get started" block on some packages #2190)<button>elements instead of anchor links, so clicking toggles the section instead of jumping the page (Package page scrolls down when clicking side bar summary titles #1518, Clicking on sidebar headings should expand/collapse their section #1392)Package page
"foo": "npm:bar@^1.0.0"now link to the real package (bar) instead of the alias name (foo) — fixed across all three dep sections (Aliased dependencies are not linked correctly #2010, Incorrect links for aliased links in code view #2044)/package/<name>/dependents, backed by the npm search API (sort dependents by download count #31)Code viewer
Search
hiddentoinvisible(Layout moves too much when typing in the search bar #2112)Clipboard & mobile
ClipboardItemwith a Promise — Safari requires the clipboard write to happen synchronously in the gesture handler ("Copy as Markdown" button on README doesn't copy to clipboard in Safari #2151)cursor-pointertoButtonBase(Use pointer cursor for interactive elements #1760)Playgrounds / README
/pen/,/full/,/details/) (Codepen link in Readme ends up in Playgrounds #2110)Other