fix: handle falsy src in NuxtImg and NuxtPicture#2218
Conversation
When these three props are combined, the preload link in `<head>` and the image load in `onMounted` can incorrectly resolve to `current-url/undefined`.
This causes `NuxtImg` to compute invalid values, for example:
{
"sizes": "32px",
"src": undefined,
"srcset": "undefined 32w, undefined 64w"
}
commit: |
📝 WalkthroughWalkthroughThe changes short-circuit sizing logic when image input is falsy, avoid non-null assertions by casting props.src where needed, prevent placeholder resolution when props.src is missing, guard server-side preload on props.src, and delay assigning img.sizes/srcset until a mainSrc exists. Two unit tests were added to verify components render safely when src is undefined. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/runtime/components/NuxtImg.vue (2)
106-129:⚠️ Potential issue | 🟡 MinorSame
srcguard likely needed for prerender block.The new
&& props.srccheck correctly avoids emitting a preload link with anundefinedhref. However, the prerender block immediately below (lines 127-129) still callsprerenderStaticImages(src.value, sizes.value.srcset, …)without an analogous guard. Withprops.srcundefined,src.valueandsizes.value.srcsetcan resolve to".../undefined"and"undefined 32w, …"respectively, so the prerenderer may attempt to fetch and persist garbage URLs at build time.🛡️ Suggested guard
// Prerender static images -if (import.meta.server && import.meta.prerender) { +if (import.meta.server && import.meta.prerender && props.src) { prerenderStaticImages(src.value, sizes.value.srcset, useRequestEvent()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/NuxtImg.vue` around lines 106 - 129, The prerender block calls prerenderStaticImages(src.value, sizes.value.srcset, useRequestEvent()) even when props.src is undefined, which can produce garbage URLs; wrap that call with the same server+src guard used above — e.g., ensure the condition includes import.meta.server, import.meta.prerender and props.src (or truthiness of props.src) before invoking prerenderStaticImages so src.value and sizes.value.srcset are not used when props.src is missing.
98-147:⚠️ Potential issue | 🟠 MajorGuard at the source:
mainSrccan still resolve to".../undefined"whenprops.srcis unset.The
if (props.sizes)wrap prevents bogussizes/srcsetassignments, but the underlyingimg.src = mainSrc.valueon line 141 can still be set to a bad URL. Whenprops.srcis undefined butprops.sizesis set,mainSrcfalls through tosizes.value.src, which is computed via$img.getSizes(props.src!, …)— the non-null assertion lies, and the helper typically returns a truthy resolved path containing the string"undefined". That makesif (mainSrc.value)truthy and you assign an invalid URL to a realImage(), triggering a network request and anerroremit.Fixing this at the source removes the need for ad‑hoc per-call-site guards (preload, prerender, onMounted) and keeps
src.value,imgAttrs.srcset, etc. consistent:♻️ Suggested centralised fix
-const mainSrc = computed(() => - props.sizes - ? sizes.value.src - : $img(props.src!, imageModifiers.value, providerOptions.value), -) +const mainSrc = computed(() => { + if (!props.src) return undefined + return props.sizes + ? sizes.value.src + : $img(props.src, imageModifiers.value, providerOptions.value) +})With this in place, the existing
if (mainSrc.value)check on line 140 already short‑circuits theimg.srcassignment, and the innerif (props.sizes)wrap becomes redundant (but harmless). Consider also propagating the guard into theplaceholdercomputed (line 89) which usesprops.src!similarly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/NuxtImg.vue` around lines 98 - 147, mainSrc computed can resolve to a bogus ".../undefined" because it calls $img with props.src! or uses sizes.value.src which itself was derived from $img.getSizes(props.src!), so add a central guard: change the mainSrc computation (the mainSrc computed) to first return null/empty if props.src is falsy (and likewise ensure sizes-based path only used when props.src is present), and update the placeholder computed to honor the same props.src guard; this ensures src.value, preload/prerender usages (href and prerenderStaticImages) and the onMounted Image() assignment only ever receive a valid URL and removes the need for per-call-site guards.
🧹 Nitpick comments (1)
src/runtime/components/NuxtImg.vue (1)
140-147: Test coverage for the bug fix.The PR description notes you couldn't get a test working with
@nuxt/test-utils. A reasonable test would render<NuxtImg :src="undefined" custom sizes="sm:100vw md:50vw" />inside a Nuxt test app and assert that:
- no
<link rel="preload" as="image">is emitted in SSR head,- on client mount no
Imageis constructed withsrcending in/undefined,- no spurious
errorevent is emitted.Want me to draft a
test/unit(ortest/e2e) spec using@nuxt/test-utils'ssetup()+$fetch/createPagecovering these three assertions? I can also open a tracking issue if you'd prefer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/NuxtImg.vue` around lines 140 - 147, Add a test that renders the NuxtImg component (NuxtImg.vue) with :src="undefined" and attributes custom sizes="sm:100vw md:50vw" to reproduce the bug: in SSR verify the rendered head does not include a <link rel="preload" as="image"> for an undefined src (check server head output), in the client verify that no Image is constructed with img.src ending in "/undefined" (monitor global Image constructor or inspect DOM after mount where mainSrc.value would be applied to img.src), and assert that no spurious "error" event is emitted from the component; target the behavior around mainSrc.value, img.src, props.sizes and sizes.value to ensure the fix prevents creating preload links or setting src to "undefined" on mount.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/runtime/components/NuxtImg.vue`:
- Around line 106-129: The prerender block calls
prerenderStaticImages(src.value, sizes.value.srcset, useRequestEvent()) even
when props.src is undefined, which can produce garbage URLs; wrap that call with
the same server+src guard used above — e.g., ensure the condition includes
import.meta.server, import.meta.prerender and props.src (or truthiness of
props.src) before invoking prerenderStaticImages so src.value and
sizes.value.srcset are not used when props.src is missing.
- Around line 98-147: mainSrc computed can resolve to a bogus ".../undefined"
because it calls $img with props.src! or uses sizes.value.src which itself was
derived from $img.getSizes(props.src!), so add a central guard: change the
mainSrc computation (the mainSrc computed) to first return null/empty if
props.src is falsy (and likewise ensure sizes-based path only used when
props.src is present), and update the placeholder computed to honor the same
props.src guard; this ensures src.value, preload/prerender usages (href and
prerenderStaticImages) and the onMounted Image() assignment only ever receive a
valid URL and removes the need for per-call-site guards.
---
Nitpick comments:
In `@src/runtime/components/NuxtImg.vue`:
- Around line 140-147: Add a test that renders the NuxtImg component
(NuxtImg.vue) with :src="undefined" and attributes custom sizes="sm:100vw
md:50vw" to reproduce the bug: in SSR verify the rendered head does not include
a <link rel="preload" as="image"> for an undefined src (check server head
output), in the client verify that no Image is constructed with img.src ending
in "/undefined" (monitor global Image constructor or inspect DOM after mount
where mainSrc.value would be applied to img.src), and assert that no spurious
"error" event is emitted from the component; target the behavior around
mainSrc.value, img.src, props.sizes and sizes.value to ensure the fix prevents
creating preload links or setting src to "undefined" on mount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d96122e-bd3d-4655-bdfd-141c2977032d
📒 Files selected for processing (1)
src/runtime/components/NuxtImg.vue
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2218 +/- ##
==========================================
+ Coverage 32.52% 33.15% +0.62%
==========================================
Files 7 7
Lines 372 374 +2
Branches 131 132 +1
==========================================
+ Hits 121 124 +3
+ Misses 194 192 -2
- Partials 57 58 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Prevent `undefined` from being coerced into strings within `srcset` and `sizes` - Fix `getSizes` in `image.ts` to return empty values for falsy inputs - Remove unsafe non-null assertions in `NuxtImg` and `NuxtPicture` - Add parity test cases for both components to verify `src: undefined` handling
Bombastickj
changed the title
src in NuxtImg and NuxtPicture
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/runtime/components/NuxtPicture.vue (1)
111-111: ⚡ Quick winConsider a safer type handling approach.
While
props.src as stringworks at runtime (sincegetSizesnow handles falsy input), the type assertion is misleading whenprops.srcis actuallyundefined. Consider one of these alternatives:
- Guard the call:
props.src ? $img.getSizes(props.src, ...) : { srcset: '', sizes: undefined, src: undefined }- Update
getSizessignature to acceptstring | undefinedinstead of juststringThe current implementation works but obscures the intent and could confuse future maintainers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/components/NuxtPicture.vue` at line 111, The call to $img.getSizes currently forces props.src to string with a type assertion which hides that props.src can be undefined; update the code to handle the undefined case explicitly by either guarding the call in NuxtPicture.vue (e.g., call $img.getSizes only when props.src is truthy and provide a fallback object otherwise) or change the getSizes function signature to accept string | undefined so the type reflects reality; locate the call to $img.getSizes(props.src as string, ...) and modify it to use a runtime guard or update the getSizes declaration to accept undefined and adjust its implementation accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/runtime/components/NuxtPicture.vue`:
- Line 111: The call to $img.getSizes currently forces props.src to string with
a type assertion which hides that props.src can be undefined; update the code to
handle the undefined case explicitly by either guarding the call in
NuxtPicture.vue (e.g., call $img.getSizes only when props.src is truthy and
provide a fallback object otherwise) or change the getSizes function signature
to accept string | undefined so the type reflects reality; locate the call to
$img.getSizes(props.src as string, ...) and modify it to use a runtime guard or
update the getSizes declaration to accept undefined and adjust its
implementation accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3af4cc39-a148-4d44-a690-d58419e67b84
📒 Files selected for processing (5)
src/runtime/components/NuxtImg.vuesrc/runtime/components/NuxtPicture.vuesrc/runtime/image.tstest/nuxt/image.test.tstest/nuxt/picture.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/runtime/components/NuxtImg.vue
|
Just pushed a few more updates after a self-review, and I've added comprehensive test cases to ensure the new logic behaves as expected. Let me know if any further changes are needed once someone has the bandwidth to review! |
2 participants
📚 Description
When
src: undefinedcombined withcustom: true&sizes, the preload link in<head>and the image load inonMountedcan incorrectly resolve tocurrent-url/undefined.This causes
NuxtImgto compute invalid values for thesizescomputed, for example:Which are then used within the component regardless of incorrect values.
Final edit:
undefinedfrom being coerced into strings withinsrcsetandsizesgetSizesinimage.tsto return empty values for falsy inputsNuxtImgandNuxtPicturesrc: undefinedhandling