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

Remove multi-host support for image component and support quality pass-through #18038

Merged
merged 4 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 5 additions & 17 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,25 +229,13 @@ export default async function getBaseWebpackConfig(
}
}

if (config.images?.hosts) {
if (!config.images.hosts.default) {
// If the image component is being used, a default host must be provided
throw new Error(
'If the image configuration property is present in next.config.js, it must have a host named "default"'
)
// Normalize defined image host to end in slash
if (config.images?.path) {
if (config.images.path[config.images.path.length - 1] !== '/') {
config.images.path += '/'
}
Object.values(config.images.hosts).forEach((host: any) => {
if (!host.path) {
throw new Error(
'All hosts defined in the image configuration property of next.config.js must define a path'
)
}
// Normalize hosts so all paths have trailing slash
if (host.path[host.path.length - 1] !== '/') {
host.path += '/'
}
})
}

const reactVersion = await getPackageVersion({ cwd: dir, name: 'react' })
const hasReactRefresh: boolean = dev && !isServer
const hasJsxRuntime: boolean =
Expand Down
129 changes: 65 additions & 64 deletions packages/next/client/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,25 @@ const loaders: { [key: string]: (props: LoaderProps) => string } = {

type ImageData = {
sizes?: number[]
hosts: {
[key: string]: {
path: string
loader: string
}
}
}

type ImageProps = Omit<
JSX.IntrinsicElements['img'],
'src' | 'srcSet' | 'ref'
> & {
src: string
host?: string
quality?: string
priority?: boolean
lazy?: boolean
unoptimized?: boolean
}

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

let cachedObserver: IntersectionObserver
const IntersectionObserver =
Expand Down Expand Up @@ -66,60 +64,56 @@ function getObserver(): IntersectionObserver | undefined {
))
}

function computeSrc(src: string, host: string, unoptimized: boolean): string {
function computeSrc(
src: string,
unoptimized: boolean,
quality?: string
): string {
if (unoptimized) {
return src
}
if (!host) {
// No host provided, use default
return callLoader(src, 'default')
} else {
let selectedHost = imageData.hosts[host]
if (!selectedHost) {
if (process.env.NODE_ENV !== 'production') {
console.error(
`Image tag is used specifying host ${host}, but that host is not defined in next.config`
)
}
return src
}
return callLoader(src, host)
}
return callLoader({ src, quality })
}

function callLoader(src: string, host: string, width?: number): string {
let loader = loaders[imageData.hosts[host].loader || 'default']
return loader({ root: imageData.hosts[host].path, src, width })
type CallLoaderProps = {
src: string
width?: number
quality?: string
}

function callLoader(loaderProps: CallLoaderProps) {
let loader = loaders[imageData.loader || 'default']
return loader({ root: imageData.path, ...loaderProps })
}

type SrcSetData = {
src: string
host: string
widths: number[]
quality?: string
}

function generateSrcSet({ src, host, widths }: SrcSetData): string {
function generateSrcSet({ src, widths, quality }: SrcSetData): string {
// 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, host, width)} ${width}w`)
.map((width: number) => `${callLoader({ src, width, quality })} ${width}w`)
.join(', ')
}

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

function generatePreload({
src,
host,
widths,
unoptimized = false,
sizes,
quality,
}: PreloadData): ReactElement {
// This function generates an image preload that makes use of the "imagesrcset" and "imagesizes"
// attributes for preloading responsive images. They're still experimental, but fully backward
Expand All @@ -130,9 +124,9 @@ function generatePreload({
<link
rel="preload"
as="image"
href={computeSrc(src, host, unoptimized)}
href={computeSrc(src, unoptimized, quality)}
// @ts-ignore: imagesrcset and imagesizes not yet in the link element type
imagesrcset={generateSrcSet({ src, host, widths })}
imagesrcset={generateSrcSet({ src, widths, quality })}
imagesizes={sizes}
/>
</Head>
Expand All @@ -141,30 +135,17 @@ function generatePreload({

export default function Image({
src,
host,
sizes,
unoptimized = false,
priority = false,
lazy = false,
className,
quality,
...rest
}: ImageProps) {
const thisEl = useRef<HTMLImageElement>(null)

// Sanity Checks:
if (process.env.NODE_ENV !== 'production') {
if (unoptimized && host) {
console.error(`Image tag used specifying both a host and the unoptimized attribute--these are mutually exclusive.
With the unoptimized attribute, no host will be used, so specify an absolute URL.`)
}
}
if (host && !imageData.hosts[host]) {
// If unregistered host is selected, log an error and use the default instead
if (process.env.NODE_ENV !== 'production') {
console.error(`Image host identifier ${host} could not be resolved.`)
}
host = 'default'
}
// If priority and lazy are present, log an error and use priority only.
if (priority && lazy) {
if (process.env.NODE_ENV !== 'production') {
Expand All @@ -175,8 +156,6 @@ export default function Image({
lazy = false
}

host = host || 'default'

// Normalize provided src
if (src[0] === '/') {
src = src.slice(1)
Expand All @@ -199,12 +178,12 @@ export default function Image({
}, [thisEl, lazy])

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

Expand Down Expand Up @@ -243,7 +222,6 @@ export default function Image({
{shouldPreload
? generatePreload({
src,
host,
widths: breakpoints,
unoptimized,
sizes,
Expand All @@ -262,23 +240,46 @@ export default function Image({

//BUILT IN LOADERS

type LoaderProps = {
root: string
src: string
width?: number
}
type LoaderProps = CallLoaderProps & { root: string }

function imgixLoader({ root, src, width }: LoaderProps): string {
return `${root}${src}${width ? '?w=' + width : ''}`
function imgixLoader({ root, src, width, quality }: LoaderProps): string {
const params = []
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}${src}${paramsString}`
}

function cloudinaryLoader({ root, src, width }: LoaderProps): string {
return `${root}${width ? 'w_' + width + '/' : ''}${src}`
function cloudinaryLoader({ root, src, width, quality }: LoaderProps): string {
const params = []
let paramsString = ''
if (!quality && imageData.autoOptimize) {
quality = 'auto'
}
if (width) {
params.push('w_' + width)
}
if (quality) {
params.push('q_' + quality)
}
if (params.length) {
paramsString = params.join(',') + '/'
}
return `${root}${paramsString}${src}`
}

function defaultLoader({ root, src, width }: LoaderProps): string {
// TODO: change quality parameter to be configurable
function defaultLoader({ root, src, width, quality }: LoaderProps): string {
return `${root}?url=${encodeURIComponent(src)}&${
width ? `w=${width}&` : ''
}q=100`
}q=${quality || '100'}`
}
13 changes: 3 additions & 10 deletions test/integration/image-component/basic/next.config.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
module.exports = {
images: {
sizes: [480, 1024, 1600],
hosts: {
default: {
path: 'https://example.com/myaccount/',
loader: 'imgix',
},
secondary: {
path: 'https://examplesecondary.com/images/',
loader: 'cloudinary',
},
},
autoOptimize: false,
path: 'https://example.com/myaccount/',
loader: 'imgix',
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const ClientSide = () => {
return (
<div>
<p id="stubtext">This is a client side page</p>
<Image id="basic-image" src="foo.jpg"></Image>
<Image id="basic-image" src="foo.jpg" quality="60"></Image>
<Image id="attribute-test" data-demo="demo-value" src="bar.jpg" />
<Image
id="secondary-image"
Expand Down
2 changes: 1 addition & 1 deletion test/integration/image-component/basic/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const Page = () => {
return (
<div>
<p>Hello World</p>
<Image id="basic-image" src="foo.jpg"></Image>
<Image id="basic-image" src="foo.jpg" quality="60"></Image>
<Image id="attribute-test" data-demo="demo-value" src="bar.jpg" />
<Image
id="secondary-image"
Expand Down
18 changes: 4 additions & 14 deletions test/integration/image-component/basic/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,19 @@ 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'
'https://example.com/myaccount/foo.jpg?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')
})
it('should support manually selecting a different host', async () => {
expect(
await browser.elementById('secondary-image').getAttribute('src')
).toBe('https://examplesecondary.com/images/foo2.jpg')
})
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 480w, https://example.com/myaccount/foo.jpg?w=1024 1024w, https://example.com/myaccount/foo.jpg?w=1600 1600w'
'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'
)
})
it('should add a srcset even with preceding slash in prop', async () => {
Expand Down Expand Up @@ -102,6 +97,7 @@ function lazyLoadingTests() {
await browser.eval(
`window.scrollTo(0, ${topOfMidImage - (viewportHeight + buffer)})`
)
await waitFor(200)
expect(await browser.elementById('lazy-mid').getAttribute('src')).toBe(
'https://example.com/myaccount/foo2.jpg'
)
Expand All @@ -126,6 +122,7 @@ function lazyLoadingTests() {
await browser.eval(
`window.scrollTo(0, ${topOfBottomImage - (viewportHeight + buffer)})`
)
await waitFor(200)
expect(await browser.elementById('lazy-bottom').getAttribute('src')).toBe(
'https://www.otherhost.com/foo3.jpg'
)
Expand Down Expand Up @@ -178,13 +175,6 @@ describe('Image Component Tests', () => {
)
).toBe(true)
})
it('should add a preload tag for a priority image, with secondary host', async () => {
expect(
await hasPreloadLinkMatchingUrl(
'https://examplesecondary.com/images/withpriority2.png'
)
).toBe(true)
})
it('should add a preload tag for a priority image, with arbitrary host', async () => {
expect(
await hasPreloadLinkMatchingUrl(
Expand Down