From c8fa2848544b16fe3ab301be91bb05d2f7382596 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Fri, 6 Nov 2020 18:03:15 -0500 Subject: [PATCH] Control prefetching with React (#18904) This pull request fixes `` not updating when new props are passed by removing external DOM mutations and relying on React to do it instead. As an added bonus, I've extracted the intersection observer from both the `` and `` component, as their instance can be shared! The increase in size is minor (+3B), and actually a decrease for apps using both `` and ``. --- Fixes #18698 Fixes #18369 --- packages/next/client/image.tsx | 99 +++-------------- packages/next/client/link.tsx | 93 +++------------- packages/next/client/use-intersection.tsx | 102 ++++++++++++++++++ .../image-component/basic/test/index.test.js | 2 +- .../image-component/default/pages/update.js | 23 ++++ .../default/test/index.test.js | 23 ++++ .../integration/size-limit/test/index.test.js | 2 +- 7 files changed, 183 insertions(+), 161 deletions(-) create mode 100644 packages/next/client/use-intersection.tsx create mode 100644 test/integration/image-component/default/pages/update.js diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx index 24454bc6358b..088ad92a1697 100644 --- a/packages/next/client/image.tsx +++ b/packages/next/client/image.tsx @@ -1,5 +1,6 @@ -import React, { ReactElement, useEffect, useRef } from 'react' +import React, { ReactElement } from 'react' import Head from '../next-server/lib/head' +import { useIntersection } from './use-intersection' const VALID_LOADING_VALUES = ['lazy', 'eager', undefined] as const type LoadingValue = typeof VALID_LOADING_VALUES[number] @@ -71,45 +72,6 @@ const allSizes = [...configDeviceSizes, ...configImageSizes] configDeviceSizes.sort((a, b) => a - b) allSizes.sort((a, b) => a - b) -let cachedObserver: IntersectionObserver - -function getObserver(): IntersectionObserver | undefined { - const IntersectionObserver = - typeof window !== 'undefined' ? window.IntersectionObserver : null - // Return shared instance of IntersectionObserver if already created - if (cachedObserver) { - return cachedObserver - } - - // Only create shared IntersectionObserver if supported in browser - if (!IntersectionObserver) { - return undefined - } - return (cachedObserver = new IntersectionObserver( - (entries) => { - entries.forEach((entry) => { - if (entry.isIntersecting) { - let lazyImage = entry.target as HTMLImageElement - unLazifyImage(lazyImage) - cachedObserver.unobserve(lazyImage) - } - }) - }, - { rootMargin: '200px' } - )) -} - -function unLazifyImage(lazyImage: HTMLImageElement): void { - if (lazyImage.dataset.src) { - lazyImage.src = lazyImage.dataset.src - } - if (lazyImage.dataset.srcset) { - lazyImage.srcset = lazyImage.dataset.srcset - } - lazyImage.style.visibility = 'visible' - lazyImage.classList.remove('__lazy') -} - function getSizes( width: number | undefined, layout: LayoutValue @@ -255,8 +217,6 @@ export default function Image({ objectPosition, ...all }: ImageProps) { - const thisEl = useRef(null) - let rest: Partial = all let layout: NonNullable = sizes ? 'responsive' : 'intrinsic' let unsized = false @@ -306,33 +266,19 @@ export default function Image({ } } - let lazy = loading === 'lazy' - if (!priority && typeof loading === 'undefined') { - lazy = true - } - + let isLazy = + !priority && (loading === 'lazy' || typeof loading === 'undefined') if (src && src.startsWith('data:')) { // https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs unoptimized = true - lazy = false + isLazy = false } - useEffect(() => { - const target = thisEl.current - if (target) { - const observer = lazy && getObserver() - - if (observer) { - observer.observe(target) - - return () => { - observer.unobserve(target) - } - } else { - unLazifyImage(target) - } - } - }, [thisEl, lazy]) + const [setRef, isIntersected] = useIntersection({ + rootMargin: '200px', + disabled: !isLazy, + }) + const isVisible = !isLazy || isIntersected const widthInt = getInt(width) const heightInt = getInt(height) @@ -342,7 +288,7 @@ export default function Image({ let sizerStyle: JSX.IntrinsicElements['div']['style'] | undefined let sizerSvg: string | undefined let imgStyle: ImgElementStyle | undefined = { - visibility: lazy ? 'hidden' : 'visible', + visibility: isVisible ? 'visible' : 'hidden', position: 'absolute', top: 0, @@ -451,29 +397,16 @@ export default function Image({ }) let imgAttributes: - | { - src: string - srcSet?: string - } - | { - 'data-src': string - 'data-srcset'?: string - } - if (!lazy) { + | Pick + | undefined + + if (isVisible) { imgAttributes = { src: imgSrc, } if (imgSrcSet) { imgAttributes.srcSet = imgSrcSet } - } else { - imgAttributes = { - 'data-src': imgSrc, - } - if (imgSrcSet) { - imgAttributes['data-srcset'] = imgSrcSet - } - className = className ? className + ' __lazy' : '__lazy' } // No need to add preloads on the client side--by the time the application is hydrated, @@ -516,7 +449,7 @@ export default function Image({ decoding="async" className={className} sizes={sizes} - ref={thisEl} + ref={setRef} style={imgStyle} /> diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 9c005fcc3f50..1f53da32c45b 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -1,4 +1,4 @@ -import React, { Children } from 'react' +import React, { Children, useEffect } from 'react' import { UrlObject } from 'url' import { addBasePath, @@ -9,6 +9,7 @@ import { resolveHref, } from '../next-server/lib/router/router' import { useRouter } from './router' +import { useIntersection } from './use-intersection' type Url = string | UrlObject type RequiredKeys = { @@ -31,60 +32,8 @@ export type LinkProps = { type LinkPropsRequired = RequiredKeys type LinkPropsOptional = OptionalKeys -let cachedObserver: IntersectionObserver -const listeners = new Map void>() -const IntersectionObserver = - typeof window !== 'undefined' ? window.IntersectionObserver : null const prefetched: { [cacheKey: string]: boolean } = {} -function getObserver(): IntersectionObserver | undefined { - // Return shared instance of IntersectionObserver if already created - if (cachedObserver) { - return cachedObserver - } - - // Only create shared IntersectionObserver if supported in browser - if (!IntersectionObserver) { - return undefined - } - - return (cachedObserver = new IntersectionObserver( - (entries) => { - entries.forEach((entry) => { - if (!listeners.has(entry.target)) { - return - } - - const cb = listeners.get(entry.target)! - if (entry.isIntersecting || entry.intersectionRatio > 0) { - cachedObserver.unobserve(entry.target) - listeners.delete(entry.target) - cb() - } - }) - }, - { rootMargin: '200px' } - )) -} - -const listenToIntersections = (el: Element, cb: () => void) => { - const observer = getObserver() - if (!observer) { - return () => {} - } - - observer.observe(el) - listeners.set(el, cb) - return () => { - try { - observer.unobserve(el) - } catch (err) { - console.error(err) - } - listeners.delete(el) - } -} - function prefetch( router: NextRouter, href: string, @@ -285,30 +234,12 @@ function Link(props: React.PropsWithChildren) { const child: any = Children.only(children) const childRef: any = child && typeof child === 'object' && child.ref - const cleanup = React.useRef<() => void>() + const [setIntersectionRef, isVisible] = useIntersection({ + rootMargin: '200px', + }) const setRef = React.useCallback( (el: Element) => { - // cleanup previous event handlers - if (cleanup.current) { - cleanup.current() - cleanup.current = undefined - } - - if (p && IntersectionObserver && el && el.tagName && isLocalURL(href)) { - // Join on an invalid URI character - const isPrefetched = prefetched[href + '%' + as] - if (!isPrefetched) { - cleanup.current = listenToIntersections(el, () => { - prefetch(router, href, as, { - locale: - typeof locale !== 'undefined' - ? locale - : router && router.locale, - }) - }) - } - } - + setIntersectionRef(el) if (childRef) { if (typeof childRef === 'function') childRef(el) else if (typeof childRef === 'object') { @@ -316,8 +247,18 @@ function Link(props: React.PropsWithChildren) { } } }, - [p, childRef, href, as, router, locale] + [childRef, setIntersectionRef] ) + useEffect(() => { + const shouldPrefetch = isVisible && p && isLocalURL(href) + const isPrefetched = prefetched[href + '%' + as] + if (shouldPrefetch && !isPrefetched) { + prefetch(router, href, as, { + locale: + typeof locale !== 'undefined' ? locale : router && router.locale, + }) + } + }, [as, href, isVisible, locale, p, router]) const childProps: { onMouseEnter?: React.MouseEventHandler diff --git a/packages/next/client/use-intersection.tsx b/packages/next/client/use-intersection.tsx new file mode 100644 index 000000000000..90c38cb8c163 --- /dev/null +++ b/packages/next/client/use-intersection.tsx @@ -0,0 +1,102 @@ +import { useCallback, useEffect, useRef, useState } from 'react' + +type UseIntersectionObserverInit = Pick +type UseIntersection = { disabled?: boolean } & UseIntersectionObserverInit +type ObserveCallback = (isVisible: boolean) => void + +const hasIntersectionObserver = typeof IntersectionObserver !== 'undefined' + +export function useIntersection({ + rootMargin, + disabled, +}: UseIntersection): [(element: T | null) => void, boolean] { + const isDisabled = disabled || !hasIntersectionObserver + + const unobserve = useRef() + const [visible, setVisible] = useState(false) + + const setRef = useCallback( + (el: T | null) => { + if (unobserve.current) { + unobserve.current() + unobserve.current = undefined + } + + if (isDisabled || visible) return + + if (el && el.tagName) { + unobserve.current = observe( + el, + (isVisible) => isVisible && setVisible(isVisible), + { rootMargin } + ) + } + }, + [isDisabled, rootMargin, visible] + ) + + useEffect(() => { + if (!hasIntersectionObserver) { + if (!visible) setVisible(true) + } + }, [visible]) + + return [setRef, visible] +} + +function observe( + element: Element, + callback: ObserveCallback, + options: UseIntersectionObserverInit +) { + const { id, observer, elements } = createObserver(options) + elements.set(element, callback) + + observer.observe(element) + return function unobserve() { + observer.unobserve(element) + + // Destroy observer when there's nothing left to watch: + if (elements.size === 0) { + observer.disconnect() + observers.delete(id) + } + } +} + +const observers = new Map< + string, + { + id: string + observer: IntersectionObserver + elements: Map + } +>() +function createObserver(options: UseIntersectionObserverInit) { + const id = options.rootMargin || '' + let instance = observers.get(id) + if (instance) { + return instance + } + + const elements = new Map() + const observer = new IntersectionObserver((entries) => { + entries.forEach((entry) => { + const callback = elements.get(entry.target) + const isVisible = entry.isIntersecting || entry.intersectionRatio > 0 + if (callback && isVisible) { + callback(isVisible) + } + }) + }, options) + + observers.set( + id, + (instance = { + id, + observer, + elements, + }) + ) + return instance +} diff --git a/test/integration/image-component/basic/test/index.test.js b/test/integration/image-component/basic/test/index.test.js index f637d729c3c4..c02ca2d569c4 100644 --- a/test/integration/image-component/basic/test/index.test.js +++ b/test/integration/image-component/basic/test/index.test.js @@ -106,7 +106,7 @@ function lazyLoadingTests() { }) it('should pass through classes on a lazy loaded image', async () => { expect(await browser.elementById('lazy-mid').getAttribute('class')).toBe( - 'exampleclass __lazy' + 'exampleclass' ) }) it('should load the second image after scrolling down', async () => { diff --git a/test/integration/image-component/default/pages/update.js b/test/integration/image-component/default/pages/update.js new file mode 100644 index 000000000000..c0a7929a5641 --- /dev/null +++ b/test/integration/image-component/default/pages/update.js @@ -0,0 +1,23 @@ +import React, { useState } from 'react' +import Image from 'next/image' + +const Page = () => { + const [toggled, setToggled] = useState(false) + return ( +
+

Update Page

+ +

This is the index page

+ +
+ ) +} + +export default Page diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js index 35ad401a47a1..32342e2b7386 100644 --- a/test/integration/image-component/default/test/index.test.js +++ b/test/integration/image-component/default/test/index.test.js @@ -90,6 +90,29 @@ function runTests(mode) { } }) + it('should update the image on src change', async () => { + let browser + try { + browser = await webdriver(appPort, '/update') + + await check( + () => browser.eval(`document.getElementById("update-image").src`), + /test\.jpg/ + ) + + await browser.eval(`document.getElementById("toggle").click()`) + + await check( + () => browser.eval(`document.getElementById("update-image").src`), + /test\.png/ + ) + } finally { + if (browser) { + await browser.close() + } + } + }) + it('should work when using flexbox', async () => { let browser try { diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index 522fc5680ee4..a5c288666abf 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -80,7 +80,7 @@ describe('Production response size', () => { ) // These numbers are without gzip compression! - const delta = responseSizesBytes - 281 * 1024 + const delta = responseSizesBytes - 282 * 1024 expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target })