From 07adc8ef2618866c77218f619d9f49942a0cbde0 Mon Sep 17 00:00:00 2001 From: Steven Date: Thu, 22 Oct 2020 14:59:42 -0400 Subject: [PATCH] Change Image component lazy=true to loading=lazy (#18138) This PR updates the `` component to follow the same property naming as native ``. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Img#attr-loading This currently allows two values,`loading=lazy` and `loading=eager`, but there might be new values added in a future spec. cc @atcastle --- packages/next/client/image.tsx | 27 ++++++++++------ .../basic/pages/client-side.js | 8 ++--- .../image-component/basic/pages/index.js | 8 ++--- .../image-component/basic/pages/lazy.js | 27 ++++++++++++++-- .../image-component/basic/test/index.test.js | 32 +++++++++++++++++++ 5 files changed, 82 insertions(+), 20 deletions(-) diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx index 14cf4abe18..7ca82668b8 100644 --- a/packages/next/client/image.tsx +++ b/packages/next/client/image.tsx @@ -1,6 +1,9 @@ import React, { ReactElement, useEffect, useRef } from 'react' import Head from '../next-server/lib/head' +const VALID_LOADING_VALUES = ['lazy', 'eager', undefined] as const +type LoadingValue = typeof VALID_LOADING_VALUES[number] + const loaders = new Map string>([ ['imgix', imgixLoader], ['cloudinary', cloudinaryLoader], @@ -18,12 +21,12 @@ type ImageData = { type ImageProps = Omit< JSX.IntrinsicElements['img'], - 'src' | 'srcSet' | 'ref' | 'width' | 'height' + 'src' | 'srcSet' | 'ref' | 'width' | 'height' | 'loading' > & { src: string quality?: string priority?: boolean - lazy?: boolean + loading?: LoadingValue unoptimized?: boolean } & ( | { width: number; height: number; unsized?: false } @@ -142,7 +145,7 @@ export default function Image({ sizes, unoptimized = false, priority = false, - lazy, + loading, className, quality, width, @@ -152,17 +155,23 @@ export default function Image({ }: ImageProps) { const thisEl = useRef(null) - // Sanity Checks: - // If priority and lazy are present, log an error and use priority only. - if (priority && lazy) { - if (process.env.NODE_ENV !== 'production') { + if (process.env.NODE_ENV !== 'production') { + if (!VALID_LOADING_VALUES.includes(loading)) { throw new Error( - `Image with src "${src}" has both "priority" and "lazy" properties. Only one should be used.` + `Image with src "${src}" has invalid "loading" property. Provided "${loading}" should be one of ${VALID_LOADING_VALUES.map( + String + ).join(',')}.` + ) + } + if (priority && loading === 'lazy') { + throw new Error( + `Image with src "${src}" has both "priority" and "loading=lazy" properties. Only one should be used.` ) } } - if (!priority && typeof lazy === 'undefined') { + let lazy = loading === 'lazy' + if (!priority && typeof loading === 'undefined') { lazy = true } diff --git a/test/integration/image-component/basic/pages/client-side.js b/test/integration/image-component/basic/pages/client-side.js index 3b0555affc..99f97951f2 100644 --- a/test/integration/image-component/basic/pages/client-side.js +++ b/test/integration/image-component/basic/pages/client-side.js @@ -9,7 +9,7 @@ const ClientSide = () => { { id="attribute-test" data-demo="demo-value" src="bar.jpg" - lazy={false} + loading="eager" width={300} height={400} /> @@ -27,7 +27,7 @@ const ClientSide = () => { data-demo="demo-value" host="secondary" src="foo2.jpg" - lazy={false} + loading="eager" width={300} height={400} /> @@ -35,7 +35,7 @@ const ClientSide = () => { id="unoptimized-image" unoptimized src="https://arbitraryurl.com/foo.jpg" - lazy={false} + loading="eager" width={300} height={400} /> diff --git a/test/integration/image-component/basic/pages/index.js b/test/integration/image-component/basic/pages/index.js index 667f567d82..18f66c9b0e 100644 --- a/test/integration/image-component/basic/pages/index.js +++ b/test/integration/image-component/basic/pages/index.js @@ -9,7 +9,7 @@ const Page = () => { { id="attribute-test" data-demo="demo-value" src="bar.jpg" - lazy={false} + loading="eager" width={300} height={400} /> @@ -27,7 +27,7 @@ const Page = () => { data-demo="demo-value" host="secondary" src="foo2.jpg" - lazy={false} + loading="eager" width={300} height={400} /> @@ -35,7 +35,7 @@ const Page = () => { id="unoptimized-image" unoptimized src="https://arbitraryurl.com/foo.jpg" - lazy={false} + loading="eager" width={300} height={400} /> diff --git a/test/integration/image-component/basic/pages/lazy.js b/test/integration/image-component/basic/pages/lazy.js index 4d9c223ab3..8a5fe4a01c 100644 --- a/test/integration/image-component/basic/pages/lazy.js +++ b/test/integration/image-component/basic/pages/lazy.js @@ -5,12 +5,18 @@ const Lazy = () => { return (

This is a page with lazy-loaded images

- +
{ height={400} width={300} unoptimized - lazy + loading="lazy" + > +
+ +
+
) diff --git a/test/integration/image-component/basic/test/index.test.js b/test/integration/image-component/basic/test/index.test.js index 0e1ce43e59..7d48d8345f 100644 --- a/test/integration/image-component/basic/test/index.test.js +++ b/test/integration/image-component/basic/test/index.test.js @@ -132,6 +132,38 @@ function lazyLoadingTests() { await browser.elementById('lazy-bottom').getAttribute('srcset') ).toBeFalsy() }) + it('should load the fourth image lazily after scrolling down', async () => { + expect( + await browser.elementById('lazy-without-attribute').getAttribute('src') + ).toBeFalsy() + expect( + await browser.elementById('lazy-without-attribute').getAttribute('srcset') + ).toBeFalsy() + let viewportHeight = await browser.eval(`window.innerHeight`) + let topOfBottomImage = await browser.eval( + `document.getElementById('lazy-without-attribute').parentElement.offsetTop` + ) + let buffer = 150 + await browser.eval( + `window.scrollTo(0, ${topOfBottomImage - (viewportHeight + buffer)})` + ) + await waitFor(200) + expect( + await browser.elementById('lazy-without-attribute').getAttribute('src') + ).toBe('https://example.com/myaccount/foo4.jpg?auto=format') + expect( + await browser.elementById('lazy-without-attribute').getAttribute('srcset') + ).toBeTruthy() + }) + + it('should load the fifth image eagerly, without scrolling', async () => { + expect(await browser.elementById('eager-loading').getAttribute('src')).toBe( + 'https://example.com/myaccount/foo5.jpg?auto=format' + ) + expect( + await browser.elementById('eager-loading').getAttribute('srcset') + ).toBeTruthy() + }) } async function hasPreloadLinkMatchingUrl(url) {