Skip to content

Commit

Permalink
Fix Image component defaults & remove autoOptimize (#18101)
Browse files Browse the repository at this point in the history
  • Loading branch information
styfle committed Oct 21, 2020
1 parent 1a8cb7e commit 3f07e55
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 41 deletions.
1 change: 0 additions & 1 deletion packages/next/build/webpack-config.ts
Expand Up @@ -994,7 +994,6 @@ export default async function getBaseWebpackConfig(
sizes: config.images.sizes,
path: config.images.path,
loader: config.images.loader,
autoOptimize: config.images.autoOptimize,
}),
'process.env.__NEXT_ROUTER_BASEPATH': JSON.stringify(config.basePath),
'process.env.__NEXT_HAS_REWRITES': JSON.stringify(hasRewrites),
Expand Down
46 changes: 19 additions & 27 deletions packages/next/client/image.tsx
@@ -1,18 +1,19 @@
import React, { ReactElement, useEffect, useRef } from 'react'
import Head from '../next-server/lib/head'

const loaders: { [key: string]: (props: LoaderProps) => string } = {
imgix: imgixLoader,
cloudinary: cloudinaryLoader,
akamai: akamaiLoader,
default: defaultLoader,
}
const loaders = new Map<LoaderKey, (props: LoaderProps) => string>([
['imgix', imgixLoader],
['cloudinary', cloudinaryLoader],
['akamai', akamaiLoader]
['default', defaultLoader],
])

type LoaderKey = 'imgix' | 'cloudinary' | 'akamai' | 'default'

type ImageData = {
sizes?: number[]
loader?: string
path?: string
autoOptimize?: boolean
sizes: number[]
loader: LoaderKey
path: string
}

type ImageProps = Omit<
Expand All @@ -30,11 +31,7 @@ type ImageProps = Omit<
)

const imageData: ImageData = process.env.__NEXT_IMAGE_OPTS as any
const breakpoints = imageData.sizes || [640, 1024, 1600]
// Auto optimize defaults to on if not specified
if (imageData.autoOptimize === undefined) {
imageData.autoOptimize = true
}
const { sizes: configSizes, loader: configLoader, path: configPath } = imageData

let cachedObserver: IntersectionObserver
const IntersectionObserver =
Expand Down Expand Up @@ -89,8 +86,8 @@ type CallLoaderProps = {
}

function callLoader(loaderProps: CallLoaderProps) {
let loader = loaders[imageData.loader || 'default']
return loader({ root: imageData.path || '/_next/image', ...loaderProps })
let load = loaders.get(configLoader) || defaultLoader
return load({ root: configPath, ...loaderProps })
}

type SrcSetData = {
Expand Down Expand Up @@ -187,7 +184,7 @@ export default function Image({
const imgSrcSet = !unoptimized
? generateSrcSet({
src,
widths: breakpoints,
widths: configSizes,
quality,
})
: undefined
Expand Down Expand Up @@ -266,7 +263,7 @@ export default function Image({
{shouldPreload
? generatePreload({
src,
widths: breakpoints,
widths: configSizes,
unoptimized,
sizes,
})
Expand All @@ -292,17 +289,15 @@ function normalizeSrc(src: string) {
}

function imgixLoader({ root, src, width, quality }: LoaderProps): string {
const params = []
const params = ['auto=format']
let paramsString = ''
if (width) {
params.push('w=' + width)
}
if (quality) {
params.push('q=' + quality)
}
if (imageData.autoOptimize) {
params.push('auto=compress')
}

if (params.length) {
paramsString = '?' + params.join('&')
}
Expand All @@ -314,11 +309,8 @@ function akamaiLoader({ root, src, width }: LoaderProps): string {
}

function cloudinaryLoader({ root, src, width, quality }: LoaderProps): string {
const params = []
const params = ['f_auto']
let paramsString = ''
if (!quality && imageData.autoOptimize) {
quality = 'auto'
}
if (width) {
params.push('w_' + width)
}
Expand Down
3 changes: 2 additions & 1 deletion packages/next/next-server/server/config.ts
Expand Up @@ -26,7 +26,8 @@ const defaultConfig: { [key: string]: any } = {
images: {
sizes: [320, 420, 768, 1024, 1200],
domains: [],
hosts: { default: { path: '/_next/image' } },
path: '/_next/image',
loader: 'default',
},
devIndicators: {
buildActivity: true,
Expand Down
1 change: 0 additions & 1 deletion test/integration/image-component/basic/next.config.js
@@ -1,7 +1,6 @@
module.exports = {
images: {
sizes: [480, 1024, 1600],
autoOptimize: false,
path: 'https://example.com/myaccount/',
loader: 'imgix',
},
Expand Down
22 changes: 11 additions & 11 deletions test/integration/image-component/basic/test/index.test.js
Expand Up @@ -33,26 +33,26 @@ function runTests() {
})
it('should modify src with the loader', async () => {
expect(await browser.elementById('basic-image').getAttribute('src')).toBe(
'https://example.com/myaccount/foo.jpg?q=60'
'https://example.com/myaccount/foo.jpg?auto=format&q=60'
)
})
it('should correctly generate src even if preceding slash is included in prop', async () => {
expect(
await browser.elementById('preceding-slash-image').getAttribute('src')
).toBe('https://example.com/myaccount/fooslash.jpg')
).toBe('https://example.com/myaccount/fooslash.jpg?auto=format')
})
it('should add a srcset based on the loader', async () => {
expect(
await browser.elementById('basic-image').getAttribute('srcset')
).toBe(
'https://example.com/myaccount/foo.jpg?w=480&q=60 480w, https://example.com/myaccount/foo.jpg?w=1024&q=60 1024w, https://example.com/myaccount/foo.jpg?w=1600&q=60 1600w'
'https://example.com/myaccount/foo.jpg?auto=format&w=480&q=60 480w, https://example.com/myaccount/foo.jpg?auto=format&w=1024&q=60 1024w, https://example.com/myaccount/foo.jpg?auto=format&w=1600&q=60 1600w'
)
})
it('should add a srcset even with preceding slash in prop', async () => {
expect(
await browser.elementById('preceding-slash-image').getAttribute('srcset')
).toBe(
'https://example.com/myaccount/fooslash.jpg?w=480 480w, https://example.com/myaccount/fooslash.jpg?w=1024 1024w, https://example.com/myaccount/fooslash.jpg?w=1600 1600w'
'https://example.com/myaccount/fooslash.jpg?auto=format&w=480 480w, https://example.com/myaccount/fooslash.jpg?auto=format&w=1024 1024w, https://example.com/myaccount/fooslash.jpg?auto=format&w=1600 1600w'
)
})
it('should support the unoptimized attribute', async () => {
Expand All @@ -70,10 +70,10 @@ function runTests() {
function lazyLoadingTests() {
it('should have loaded the first image immediately', async () => {
expect(await browser.elementById('lazy-top').getAttribute('src')).toBe(
'https://example.com/myaccount/foo1.jpg'
'https://example.com/myaccount/foo1.jpg?auto=format'
)
expect(await browser.elementById('lazy-top').getAttribute('srcset')).toBe(
'https://example.com/myaccount/foo1.jpg?w=480 480w, https://example.com/myaccount/foo1.jpg?w=1024 1024w, https://example.com/myaccount/foo1.jpg?w=1600 1600w'
'https://example.com/myaccount/foo1.jpg?auto=format&w=480 480w, https://example.com/myaccount/foo1.jpg?auto=format&w=1024 1024w, https://example.com/myaccount/foo1.jpg?auto=format&w=1600 1600w'
)
})
it('should not have loaded the second image immediately', async () => {
Expand Down Expand Up @@ -101,11 +101,11 @@ function lazyLoadingTests() {

await check(() => {
return browser.elementById('lazy-mid').getAttribute('src')
}, 'https://example.com/myaccount/foo2.jpg')
}, 'https://example.com/myaccount/foo2.jpg?auto=format')

await check(() => {
return browser.elementById('lazy-mid').getAttribute('srcset')
}, 'https://example.com/myaccount/foo2.jpg?w=480 480w, https://example.com/myaccount/foo2.jpg?w=1024 1024w, https://example.com/myaccount/foo2.jpg?w=1600 1600w')
}, 'https://example.com/myaccount/foo2.jpg?auto=format&w=480 480w, https://example.com/myaccount/foo2.jpg?auto=format&w=1024 1024w, https://example.com/myaccount/foo2.jpg?auto=format&w=1600 1600w')
})
it('should not have loaded the third image after scrolling down', async () => {
expect(
Expand Down Expand Up @@ -166,14 +166,14 @@ describe('Image Component Tests', () => {
it('should add a preload tag for a priority image', async () => {
expect(
await hasPreloadLinkMatchingUrl(
'https://example.com/myaccount/withpriority.png'
'https://example.com/myaccount/withpriority.png?auto=format'
)
).toBe(true)
})
it('should add a preload tag for a priority image with preceding slash', async () => {
expect(
await hasPreloadLinkMatchingUrl(
'https://example.com/myaccount/fooslash.jpg'
'https://example.com/myaccount/fooslash.jpg?auto=format'
)
).toBe(true)
})
Expand All @@ -197,7 +197,7 @@ describe('Image Component Tests', () => {
it('should NOT add a preload tag for a priority image', async () => {
expect(
await hasPreloadLinkMatchingUrl(
'https://example.com/myaccount/withpriorityclient.png'
'https://example.com/myaccount/withpriorityclient.png?auto=format'
)
).toBe(false)
})
Expand Down

0 comments on commit 3f07e55

Please sign in to comment.