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

Add experimental blurry placeholder to image component #24153

Merged
merged 14 commits into from
Apr 30, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,7 @@ export default async function getBaseWebpackConfig(
domains: config.images.domains,
}
: {}),
enableBlurryPlaceholder: config.experimental.enableBlurryPlaceholder,
}),
'process.env.__NEXT_ROUTER_BASEPATH': JSON.stringify(config.basePath),
'process.env.__NEXT_HAS_REWRITES': JSON.stringify(hasRewrites),
Expand Down
54 changes: 53 additions & 1 deletion packages/next/client/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ const VALID_LAYOUT_VALUES = [
] as const
type LayoutValue = typeof VALID_LAYOUT_VALUES[number]

type PlaceholderValue = 'blur' | 'empty'
Copy link
Member

Choose a reason for hiding this comment

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

@timneutkens @Timer Do you think we should use empty or none? Do we have any precedence for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think none would be a good name since it may imply that we don't placeholder the image in any way, causing layout shift. empty was used in an effort to promote the understanding that there will at least be empty space.

Copy link
Member

Choose a reason for hiding this comment

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

Another option here is blank. I agree that none might imply that we don't have a placeholder here.


type ImgElementStyle = NonNullable<JSX.IntrinsicElements['img']['style']>

export type ImageProps = Omit<
Expand Down Expand Up @@ -72,6 +74,13 @@ export type ImageProps = Omit<
height: number | string
layout?: Exclude<LayoutValue, 'fill'>
}
) &
(
| {
placeholder?: Exclude<PlaceholderValue, 'blur'>
blurDataURL?: never
}
| { placeholder: 'blur'; blurDataURL: string }
)

const {
Expand All @@ -80,6 +89,7 @@ const {
loader: configLoader,
path: configPath,
domains: configDomains,
enableBlurryPlaceholder: configEnableBlurryPlaceholder,
} =
((process.env.__NEXT_IMAGE_OPTS as any) as ImageConfig) || imageConfigDefault
// sort smallest to largest
Expand Down Expand Up @@ -211,6 +221,26 @@ function defaultImageLoader(loaderProps: ImageLoaderProps) {
)
}

// See https://stackoverflow.com/q/39777833/266535 for why we use this ref
// handler instead of the img's onLoad attribute.
function removePlaceholder(
element: HTMLImageElement | null,
placeholder: PlaceholderValue
) {
if (placeholder === 'blur' && element) {
if (element.complete) {
Joonpark13 marked this conversation as resolved.
Show resolved Hide resolved
// If the real image fails to load, this will still remove the placeholder.
// This is the desired behavior for now, and will be revisited when error
// handling is worked on for the image component itself.
element.style.backgroundImage = 'none'
} else {
element.onload = () => {
element.style.backgroundImage = 'none'
}
}
}
}

export default function Image({
src,
sizes,
Expand All @@ -224,6 +254,8 @@ export default function Image({
objectFit,
objectPosition,
loader = defaultImageLoader,
placeholder = 'empty',
blurDataURL,
...all
}: ImageProps) {
let rest: Partial<ImageProps> = all
Expand All @@ -241,6 +273,10 @@ export default function Image({
delete rest['layout']
}

if (!configEnableBlurryPlaceholder) {
placeholder = 'empty'
}

if (process.env.NODE_ENV !== 'production') {
if (!src) {
throw new Error(
Expand Down Expand Up @@ -293,6 +329,12 @@ export default function Image({
const heightInt = getInt(height)
const qualityInt = getInt(quality)

const MIN_IMG_SIZE_FOR_PLACEHOLDER = 5000
const tooSmallForBlurryPlaceholder =
widthInt && heightInt && widthInt * heightInt < MIN_IMG_SIZE_FOR_PLACEHOLDER
const shouldShowBlurryPlaceholder =
placeholder === 'blur' && !tooSmallForBlurryPlaceholder

let wrapperStyle: JSX.IntrinsicElements['div']['style'] | undefined
let sizerStyle: JSX.IntrinsicElements['div']['style'] | undefined
let sizerSvg: string | undefined
Expand All @@ -318,6 +360,13 @@ export default function Image({

objectFit,
objectPosition,

...(shouldShowBlurryPlaceholder
? {
backgroundSize: 'cover',
backgroundImage: `url("${blurDataURL}")`,
}
: undefined),
}
if (
typeof widthInt !== 'undefined' &&
Expand Down Expand Up @@ -464,7 +513,10 @@ export default function Image({
{...imgAttributes}
decoding="async"
className={className}
ref={setRef}
ref={(element) => {
setRef(element)
removePlaceholder(element, placeholder)
Joonpark13 marked this conversation as resolved.
Show resolved Hide resolved
}}
style={imgStyle}
/>
{priority ? (
Expand Down
2 changes: 2 additions & 0 deletions packages/next/next-server/server/config-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export type NextConfig = { [key: string]: any } & {
turboMode: boolean
eslint?: boolean
reactRoot: boolean
enableBlurryPlaceholder: boolean
}
}

Expand Down Expand Up @@ -117,6 +118,7 @@ export const defaultConfig: NextConfig = {
turboMode: false,
eslint: false,
reactRoot: Number(process.env.NEXT_PRIVATE_REACT_ROOT) > 0,
enableBlurryPlaceholder: false,
},
future: {
strictPostcssConfiguration: false,
Expand Down
2 changes: 2 additions & 0 deletions packages/next/next-server/server/image-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type ImageConfig = {
loader: LoaderValue
path: string
domains?: string[]
enableBlurryPlaceholder: boolean
}

export const imageConfigDefault: ImageConfig = {
Expand All @@ -21,4 +22,5 @@ export const imageConfigDefault: ImageConfig = {
path: '/_next/image',
loader: 'default',
domains: [],
enableBlurryPlaceholder: false,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React from 'react'
import Image from 'next/image'

export default function Page() {
return (
<div>
<p>Blurry Placeholder</p>
<Image
priority
id="blurry-placeholder"
src="/test.jpg"
width="400"
height="400"
placeholder="blur"
blurDataURL="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='400' height='400' viewBox='0 0 400 400'%3E%3Cfilter id='blur' filterUnits='userSpaceOnUse' color-interpolation-filters='sRGB'%3E%3CfeGaussianBlur stdDeviation='20' edgeMode='duplicate' /%3E%3CfeComponentTransfer%3E%3CfeFuncA type='discrete' tableValues='1 1' /%3E%3C/feComponentTransfer%3E%3C/filter%3E%3Cimage filter='url(%23blur)' href='data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMDAwMDAwQEBAQFBQUFBQcHBgYHBwsICQgJCAsRCwwLCwwLEQ8SDw4PEg8bFRMTFRsfGhkaHyYiIiYwLTA+PlT/wAALCAAKAAoBAREA/8QAMwABAQEAAAAAAAAAAAAAAAAAAAcJEAABAwUAAwAAAAAAAAAAAAAFAAYRAQMEEyEVMlH/2gAIAQEAAD8Az1bLPaxhiuk0QdeCOLDtHixN2dmd2bsc5FPX7VTREX//2Q==' x='0' y='0' height='100%25' width='100%25'/%3E%3C/svg%3E"
Joonpark13 marked this conversation as resolved.
Show resolved Hide resolved
/>
</div>
)
}
28 changes: 27 additions & 1 deletion test/integration/image-component/default/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,10 @@ describe('Image Component Tests', () => {
nextConfig,
`
module.exports = {
target: 'serverless'
target: 'serverless',
experimental: {
enableBlurryPlaceholder: true,
},
}
`
)
Expand All @@ -600,6 +603,29 @@ describe('Image Component Tests', () => {
await killApp(app)
})

it('should have blurry placeholder when enabled', async () => {
const html = await renderViaHTTP(appPort, '/blurry-placeholder')
expect(html).toContain(
'background-image:url(&quot;data:image/svg+xml,%3Csvg xmlns=&#x27;http://www.w3.org/2000/svg&#x27; width=&#x27;400&#x27; height=&#x27;400&#x27; viewBox=&#x27;0 0 400 400&#x27;%3E%3Cfilter id=&#x27;blur&#x27; filterUnits=&#x27;userSpaceOnUse&#x27; color-interpolation-filters=&#x27;sRGB&#x27;%3E%3CfeGaussianBlur stdDeviation=&#x27;20&#x27; edgeMode=&#x27;duplicate&#x27; /%3E%3CfeComponentTransfer%3E%3CfeFuncA type=&#x27;discrete&#x27; tableValues=&#x27;1 1&#x27; /%3E%3C/feComponentTransfer%3E%3C/filter%3E%3Cimage filter=&#x27;url(%23blur)&#x27; href=&#x27;data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMDAwMDAwQEBAQFBQUFBQcHBgYHBwsICQgJCAsRCwwLCwwLEQ8SDw4PEg8bFRMTFRsfGhkaHyYiIiYwLTA+PlT/wAALCAAKAAoBAREA/8QAMwABAQEAAAAAAAAAAAAAAAAAAAcJEAABAwUAAwAAAAAAAAAAAAAFAAYRAQMEEyEVMlH/2gAIAQEAAD8Az1bLPaxhiuk0QdeCOLDtHixN2dmd2bsc5FPX7VTREX//2Q==&#x27; x=&#x27;0&#x27; y=&#x27;0&#x27; height=&#x27;100%25&#x27; width=&#x27;100%25&#x27;/%3E%3C/svg%3E&quot;)'
)
})

it('should remove blurry placeholder after image loads', async () => {
let browser
try {
browser = await webdriver(appPort, '/blurry-placeholder')
const id = 'blurry-placeholder'
const backgroundImage = await browser.eval(
`window.getComputedStyle(document.getElementById('${id}')).getPropertyValue('background-image')`
)
expect(backgroundImage).toBe('none')
} finally {
if (browser) {
await browser.close()
}
}
})

Comment on lines +606 to +628
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As suggested by @atcastle I'm adding the integration tests to the serverless suite only in order to avoid changing the config and having to rebuild. This should be fine for now since there's no functional difference for this feature between the modes and when this feature graduates out of experimental, we can come back and place this where it should live with the right config.

runTests('serverless')
})
})