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.

Sign up for GitHub

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

Jump to bottom

Remove useState from next/image #43587

Merged
merged 4 commits into from Dec 1, 2022
Merged

Remove useState from next/image #43587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ff2cfc9
Remove `useState` from `next/image`
styfle Nov 30, 2022
19d681b
Update tests
styfle Dec 1, 2022
ff6c654
Fix lint
styfle Dec 1, 2022
cb49546
Fix prod tests
styfle Dec 1, 2022
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
47 changes: 19 additions & 28 deletions packages/next/client/image.tsx
View file
Open in desktop
Expand Up @@ -6,7 +6,6 @@ import React, {
useCallback,
useContext,
useMemo,
useState,
forwardRef,
} from 'react'
import Head from '../shared/lib/head'
Expand Down Expand Up @@ -149,8 +148,6 @@ type ImageElementProps = Omit<ImageProps, 'src' | 'alt' | 'loader'> & {
placeholder: PlaceholderValue
onLoadRef: React.MutableRefObject<OnLoad | undefined>
onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>
setBlurComplete: (b: boolean) => void
setShowAltText: (b: boolean) => void
}

function getWidths(
Expand Down Expand Up @@ -264,10 +261,8 @@ function getInt(x: unknown): number | undefined {
function handleLoading(
img: ImgElementWithDataProp,
src: string,
placeholder: PlaceholderValue,
onLoadRef: React.MutableRefObject<OnLoad | undefined>,
onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>,
setBlurComplete: (b: boolean) => void,
unoptimized: boolean
) {
if (!img || img['data-loaded-src'] === src) {
Expand All @@ -284,9 +279,6 @@ function handleLoading(
// - decode() completes
return
}
if (placeholder === 'blur') {
setBlurComplete(true)
}
if (onLoadRef?.current) {
// Since we don't have the SyntheticEvent here,
// we must create one with the same shape.
Expand Down Expand Up @@ -383,8 +375,6 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
loader,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
setShowAltText,
onLoad,
onError,
...rest
Expand Down Expand Up @@ -441,20 +431,16 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
handleLoading(
img,
srcString,
placeholder,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
unoptimized
)
}
},
[
srcString,
placeholder,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
onError,
unoptimized,
forwardedRef,
Expand All @@ -465,20 +451,30 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
handleLoading(
img,
srcString,
placeholder,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
unoptimized
)
}}
onError={(event) => {
// if the real image fails to load, this will ensure "alt" is visible
setShowAltText(true)
if (placeholder === 'blur') {
// If the real image fails to load, this will still remove the placeholder.
setBlurComplete(true)
// Note: We removed React.useState() in the error case here
// because it was causing Safari to become very slow when
// there were many images on the same page.
const { style } = event.currentTarget

if (style.color === 'transparent') {
// If src image fails to load, this will ensure "alt" is visible
style.color = ''
}

if (placeholder === 'blur' && style.backgroundImage) {
// If src image fails to load, this will ensure the placeholder is removed
style.backgroundSize = ''
style.backgroundPosition = ''
style.backgroundRepeat = ''
style.backgroundImage = ''
}

if (onError) {
onError(event)
}
Expand Down Expand Up @@ -626,9 +622,6 @@ const Image = forwardRef<HTMLImageElement | null, ImageProps>(
unoptimized = true
}

const [blurComplete, setBlurComplete] = useState(false)
const [showAltText, setShowAltText] = useState(false)

const qualityInt = getInt(quality)

if (process.env.NODE_ENV !== 'production') {
Expand Down Expand Up @@ -810,12 +803,12 @@ const Image = forwardRef<HTMLImageElement | null, ImageProps>(
objectPosition,
}
: {},
showAltText ? {} : { color: 'transparent' },
{ color: 'transparent' },
style
)

const blurStyle =
placeholder === 'blur' && blurDataURL && !blurComplete
placeholder === 'blur' && blurDataURL
? {
backgroundSize: imgStyle.objectFit || 'cover',
backgroundPosition: imgStyle.objectPosition || '50% 50%',
Expand Down Expand Up @@ -905,8 +898,6 @@ const Image = forwardRef<HTMLImageElement | null, ImageProps>(
srcString,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
setShowAltText,
...rest,
}
return (
Expand Down
31 changes: 0 additions & 31 deletions test/integration/next-image-new/default/pages/blurry-placeholder.js
View file
Open in desktop

This file was deleted.

66 changes: 4 additions & 62 deletions test/integration/next-image-new/default/test/index.test.ts
View file
Open in desktop
Expand Up @@ -599,8 +599,8 @@ function runTests(mode) {
'lazy'
)
expect(await browser.elementById('blur1').getAttribute('sizes')).toBeNull()
expect(await browser.elementById('blur1').getAttribute('style')).toBe(
'color: transparent;'
expect(await browser.elementById('blur1').getAttribute('style')).toMatch(
'color:transparent;background-size:cover;background-position:50% 50%;background-repeat:no-repeat;'
)
expect(await browser.elementById('blur1').getAttribute('height')).toBe(
'400'
Expand Down Expand Up @@ -646,8 +646,8 @@ function runTests(mode) {
expect(await browser.elementById('blur2').getAttribute('loading')).toBe(
'lazy'
)
expect(await browser.elementById('blur2').getAttribute('style')).toBe(
'color: transparent;'
expect(await browser.elementById('blur2').getAttribute('style')).toMatch(
'color:transparent;background-size:cover;background-position:50% 50%;background-repeat:no-repeat'
)
expect(await browser.elementById('blur2').getAttribute('height')).toBe(
'400'
Expand Down Expand Up @@ -1084,15 +1084,6 @@ function runTests(mode) {
expect(await getComputedStyle(browser, 'img-blur', 'filter')).toBe(
'opacity(0.5)'
)
expect(await getComputedStyle(browser, 'img-blur', 'background-size')).toBe(
'30%'
)
expect(
await getComputedStyle(browser, 'img-blur', 'background-image')
).toMatch('iVBORw0KGgo=')
expect(
await getComputedStyle(browser, 'img-blur', 'background-position')
).toBe('1px 2px')
})
describe('Fill-mode tests', () => {
let browser
Expand Down Expand Up @@ -1187,55 +1178,6 @@ function runTests(mode) {
})
}

it('should have blurry placeholder when enabled', async () => {
const html = await renderViaHTTP(appPort, '/blurry-placeholder')
const $html = cheerio.load(html)

$html('noscript > img').attr('id', 'unused')

expect($html('#blurry-placeholder-raw')[0].attribs.style).toContain(
`color:transparent;background-size:cover;background-position:50% 50%;background-repeat:no-repeat;background-image:url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http%3A//www.w3.org/2000/svg' viewBox='0 0 400 400'%3E%3Cfilter id='b' color-interpolation-filters='sRGB'%3E%3CfeGaussianBlur stdDeviation='20'/%3E%3C/filter%3E%3Cimage preserveAspectRatio='none' filter='url(%23b)' x='0' y='0' height='100%25' width='100%25' href='data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8P4nhDwAGuAKPn6cicwAAAABJRU5ErkJggg=='/%3E%3C/svg%3E")`
)

expect($html('#blurry-placeholder-with-lazy')[0].attribs.style).toContain(
`color:transparent;background-size:cover;background-position:50% 50%;background-repeat:no-repeat;background-image:url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http%3A//www.w3.org/2000/svg' viewBox='0 0 400 400'%3E%3Cfilter id='b' color-interpolation-filters='sRGB'%3E%3CfeGaussianBlur stdDeviation='20'/%3E%3C/filter%3E%3Cimage preserveAspectRatio='none' filter='url(%23b)' x='0' y='0' height='100%25' width='100%25' href='data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mO0/8/wBwAE/wI85bEJ6gAAAABJRU5ErkJggg=='/%3E%3C/svg%3E")`
)
})

it('should remove blurry placeholder after image loads', async () => {
const browser = await webdriver(appPort, '/blurry-placeholder')
await check(
async () =>
await getComputedStyle(
browser,
'blurry-placeholder-raw',
'background-image'
),
'none'
)
expect(
await getComputedStyle(
browser,
'blurry-placeholder-with-lazy',
'background-image'
)
).toBe(
`url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http%3A//www.w3.org/2000/svg' viewBox='0 0 400 400'%3E%3Cfilter id='b' color-interpolation-filters='sRGB'%3E%3CfeGaussianBlur stdDeviation='20'/%3E%3C/filter%3E%3Cimage preserveAspectRatio='none' filter='url(%23b)' x='0' y='0' height='100%25' width='100%25' href='data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mO0/8/wBwAE/wI85bEJ6gAAAABJRU5ErkJggg=='/%3E%3C/svg%3E")`
)

await browser.eval('document.getElementById("spacer").remove()')

await check(
async () =>
await getComputedStyle(
browser,
'blurry-placeholder-with-lazy',
'background-image'
),
'none'
)
})

it('should be valid HTML', async () => {
let browser
try {
Expand Down
40 changes: 20 additions & 20 deletions test/integration/production/test/index.test.js
View file
Open in desktop
Expand Up @@ -1301,7 +1301,7 @@ describe('Production Usage', () => {
})
}

it('should remove placeholder for next/image correctly', async () => {
it('should loading next/image correctly', async () => {
const browser = await webdriver(context.appPort, '/')

await browser.eval(`(function() {
Expand All @@ -1312,10 +1312,16 @@ describe('Production Usage', () => {

expect(await browser.eval('window.beforeNav')).toBe(1)

await check(
() => browser.elementByCss('img').getComputedCss('background-image'),
'none'
)
await check(async () => {
const result = await browser.eval(
`document.getElementById('static-image').width`
)
if (result === 0) {
throw new Error('Incorrectly loaded image')
}

return 'result-correct'
}, /result-correct/)

await browser.eval(`(function() {
window.beforeNav = 1
Expand All @@ -1332,22 +1338,16 @@ describe('Production Usage', () => {

expect(await browser.eval('window.beforeNav')).toBe(1)

await check(
() =>
browser
.elementByCss('#static-image')
.getComputedCss('background-image'),
'none'
)
await check(async () => {
const result = await browser.eval(
`document.getElementById('static-image').width`
)
if (result === 0) {
throw new Error('Incorrectly loaded image')
}

for (let i = 0; i < 5; i++) {
expect(
await browser
.elementByCss('#static-image')
.getComputedCss('background-image')
).toBe('none')
await waitFor(500)
}
return 'result-correct'
}, /result-correct/)
})

dynamicImportTests(context, (p, q) => renderViaHTTP(context.appPort, p, q))
Expand Down