Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: nuxtpicture placeholder #1396

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
12 changes: 12 additions & 0 deletions playground/pages/picture.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<template>
<div>
<h1>Original</h1>
<nuxt-picture
src="/images/colors.jpg"
format="avif,webp"
Expand All @@ -8,11 +9,22 @@
@load="isLoaded = true"
/>
<p>Received onLoad event: {{ isLoaded }}</p>
<h1>Placeholder</h1>
<nuxt-picture
src="/images/colors.jpg"
placeholder
format="avif,webp"
width="500"
height="500"
@load="isLoaded2 = true"
/>
<p>Received onLoad event: {{ isLoaded2 }}</p>
</div>
</template>

<script setup lang="ts">
import { ref } from '#imports'

const isLoaded = ref(false)
const isLoaded2 = ref(false)
</script>
47 changes: 9 additions & 38 deletions src/runtime/components/NuxtImg.vue
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<template>
<img
ref="imgEl"
:class="props.placeholder && !placeholderLoaded ? props.placeholderClass : undefined"
v-bind="{
...isServer ? { onerror: 'this.setAttribute(\'data-error\', 1)' } : {},
..._attrs,
...attrs,
...(placeholderClass && placeholder) ? { class: placeholderClass } : {},
}"
:src="src"
>
Expand Down Expand Up @@ -34,30 +34,29 @@ const isServer = import.meta.server

const $img = useImage()

const _base = useBaseImage(props)
const { placeholder, placeholderLoaded, options: baseOptions, modifiers: baseModifiers, attrs: baseAttrs } = useBaseImage(props)

const placeholderLoaded = ref(false)
const imgEl = ref<HTMLImageElement>()

type AttrsT = typeof _base.attrs.value & {
type AttrsT = typeof baseAttrs.value & {
'sizes'?: string
'srcset'?: string
'data-nuxt-img'?: string
}

const sizes = computed(() => $img.getSizes(props.src!, {
..._base.options.value,
...baseOptions.value,
sizes: props.sizes,
densities: props.densities,
modifiers: {
..._base.modifiers.value,
...baseModifiers.value,
width: parseSize(props.width),
height: parseSize(props.height),
},
}))

const _attrs = computed(() => {
const attrs: AttrsT = { ..._base.attrs.value, 'data-nuxt-img': '' }
const attrs: AttrsT = { ...baseAttrs.value, 'data-nuxt-img': '' }

if (!props.placeholder || placeholderLoaded.value) {
attrs.sizes = sizes.value.sizes
Expand All @@ -67,38 +66,10 @@ const _attrs = computed(() => {
return attrs
})

const placeholder = computed(() => {
let placeholder = props.placeholder

if (placeholder === '') {
placeholder = true
}

if (!placeholder || placeholderLoaded.value) {
return false
}

if (typeof placeholder === 'string') {
return placeholder
}

const size = (Array.isArray(placeholder)
? placeholder
: (typeof placeholder === 'number' ? [placeholder, placeholder] : [10, 10])) as [w: number, h: number, q: number, b: number]

return $img(props.src!, {
..._base.modifiers.value,
width: size[0],
height: size[1],
quality: size[2] || 50,
blur: size[3] || 3,
}, _base.options.value)
})

const mainSrc = computed(() =>
props.sizes
? sizes.value.src
: $img(props.src!, _base.modifiers.value, _base.options.value),
: $img(props.src!, baseModifiers.value, baseOptions.value),
)

const src = computed(() => placeholder.value ? placeholder.value : mainSrc.value)
Expand Down Expand Up @@ -138,13 +109,13 @@ onMounted(() => {
if (placeholder.value) {
const img = new Image()

img.src = mainSrc.value

if (props.sizes) {
img.sizes = sizes.value.sizes || ''
img.srcset = sizes.value.srcset
}

img.src = mainSrc.value

img.onload = (event) => {
placeholderLoaded.value = true
emit('load', event)
Expand Down
29 changes: 23 additions & 6 deletions src/runtime/components/NuxtPicture.vue
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<template>
<picture>
<picture v-bind="(placeholderClass && placeholder) ? { class: placeholderClass } : {}">
<source
v-for="source in sources.slice(0, -1)"
:key="source.src"
:type="source.type"
:type="placeholder ? 'display/never' : source.type"
:sizes="source.sizes"
:srcset="source.srcset"
>
Expand All @@ -15,9 +15,9 @@
...(isServer ? { onerror: 'this.setAttribute(\'data-error\', 1)' } : {}),
...imgAttrs,
}"
:src="sources[lastSourceIndex].src"
:sizes="sources[lastSourceIndex].sizes"
:srcset="sources[lastSourceIndex].srcset"
:src="placeholder ? placeholder : sources[lastSourceIndex]?.src"
:sizes="placeholder ? undefined : sources[lastSourceIndex]?.sizes"
:srcset="placeholder ? undefined : sources[lastSourceIndex]?.srcset"
>
</picture>
</template>
Expand Down Expand Up @@ -46,7 +46,7 @@ const isServer = import.meta.server

const $img = useImage()

const { attrs: baseAttrs, options: baseOptions, modifiers: baseModifiers } = useBaseImage(props)
const { placeholder, placeholderLoaded, attrs: baseAttrs, options: baseOptions, modifiers: baseModifiers } = useBaseImage(props)

const originalFormat = computed(() => getFileExtension(props.src))

Expand Down Expand Up @@ -90,6 +90,7 @@ const sources = computed<Source[]>(() => {
})

const lastSourceIndex = computed(() => sources.value.length - 1)
const mainSrc = computed(() => sources.value[lastSourceIndex.value])

if (import.meta.server && props.preload) {
const link: NonNullable<Head['link']>[number] = {
Expand Down Expand Up @@ -131,6 +132,22 @@ const nuxtApp = useNuxtApp()
const initialLoad = nuxtApp.isHydrating

onMounted(() => {
if (placeholder.value) {
const img = new Image()

if (mainSrc.value.sizes) img.sizes = mainSrc.value.sizes
if (mainSrc.value.srcset) img.srcset = mainSrc.value.srcset
if (mainSrc.value.src) img.src = mainSrc.value.src
Comment on lines +138 to +140
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit worried that we end up loading more images than necessary. So, mainSrc will load, but then if there is another src that better suits browser needs (e.g. avif/webp), this will swap in, and we will have downloaded twice as much image data as we need.

do you have any ideas for resolving that? maybe using a picture element as placeholder instead?


img.onload = (event) => {
placeholderLoaded.value = true
emit('load', event)
}

markFeatureUsage('nuxt-picture')
return
}

if (!imgEl.value) {
return
}
Expand Down
42 changes: 37 additions & 5 deletions src/runtime/components/_base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ export const baseImageProps = {

// csp
nonce: { type: [String], default: undefined },

// placeholders
placeholder: { type: [Boolean, String, Number, Array], default: undefined },
placeholderClass: { type: String, default: undefined },
}

export interface BaseImageAttrs {
Expand Down Expand Up @@ -117,7 +121,39 @@ export const useBaseImage = (props: ExtractPropTypes<typeof baseImageProps>) =>
}
})

const placeholderLoaded = ref(false)

const placeholder = computed(() => {
let placeholder = props.placeholder

if (placeholder === '') {
placeholder = true
}

if (!placeholder || placeholderLoaded.value) {
return false
}

if (typeof placeholder === 'string') {
return placeholder
}

const size = (Array.isArray(placeholder)
? placeholder
: (typeof placeholder === 'number' ? [placeholder, placeholder] : [10, 10])) as [w: number, h: number, q: number, b: number]

return $img(props.src!, {
...modifiers.value,
width: size[0],
height: size[1],
quality: size[2] || 50,
blur: size[3] || 3,
}, options.value)
})

return {
placeholder,
placeholderLoaded,
options,
attrs,
modifiers,
Expand All @@ -130,8 +166,4 @@ export const pictureProps = {
imgAttrs: { type: Object, default: null },
}

export const imgProps = {
...baseImageProps,
placeholder: { type: [Boolean, String, Number, Array], default: undefined },
placeholderClass: { type: String, default: undefined },
}
export const imgProps = baseImageProps
27 changes: 27 additions & 0 deletions test/unit/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { vi } from 'vitest'

export const getImageLoad = (cb = () => {}) => {
let resolve = () => {}
let image = {} as HTMLImageElement
const loadEvent = Symbol('loadEvent')
const ImageMock = vi.fn(() => {
const _image = {
onload: () => {},
} as unknown as HTMLImageElement
image = _image
// @ts-expect-error not valid argument for onload
resolve = () => _image.onload?.(loadEvent)

return _image
})

vi.stubGlobal('Image', ImageMock)
cb()
vi.unstubAllGlobals()

return {
resolve,
image,
loadEvent,
}
}
29 changes: 2 additions & 27 deletions test/unit/image.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// @vitest-environment nuxt

import { beforeEach, describe, it, expect, vi } from 'vitest'
import { beforeEach, describe, it, expect } from 'vitest'
import { mount } from '@vue/test-utils'
import type { ComponentMountingOptions, VueWrapper } from '@vue/test-utils'
import { getImageLoad } from './helpers'
// @ts-expect-error virtual file
import { imageOptions } from '#build/image-options'
import { NuxtImg } from '#components'
Expand Down Expand Up @@ -158,32 +159,6 @@ describe('Renders simple image', () => {
})
})

const getImageLoad = (cb = () => {}) => {
let resolve = () => {}
let image = {} as HTMLImageElement
const loadEvent = Symbol('loadEvent')
const ImageMock = vi.fn(() => {
const _image = {
onload: () => {},
} as unknown as HTMLImageElement
image = _image
// @ts-expect-error not valid argument for onload
resolve = () => _image.onload?.(loadEvent)

return _image
})

vi.stubGlobal('Image', ImageMock)
cb()
vi.unstubAllGlobals()

return {
resolve,
image,
loadEvent,
}
}

describe('Renders placeholder image', () => {
let wrapper: VueWrapper<any>
const src = '/image.png'
Expand Down
Loading