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 Image srcset to ensure the component's width is the largest possible image #18236

Merged
merged 3 commits into from Oct 26, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
154 changes: 98 additions & 56 deletions packages/next/client/image.tsx
Expand Up @@ -42,7 +42,6 @@ const {
domains: configDomains,
} = imageData
configSizes.sort((a, b) => a - b) // smallest to largest
const largestSize = configSizes[configSizes.length - 1]

let cachedObserver: IntersectionObserver
const IntersectionObserver =
Expand Down Expand Up @@ -80,15 +79,32 @@ function getObserver(): IntersectionObserver | undefined {
))
}

function getWidthsFromConfig(width: number | undefined) {
if (typeof width !== 'number') {
return configSizes
}
const widths: number[] = []
for (let size of configSizes) {
widths.push(size)
if (size >= width) {
break
}
}
return widths
}

function computeSrc(
src: string,
unoptimized: boolean,
width: number | undefined,
quality?: string
): string {
if (unoptimized) {
return src
}
return callLoader({ src, width: largestSize, quality })
const widths = getWidthsFromConfig(width)
const largest = widths[widths.length - 1]
return callLoader({ src, width: largest, quality })
}

type CallLoaderProps = {
Expand All @@ -104,29 +120,38 @@ function callLoader(loaderProps: CallLoaderProps) {

type SrcSetData = {
src: string
widths: number[]
quality?: string
unoptimized: boolean
width: number | undefined
quality: string | undefined
}

function generateSrcSet({ src, widths, quality }: SrcSetData): string {
function generateSrcSet({
src,
unoptimized,
width,
quality,
}: SrcSetData): string | undefined {
// At each breakpoint, generate an image url using the loader, such as:
// ' www.example.com/foo.jpg?w=480 480w, '
return widths
.map((width: number) => `${callLoader({ src, width, quality })} ${width}w`)
if (unoptimized) {
return undefined
}
return getWidthsFromConfig(width)
.map((w) => `${callLoader({ src, width: w, quality })} ${w}w`)
.join(', ')
}

type PreloadData = {
src: string
widths: number[]
unoptimized: boolean
width: number | undefined
sizes?: string
unoptimized?: boolean
quality?: string
}

function generatePreload({
src,
widths,
width,
unoptimized = false,
sizes,
quality,
Expand All @@ -140,15 +165,25 @@ function generatePreload({
<link
rel="preload"
as="image"
href={computeSrc(src, unoptimized, quality)}
href={computeSrc(src, unoptimized, width, quality)}
// @ts-ignore: imagesrcset and imagesizes not yet in the link element type
imagesrcset={generateSrcSet({ src, widths, quality })}
imagesrcset={generateSrcSet({ src, unoptimized, width, quality })}
imagesizes={sizes}
/>
</Head>
)
}

function getInt(x: unknown): number | undefined {
if (typeof x === 'number') {
return x
}
if (typeof x === 'string') {
return parseInt(x, 10)
}
return undefined
}

export default function Image({
src,
sizes,
Expand All @@ -165,6 +200,13 @@ export default function Image({
const thisEl = useRef<HTMLImageElement>(null)

if (process.env.NODE_ENV !== 'production') {
if (!src) {
throw new Error(
`Image is missing required "src" property. Make sure you pass "src" in props to the \`next/image\` component. Received: ${JSON.stringify(
{ width, height, quality, unsized }
)}`
)
}
if (!VALID_LOADING_VALUES.includes(loading)) {
throw new Error(
`Image with src "${src}" has invalid "loading" property. Provided "${loading}" should be one of ${VALID_LOADING_VALUES.map(
Expand Down Expand Up @@ -200,58 +242,23 @@ export default function Image({
}
}, [thisEl, lazy])

// Generate attribute values
const imgSrc = computeSrc(src, unoptimized, quality)
const imgSrcSet = !unoptimized
? generateSrcSet({
src,
widths: configSizes,
quality,
})
: undefined

let imgAttributes:
| {
src: string
srcSet?: string
}
| {
'data-src': string
'data-srcset'?: string
}
if (!lazy) {
imgAttributes = {
src: imgSrc,
}
if (imgSrcSet) {
imgAttributes.srcSet = imgSrcSet
}
} else {
imgAttributes = {
'data-src': imgSrc,
}
if (imgSrcSet) {
imgAttributes['data-srcset'] = imgSrcSet
}
className = className ? className + ' __lazy' : '__lazy'
}

let widthInt = getInt(width)
let heightInt = getInt(height)
let divStyle: React.CSSProperties | undefined
let imgStyle: React.CSSProperties | undefined
let wrapperStyle: React.CSSProperties | undefined
if (
typeof height !== 'undefined' &&
typeof width !== 'undefined' &&
typeof widthInt !== 'undefined' &&
typeof heightInt !== 'undefined' &&
!unsized
) {
// <Image src="i.png" width={100} height={100} />
// <Image src="i.png" width="100" height="100" />
const quotient =
parseInt(height as string, 10) / parseInt(width as string, 10)
const quotient = heightInt / widthInt
const ratio = isNaN(quotient) ? 1 : quotient * 100
wrapperStyle = {
maxWidth: '100%',
width,
width: widthInt,
}
divStyle = {
position: 'relative',
Expand All @@ -266,8 +273,8 @@ export default function Image({
width: '100%',
}
} else if (
typeof height === 'undefined' &&
typeof width === 'undefined' &&
typeof widthInt === 'undefined' &&
typeof heightInt === 'undefined' &&
unsized
) {
// <Image src="i.png" unsized />
Expand All @@ -288,6 +295,41 @@ export default function Image({
}
}

// Generate attribute values
const imgSrc = computeSrc(src, unoptimized, widthInt, quality)
const imgSrcSet = generateSrcSet({
src,
width: widthInt,
unoptimized,
quality,
})

let imgAttributes:
| {
src: string
srcSet?: string
}
| {
'data-src': string
'data-srcset'?: string
}
if (!lazy) {
imgAttributes = {
src: imgSrc,
}
if (imgSrcSet) {
imgAttributes.srcSet = imgSrcSet
}
} else {
imgAttributes = {
'data-src': imgSrc,
}
if (imgSrcSet) {
imgAttributes['data-srcset'] = imgSrcSet
}
className = className ? className + ' __lazy' : '__lazy'
}

// No need to add preloads on the client side--by the time the application is hydrated,
// it's too late for preloads
const shouldPreload = priority && typeof window === 'undefined'
Expand All @@ -298,7 +340,7 @@ export default function Image({
{shouldPreload
? generatePreload({
src,
widths: configSizes,
width: widthInt,
unoptimized,
sizes,
quality,
Expand Down
2 changes: 1 addition & 1 deletion test/integration/image-component/basic/next.config.js
@@ -1,6 +1,6 @@
module.exports = {
images: {
sizes: [480, 1024, 1600],
sizes: [480, 1024, 1600, 2000],
path: 'https://example.com/myaccount/',
loader: 'imgix',
},
Expand Down
3 changes: 1 addition & 2 deletions test/integration/image-component/basic/pages/index.js
Expand Up @@ -19,7 +19,7 @@ const Page = () => {
data-demo="demo-value"
src="bar.jpg"
loading="eager"
width={300}
width={1024}
height={400}
/>
<Image
Expand All @@ -39,7 +39,6 @@ const Page = () => {
width={300}
height={400}
/>
<Image id="priority-image" priority src="withpriority.png" />
<Image
id="priority-image"
priority
Expand Down
6 changes: 3 additions & 3 deletions test/integration/image-component/basic/pages/lazy.js
Expand Up @@ -9,7 +9,7 @@ const Lazy = () => {
id="lazy-top"
src="foo1.jpg"
height={400}
width={300}
width={1024}
loading="lazy"
></Image>
<div style={{ height: '2000px' }}></div>
Expand All @@ -35,15 +35,15 @@ const Lazy = () => {
id="lazy-without-attribute"
src="foo4.jpg"
height={400}
width={300}
width={800}
></Image>
<div style={{ height: '2000px' }}></div>
<Image
id="eager-loading"
src="foo5.jpg"
loading="eager"
height={400}
width={300}
width={1900}
></Image>
</div>
)
Expand Down