Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/composables/npm/useResolvedVersion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ export function useResolvedVersion(
const data = await $fetch<ResolvedPackageVersion>(url)
return data.version
},
{ default: () => null },
{ default: () => undefined },
)
}
89 changes: 84 additions & 5 deletions app/pages/package/[[org]]/[name].vue
Original file line number Diff line number Diff line change
Expand Up @@ -210,21 +210,79 @@ const { data: skillsData } = useLazyFetch<SkillsListResponse>(
const { data: packageAnalysis } = usePackageAnalysis(packageName, requestedVersion)
const { data: moduleReplacement } = useModuleReplacement(packageName)

const { data: resolvedVersion } = await useResolvedVersion(packageName, requestedVersion)
const { data: resolvedVersion, status: resolvedStatus } = await useResolvedVersion(
packageName,
requestedVersion,
)

if (resolvedVersion.value === null) {
if (
import.meta.server &&
!resolvedVersion.value &&
['success', 'error'].includes(resolvedStatus.value)
) {
throw createError({
statusCode: 404,
statusMessage: $t('package.not_found'),
message: $t('package.not_found_message'),
})
}

watch(
[resolvedStatus, resolvedVersion],
([status, version]) => {
if ((!version && status === 'success') || status === 'error') {
showError({
statusCode: 404,
statusMessage: $t('package.not_found'),
message: $t('package.not_found_message'),
})
}
},
{ immediate: true },
)
Comment on lines +230 to +242
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

watch with immediate: true may call showError during SSR.

On the server, the throw createError guard at lines 218–228 only fires when resolvedStatus is already 'success' or 'error'. If resolvedStatus is still 'pending' at that point, the guard is skipped and the watch is registered. With immediate: true, it fires synchronously during setup — on the server context as well as the client. If, by the time the immediate callback runs, resolvedStatus is already 'error' (e.g., if useResolvedVersion resolves synchronously to an error on the server), showError is called from the server context instead of throw createError, which produces a different error-response shape (soft error page vs. HTTP 404).

Consider guarding the watch so it only runs on the client:

🛡️ Proposed fix
-watch(
-  [resolvedStatus, resolvedVersion],
-  ([status, version]) => {
-    if ((!version && status === 'success') || status === 'error') {
-      showError({
-        statusCode: 404,
-        statusMessage: $t('package.not_found'),
-        message: $t('package.not_found_message'),
-      })
-    }
-  },
-  { immediate: true },
-)
+if (import.meta.client) {
+  watch(
+    [resolvedStatus, resolvedVersion],
+    ([status, version]) => {
+      if ((!version && status === 'success') || status === 'error') {
+        showError({
+          statusCode: 404,
+          statusMessage: $t('package.not_found'),
+          message: $t('package.not_found_message'),
+        })
+      }
+    },
+    { immediate: true },
+  )
+}
📝 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
watch(
[resolvedStatus, resolvedVersion],
([status, version]) => {
if ((!version && status === 'success') || status === 'error') {
showError({
statusCode: 404,
statusMessage: $t('package.not_found'),
message: $t('package.not_found_message'),
})
}
},
{ immediate: true },
)
if (import.meta.client) {
watch(
[resolvedStatus, resolvedVersion],
([status, version]) => {
if ((!version && status === 'success') || status === 'error') {
showError({
statusCode: 404,
statusMessage: $t('package.not_found'),
message: $t('package.not_found_message'),
})
}
},
{ immediate: true },
)
}


const {
data: pkg,
status,
error,
} = usePackage(packageName, () => resolvedVersion.value ?? requestedVersion.value)

// Detect two hydration scenarios where the external _payload.json is missing:
//
// 1. SPA fallback (200.html): No real content was server-rendered.
// → Show skeleton while data fetches on the client.
//
// 2. SSR-rendered HTML with missing payload: Content was rendered but the external _payload.json
// returned an ISR fallback.
// → Preserve the server-rendered DOM, don't flash to skeleton.
const nuxtApp = useNuxtApp()
const route = useRoute()
const hasEmptyPayload =
import.meta.client &&
nuxtApp.isHydrating &&
nuxtApp.payload.serverRendered &&
!Object.keys(nuxtApp.payload.data ?? {}).length
const isSpaFallback = shallowRef(hasEmptyPayload && !nuxtApp.payload.path)
const isHydratingWithServerContent = shallowRef(
hasEmptyPayload && nuxtApp.payload.path === route.path,
)
// When we have server-rendered content but no payload data, capture the server
// DOM before Vue's hydration replaces it. This lets us show the server-rendered
// HTML as a static snapshot while data refetches, avoiding any visual flash.
const serverRenderedHtml = shallowRef<string | null>(
isHydratingWithServerContent.value
? (document.getElementById('package-article')?.innerHTML ?? null)
: null,
)
Comment on lines +265 to +276
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add a hard fallback when snapshot capture is unavailable.

Line 737 suppresses the skeleton while isHydratingWithServerContent is true, but Line 746 also requires serverRenderedHtml. If Line 274 cannot find #package-article, the page can render a blank main area until async data resolves.

💡 Suggested fix
 const serverRenderedHtml = shallowRef<string | null>(
   isHydratingWithServerContent.value
     ? (document.getElementById('package-article')?.innerHTML ?? null)
     : null,
 )
+
+if (isHydratingWithServerContent.value && !serverRenderedHtml.value) {
+  isHydratingWithServerContent.value = false
+  isSpaFallback.value = true
+}

Also applies to: 736-750


if (isSpaFallback.value || isHydratingWithServerContent.value) {
nuxtApp.hooks.hookOnce('app:suspense:resolve', () => {
isSpaFallback.value = false
isHydratingWithServerContent.value = false
serverRenderedHtml.value = null
})
}

const displayVersion = computed(() => pkg.value?.requestedVersion ?? null)
const versionSecurityMetadata = computed<PackageVersionInfo[]>(() => {
if (!pkg.value) return []
Expand Down Expand Up @@ -672,9 +730,30 @@ const showSkeleton = shallowRef(false)
</ButtonBase>
</DevOnly>
<main class="container flex-1 w-full py-8">
<PackageSkeleton v-if="showSkeleton || status === 'pending'" />

<article v-else-if="status === 'success' && pkg" :class="$style.packagePage">
<!-- Scenario 1: SPA fallback — show skeleton (no real content to preserve) -->
<!-- Scenario 2: SSR with missing payload — preserve server DOM, skip skeleton -->
<PackageSkeleton
v-if="
isSpaFallback || (!isHydratingWithServerContent && (showSkeleton || status === 'pending'))
"
/>

<!-- During hydration without payload, show captured server HTML as a static snapshot.
This avoids a visual flash: the user sees the server content while data refetches.
v-html is safe here: the content originates from the server's own SSR output,
captured from the DOM before hydration — it is not user-controlled input. -->
<article
v-else-if="isHydratingWithServerContent && serverRenderedHtml"
id="package-article"
:class="$style.packagePage"
v-html="serverRenderedHtml"
/>
Comment on lines 745 to 750
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

v-html binding missing the eslint-disable comment used elsewhere in the file.

Line 1374 uses <!-- eslint-disable vue/no-v-html -- HTML is sanitized server-side --> before the v-html binding on the README, establishing a project convention for documenting why raw HTML is trusted. The new v-html on the hydration snapshot article at line 746 doesn't follow this pattern. Even though the source is our own server-rendered markup, the missing annotation leaves the intent undocumented and will produce a lint warning.

💅 Proposed fix
+    <!-- eslint-disable-next-line vue/no-v-html -- snapshot of server-rendered HTML, not user-supplied content -->
     <article
       v-else-if="isHydratingWithServerContent && serverRenderedHtml"
       :class="$style.packagePage"
       v-html="serverRenderedHtml"
     />


<article
v-else-if="status === 'success' && pkg"
id="package-article"
:class="$style.packagePage"
>
<!-- Package header -->
<header
class="sticky top-14 z-1 bg-[--bg] py-2 border-border"
Expand Down
14 changes: 14 additions & 0 deletions app/plugins/fix.client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,19 @@ export default defineNuxtPlugin({
setup(nuxtApp) {
// TODO: investigate why this is needed
nuxtApp.payload.data ||= {}

// When a _payload.json returns an ISR fallback (empty payload), data fetching composables
// with non-undefined defaults skip refetching during hydration. After hydration completes,
// refresh all async data so these composables (e.g. README, skills, package analysis) fetch
// fresh data.
if (
nuxtApp.isHydrating &&
nuxtApp.payload.serverRendered &&
!Object.keys(nuxtApp.payload.data).length
) {
nuxtApp.hooks.hookOnce('app:suspense:resolve', () => {
refreshNuxtData()
})
}
},
})
63 changes: 63 additions & 0 deletions app/plugins/payload-cache.server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { stringify } from 'devalue'

/**
* Nuxt server plugin that serializes the payload after SSR rendering
* and stashes it on the request event context.
*
* This allows the Nitro payload-cache plugin to cache the payload
* when rendering HTML pages, so that subsequent _payload.json requests
* for the same route can be served from cache without a full re-render.
*
* This mirrors what Nuxt does during pre-rendering (via `payloadCache`),
* but extends it to runtime for ISR-enabled routes.
*/
export default defineNuxtPlugin({
name: 'payload-cache',
setup(nuxtApp) {
// Only run on the server during SSR
if (import.meta.client) return

nuxtApp.hooks.hook('app:rendered', () => {
const ssrContext = nuxtApp.ssrContext
if (!ssrContext) return

// Don't cache error responses or noSSR renders
if (ssrContext.noSSR || ssrContext.error || ssrContext.payload?.error) return

// Don't cache if payload data is empty
const payloadData = ssrContext.payload?.data
if (!payloadData || Object.keys(payloadData).length === 0) return

Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid caching placeholder payloads that are effectively empty.

Line 29 only checks top-level key count. Payload shapes like placeholder entries (for example, fallback-style empty records wrapped in arrays) can still pass and get cached as valid data, which risks stale empty payload reuse and hydration inconsistencies. Please harden this emptiness check before caching.

Suggested patch
       // Don't cache if payload data is empty
       const payloadData = ssrContext.payload?.data
-      if (!payloadData || Object.keys(payloadData).length === 0) return
+      const isPlaceholderValue = (value: unknown) =>
+        Array.isArray(value)
+        && value.length === 1
+        && typeof value[0] === 'object'
+        && value[0] !== null
+        && Object.keys(value[0] as Record<string, unknown>).length === 0
+
+      const isEmptyPayload =
+        !payloadData
+        || Object.keys(payloadData).length === 0
+        || Object.values(payloadData).every((value) => value == null || isPlaceholderValue(value))
+
+      if (isEmptyPayload) return

try {
// Serialize the payload using devalue (same as Nuxt's renderPayloadResponse)
// splitPayload extracts only { data, prerenderedAt } for the external payload
const payload = {
data: ssrContext.payload.data,
prerenderedAt: ssrContext.payload.prerenderedAt,
}
const reducers = ssrContext['~payloadReducers'] ?? {}
const body = stringify(payload, reducers)

// Stash the serialized payload on the event context
// The Nitro payload-cache plugin will pick this up in render:response
const event = ssrContext.event
if (event) {
event.context._cachedPayloadResponse = {
body,
statusCode: 200,
headers: {
'content-type': 'application/json;charset=utf-8',
'x-powered-by': 'Nuxt',
},
}
}
} catch (error) {
// Serialization failed — don't cache, but don't break the render
if (import.meta.dev) {
// eslint-disable-next-line no-console
console.warn('[payload-cache] Failed to serialize payload:', error)
}
}
})
},
})
9 changes: 9 additions & 0 deletions modules/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import { provider } from 'std-env'
// Storage key for fetch cache - must match shared/utils/fetch-cache-config.ts
const FETCH_CACHE_STORAGE_BASE = 'fetch-cache'

// Storage key for payload cache - must match server/plugins/payload-cache.ts
const PAYLOAD_CACHE_STORAGE_KEY = 'payload-cache'

export default defineNuxtModule({
meta: {
name: 'vercel-cache',
Expand Down Expand Up @@ -37,6 +40,12 @@ export default defineNuxtModule({
...nitroConfig.storage[FETCH_CACHE_STORAGE_BASE],
driver: 'vercel-runtime-cache',
}

// Payload cache storage (for runtime payload caching)
nitroConfig.storage[PAYLOAD_CACHE_STORAGE_KEY] = {
...nitroConfig.storage[PAYLOAD_CACHE_STORAGE_KEY],
driver: 'vercel-runtime-cache',
}
}

const env = process.env.VERCEL_ENV
Expand Down
2 changes: 1 addition & 1 deletion modules/isr-fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default defineNuxtModule({
const outputPath = resolve(nitro.options.output.serverDir, '..', path, htmlFallback)
mkdirSync(resolve(nitro.options.output.serverDir, '..', path), { recursive: true })
writeFileSync(outputPath, spaTemplate)
writeFileSync(outputPath.replace(htmlFallback, jsonFallback), '{}')
writeFileSync(outputPath.replace(htmlFallback, jsonFallback), '[{}]')
}
})
})
Expand Down
4 changes: 4 additions & 0 deletions nuxt.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,10 @@ export default defineNuxtConfig({
driver: 'fsLite',
base: './.cache/fetch',
},
'payload-cache': {
driver: 'fsLite',
base: './.cache/payload',
},
'atproto': {
driver: 'fsLite',
base: './.cache/atproto',
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
"@vue/test-utils": "2.4.6",
"axe-core": "4.11.1",
"defu": "6.1.4",
"devalue": "5.6.3",
"eslint-plugin-regexp": "3.0.0",
"fast-check": "4.5.3",
"h3": "1.15.5",
Expand Down
15 changes: 9 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading