Fix infinite loop when image onLoadingComplete calls setState

vercel · Jan 19, 2022 · bfdf0c3 · bfdf0c3
1 parent 1d4f364
commit bfdf0c3
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 60 deletions.
diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx
@@ -263,14 +263,15 @@ function handleLoading(
     if (img.src !== emptyDataURL) {
       const p = 'decode' in img ? img.decode() : Promise.resolve()
       p.catch(() => {}).then(() => {
+        const isLoaded = loadedImageURLs.has(src)
+        loadedImageURLs.add(src)
         if (placeholder === 'blur') {
           img.style.filter = ''
           img.style.backgroundSize = ''
           img.style.backgroundImage = ''
           img.style.backgroundPosition = ''
         }
-        loadedImageURLs.add(src)
-        if (onLoadingComplete) {
+        if (onLoadingComplete && !isLoaded) {
           const { naturalWidth, naturalHeight } = img
           // Pass back read-only primitive values but not the
           // underlying DOM element because it could be misused.

diff --git a/test/integration/image-component/default/pages/on-loading-complete.js b/test/integration/image-component/default/pages/on-loading-complete.js
@@ -1,74 +1,108 @@
 import { useState } from 'react'
 import Image from 'next/image'
 
-const Page = () => (
-  <div>
-    <h1>On Loading Complete Test</h1>
-    <ImageWithMessage
-      id="1"
-      src="/test.jpg"
-      layout="intrinsic"
-      width="128"
-      height="128"
-    />
-    <hr />
-    <ImageWithMessage
-      id="2"
-      src={require('../public/test.png')}
-      placeholder="blur"
-      layout="fixed"
-    />
-    <hr />
-    <ImageWithMessage
-      id="3"
-      src="/test.svg"
-      layout="responsive"
-      width="1200"
-      height="1200"
-    />
-    <hr />
-    <div style={{ position: 'relative', width: '64px', height: '64px' }}>
+const Page = () => {
+  // Hoisted state to count each image load callback
+  const [idToCount, setIdToCount] = useState({})
+
+  return (
+    <div>
+      <h1>On Loading Complete Test</h1>
+      <ImageWithMessage
+        id="1"
+        src="/test.jpg"
+        layout="intrinsic"
+        width="128"
+        height="128"
+        idToCount={idToCount}
+        setIdToCount={setIdToCount}
+      />
+
+      <ImageWithMessage
+        id="2"
+        src={require('../public/test.png')}
+        placeholder="blur"
+        layout="fixed"
+        idToCount={idToCount}
+        setIdToCount={setIdToCount}
+      />
+
+      <ImageWithMessage
+        id="3"
+        src="/test.svg"
+        layout="responsive"
+        width="1200"
+        height="1200"
+        idToCount={idToCount}
+        setIdToCount={setIdToCount}
+      />
+
       <ImageWithMessage
         id="4"
         src="/test.ico"
         layout="fill"
         objectFit="contain"
+        idToCount={idToCount}
+        setIdToCount={setIdToCount}
       />
-    </div>
-    <hr />
-    <ImageWithMessage
-      id="5"
-      src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAMAAAAFCAYAAACAcVaiAAAAE0lEQVR42mNk+P+/ngEKGMngAADDdwx3Uz/3AAAAAABJRU5ErkJggg=="
-      layout="fixed"
-      width={64}
-      height={64}
-    />
-    <hr />
-    <div style={{ position: 'relative', width: '64px', height: '64px' }}>
+
       <ImageWithMessage
-        id="6"
+        id="5"
         src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAMAAAAFCAYAAACAcVaiAAAAE0lEQVR42mNk+P+/ngEKGMngAADDdwx3Uz/3AAAAAABJRU5ErkJggg=="
+        layout="fixed"
+        width={64}
+        height={64}
+        idToCount={idToCount}
+        setIdToCount={setIdToCount}
+      />
+
+      <ImageWithMessage
+        id="6"
+        src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg=="
         layout="fill"
+        idToCount={idToCount}
+        setIdToCount={setIdToCount}
       />
+
+      <ImageWithMessage
+        id="7"
+        src="/test.bmp"
+        loading="eager"
+        width={400}
+        height={400}
+        idToCount={idToCount}
+        setIdToCount={setIdToCount}
+      />
+      <div id="footer" />
     </div>
-    <div id="footer" />
-  </div>
-)
+  )
+}
 
-function ImageWithMessage({ id, ...props }) {
+function ImageWithMessage({ id, idToCount, setIdToCount, ...props }) {
   const [msg, setMsg] = useState('[LOADING]')
+  const style =
+    props.layout === 'fill'
+      ? { position: 'relative', width: '64px', height: '64px' }
+      : {}
   return (
     <>
-      <Image
-        id={`img${id}`}
-        onLoadingComplete={({ naturalWidth, naturalHeight }) =>
-          setMsg(
-            `loaded img${id} with dimensions ${naturalWidth}x${naturalHeight}`
-          )
-        }
-        {...props}
-      />
+      <div className="wrap" style={style}>
+        <Image
+          id={`img${id}`}
+          onLoadingComplete={({ naturalWidth, naturalHeight }) => {
+            const count = idToCount[id] || 0
+            count++
+            idToCount[id] = count
+            setIdToCount(idToCount)
+            const msg = `loaded ${count} img${id} with dimensions ${naturalWidth}x${naturalHeight}`
+            setMsg(msg)
+            console.log(msg)
+          }}
+          {...props}
+        />
+      </div>
       <p id={`msg${id}`}>{msg}</p>
+      <hr />
     </>
   )
 }

diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js
@@ -213,27 +213,31 @@ function runTests(mode) {
       )
       await check(
         () => browser.eval(`document.getElementById("msg1").textContent`),
-        'loaded img1 with dimensions 128x128'
+        'loaded 1 img1 with dimensions 128x128'
       )
       await check(
         () => browser.eval(`document.getElementById("msg2").textContent`),
-        'loaded img2 with dimensions 400x400'
+        'loaded 1 img2 with dimensions 400x400'
       )
       await check(
         () => browser.eval(`document.getElementById("msg3").textContent`),
-        'loaded img3 with dimensions 266x266'
+        'loaded 1 img3 with dimensions 266x266'
       )
       await check(
         () => browser.eval(`document.getElementById("msg4").textContent`),
-        'loaded img4 with dimensions 21x21'
+        'loaded 1 img4 with dimensions 21x21'
       )
       await check(
         () => browser.eval(`document.getElementById("msg5").textContent`),
-        'loaded img5 with dimensions 3x5'
+        'loaded 1 img5 with dimensions 3x5'
       )
       await check(
         () => browser.eval(`document.getElementById("msg6").textContent`),
-        'loaded img6 with dimensions 3x5'
+        'loaded 1 img6 with dimensions 3x5'
+      )
+      await check(
+        () => browser.eval(`document.getElementById("msg7").textContent`),
+        'loaded 1 img7 with dimensions 200x200'
       )
     } finally {
       if (browser) {