Skip to content

Commit

Permalink
fix: reset visible state when src/href changed (#35287)
Browse files Browse the repository at this point in the history
* fix(#35286): reset visible state when image src changed

* fix(#35286): reset visible state when link href changed

* test(image): image with new src should be re-lazyloaded

* test: apply code suggestions from @styfle's review

* test: incorrect condition of image load check
  • Loading branch information
SukkaW committed Apr 4, 2022
1 parent 4db8c49 commit 081e7b0
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 11 deletions.
20 changes: 13 additions & 7 deletions packages/next/client/image.tsx
Expand Up @@ -417,11 +417,12 @@ export default function Image({
isLazy = false
}

const [setIntersection, isIntersected] = useIntersection<HTMLImageElement>({
rootRef: lazyRoot,
rootMargin: lazyBoundary,
disabled: !isLazy,
})
const [setIntersection, isIntersected, resetIntersected] =
useIntersection<HTMLImageElement>({
rootRef: lazyRoot,
rootMargin: lazyBoundary,
disabled: !isLazy,
})
const isVisible = !isLazy || isIntersected

const wrapperStyle: JSX.IntrinsicElements['span']['style'] = {
Expand Down Expand Up @@ -639,7 +640,6 @@ export default function Image({
? { aspectRatio: `${widthInt} / ${heightInt}` }
: layoutStyle
)

const blurStyle =
placeholder === 'blur'
? {
Expand Down Expand Up @@ -743,14 +743,20 @@ export default function Image({
typeof window === 'undefined' ? React.useEffect : React.useLayoutEffect
const onLoadingCompleteRef = useRef(onLoadingComplete)

const previousImageSrc = useRef<string | StaticImport>(src)
const imgRef = useRef<HTMLImageElement>(null)
useEffect(() => {
onLoadingCompleteRef.current = onLoadingComplete
}, [onLoadingComplete])

useLayoutEffect(() => {
if (previousImageSrc.current !== src) {
resetIntersected()
previousImageSrc.current = src
}

setIntersection(imgRef.current)
}, [setIntersection])
}, [setIntersection, resetIntersected, src])

useEffect(() => {
handleLoading(imgRef, srcString, layout, placeholder, onLoadingCompleteRef)
Expand Down
15 changes: 13 additions & 2 deletions packages/next/client/link.tsx
Expand Up @@ -219,6 +219,9 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
}
}, [router, props.href, props.as])

const previousHref = React.useRef<string>(href)
const previousAs = React.useRef<string>(as)

let { children, replace, shallow, scroll, locale } = props

if (typeof children === 'string') {
Expand Down Expand Up @@ -248,11 +251,19 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
}
const childRef: any = child && typeof child === 'object' && child.ref

const [setIntersectionRef, isVisible] = useIntersection({
const [setIntersectionRef, isVisible, resetVisible] = useIntersection({
rootMargin: '200px',
})

const setRef = React.useCallback(
(el: Element) => {
// Before the link getting observed, check if visible state need to be reset
if (previousAs.current !== as || previousHref.current !== href) {
resetVisible()
previousAs.current = as
previousHref.current = href
}

setIntersectionRef(el)
if (childRef) {
if (typeof childRef === 'function') childRef(el)
Expand All @@ -261,7 +272,7 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
}
}
},
[childRef, setIntersectionRef]
[as, childRef, href, resetVisible, setIntersectionRef]
)
React.useEffect(() => {
const shouldPrefetch = isVisible && p && isLocalURL(href)
Expand Down
8 changes: 6 additions & 2 deletions packages/next/client/use-intersection.tsx
Expand Up @@ -29,7 +29,7 @@ export function useIntersection<T extends Element>({
rootRef,
rootMargin,
disabled,
}: UseIntersection): [(element: T | null) => void, boolean] {
}: UseIntersection): [(element: T | null) => void, boolean, () => void] {
const isDisabled: boolean = disabled || !hasIntersectionObserver

const unobserve = useRef<Function>()
Expand All @@ -55,6 +55,10 @@ export function useIntersection<T extends Element>({
[isDisabled, root, rootMargin, visible]
)

const resetVisible = useCallback(() => {
setVisible(false)
}, [])

useEffect(() => {
if (!hasIntersectionObserver) {
if (!visible) {
Expand All @@ -67,7 +71,7 @@ export function useIntersection<T extends Element>({
useEffect(() => {
if (rootRef) setRoot(rootRef.current)
}, [rootRef])
return [setRef, visible]
return [setRef, visible, resetVisible]
}

function observe(
Expand Down
18 changes: 18 additions & 0 deletions test/integration/image-component/default/pages/lazy-src-change.js
@@ -0,0 +1,18 @@
import React from 'react'
import Image from 'next/image'

const Page = () => {
const [src, setSrc] = React.useState('/test.jpg')
return (
<div>
<p>Home Page</p>
<div id="spacer" style={{ height: '150vh' }} />
<Image id="basic-image" src={src} width="400" height="400"></Image>
<button id="button-change-image-src" onClick={() => setSrc('/test.png')}>
Change Image
</button>
</div>
)
}

export default Page
89 changes: 89 additions & 0 deletions test/integration/image-component/default/test/index.test.js
Expand Up @@ -1183,6 +1183,95 @@ function runTests(mode) {
}
})

it('should re-lazyload images after src changes', async () => {
let browser
try {
browser = await webdriver(appPort, '/lazy-src-change')
// image should not be loaded as it is out of viewport
await check(async () => {
const result = await browser.eval(
`document.getElementById('basic-image').naturalWidth`
)

if (result >= 400) {
throw new Error('Incorrectly loaded image')
}

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

// Move image into viewport
await browser.eval(
'document.getElementById("spacer").style.display = "none"'
)

// image should be loaded by now
await check(async () => {
const result = await browser.eval(
`document.getElementById('basic-image').naturalWidth`
)

if (result < 400) {
throw new Error('Incorrectly loaded image')
}

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

await check(
() => browser.eval(`document.getElementById("basic-image").currentSrc`),
/test\.jpg/
)

// Make image out of viewport again
await browser.eval(
'document.getElementById("spacer").style.display = "block"'
)
// Toggle image's src
await browser.eval(
'document.getElementById("button-change-image-src").click()'
)
// "new" image should be lazy loaded
await check(async () => {
const result = await browser.eval(
`document.getElementById('basic-image').naturalWidth`
)

if (result >= 400) {
throw new Error('Incorrectly loaded image')
}

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

// Move image into viewport again
await browser.eval(
'document.getElementById("spacer").style.display = "none"'
)
// "new" image should be loaded by now
await check(async () => {
const result = await browser.eval(
`document.getElementById('basic-image').naturalWidth`
)

if (result < 400) {
throw new Error('Incorrectly loaded image')
}

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

await check(
() => browser.eval(`document.getElementById("basic-image").currentSrc`),
/test\.png/
)
} finally {
if (browser) {
await browser.close()
}
}
})

it('should initially load only two of four images using lazyroot', async () => {
let browser
try {
Expand Down

0 comments on commit 081e7b0

Please sign in to comment.