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 component defaults & remove autoOptimize #18101

Merged
merged 6 commits into from Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
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
44 changes: 18 additions & 26 deletions packages/next/client/image.tsx
@@ -1,17 +1,18 @@
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,
default: defaultLoader,
}
const loaders = new Map<LoaderKey, (props: LoaderProps) => string>([
['imgix', imgixLoader],
['cloudinary', cloudinaryLoader],
['default', defaultLoader],
])

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

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

type ImageProps = Omit<
Expand All @@ -29,11 +30,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 @@ -88,8 +85,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 @@ -186,7 +183,7 @@ export default function Image({
const imgSrcSet = !unoptimized
? generateSrcSet({
src,
widths: breakpoints,
widths: configSizes,
quality,
})
: undefined
Expand Down Expand Up @@ -265,7 +262,7 @@ export default function Image({
{shouldPreload
? generatePreload({
src,
widths: breakpoints,
widths: configSizes,
unoptimized,
sizes,
})
Expand All @@ -291,29 +288,24 @@ 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('&')
}
return `${root}${normalizeSrc(src)}${paramsString}`
}

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