Fix lazyRoot functionality for next/image (#33933)

Fixed lazyRoot functionality (#33290). Changed the unique id for Intersection Observers discrimination since previously they were only identified by the different rootMargins, now each being identified by the rootMargin and the root element as well Added more Images to the test with different margins and with/without lazyRoot prop. Browser correctly initially loading two of the four Images according to the props' specifications. Co-authored-by: Steven <[email protected]>
vercel · Feb 7, 2022 · d4b29db · d4b29db
1 parent 1ac0fc4
commit d4b29db
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 41 deletions.
diff --git a/packages/next/client/use-intersection.tsx b/packages/next/client/use-intersection.tsx
@@ -13,8 +13,12 @@ type UseIntersection = { disabled?: boolean } & UseIntersectionObserverInit & {
     rootRef?: React.RefObject<HTMLElement> | null
   }
 type ObserveCallback = (isVisible: boolean) => void
+type Identifier = {
+  root: Element | Document | null
+  margin: string
+}
 type Observer = {
-  id: string
+  id: Identifier
   observer: IntersectionObserver
   elements: Map<Element, ObserveCallback>
 }
@@ -83,14 +87,35 @@ function observe(
     if (elements.size === 0) {
       observer.disconnect()
       observers.delete(id)
+      let index = idList.findIndex(
+        (obj) => obj.root === id.root && obj.margin === id.margin
+      )
+      if (index > -1) {
+        idList.splice(index, 1)
+      }
     }
   }
 }
 
-const observers = new Map<string, Observer>()
+const observers = new Map<Identifier, Observer>()
+
+const idList: Identifier[] = []
+
 function createObserver(options: UseIntersectionObserverInit): Observer {
-  const id = options.rootMargin || ''
-  let instance = observers.get(id)
+  const id = {
+    root: options.root || null,
+    margin: options.rootMargin || '',
+  }
+  let existing = idList.find(
+    (obj) => obj.root === id.root && obj.margin === id.margin
+  )
+  let instance
+  if (existing) {
+    instance = observers.get(existing)
+  } else {
+    instance = observers.get(id)
+    idList.push(id)
+  }
   if (instance) {
     return instance
   }

diff --git a/test/integration/image-component/default/pages/lazy-noref.js b/test/integration/image-component/default/pages/lazy-noref.js
diff --git a/test/integration/image-component/default/pages/lazy-withref.js b/test/integration/image-component/default/pages/lazy-withref.js
@@ -18,13 +18,41 @@ const Page = () => {
         <div style={{ width: '400px', height: '600px' }}>hello</div>
         <div style={{ width: '400px', position: 'relative', height: '600px' }}>
           <Image
-            lazyRoot={myRef}
-            id="myImage"
+            id="myImage1"
             src="/test.jpg"
+            alt="mine"
+            width="400"
+            height="400"
+            lazyBoundary="1500px"
+          />
+          <Image
+            lazyRoot={myRef}
+            id="myImage2"
+            src="/test.png"
+            alt="mine"
+            width="400"
+            height="400"
+            lazyBoundary="1800px"
+          />
+          <Image
+            lazyRoot={myRef}
+            id="myImage3"
+            src="/test.svg"
+            alt="mine"
             width="400"
             height="400"
             lazyBoundary="1800px"
           />
+
+          <Image
+            lazyRoot={myRef}
+            id="myImage4"
+            src="/test.webp"
+            alt="mine"
+            width="400"
+            height="400"
+            lazyBoundary="200px"
+          />
         </div>
       </div>
     </>

diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js
@@ -1010,15 +1010,49 @@ function runTests(mode) {
     }
   })
 
-  it('should load the image when the lazyRoot prop is used', async () => {
+  it('should initially load only two of four images using lazyroot', async () => {
     let browser
     try {
-      //trying on '/lazy-noref' it fails
       browser = await webdriver(appPort, '/lazy-withref')
+      await check(async () => {
+        const result = await browser.eval(
+          `document.getElementById('myImage1').naturalWidth`
+        )
+
+        if (result >= 400) {
+          throw new Error('Incorrectly loaded image')
+        }
+
+        return 'result-correct'
+      }, /result-correct/)
+
+      await check(async () => {
+        const result = await browser.eval(
+          `document.getElementById('myImage4').naturalWidth`
+        )
+
+        if (result >= 400) {
+          throw new Error('Incorrectly loaded image')
+        }
+
+        return 'result-correct'
+      }, /result-correct/)
 
       await check(async () => {
         const result = await browser.eval(
-          `document.getElementById('myImage').naturalWidth`
+          `document.getElementById('myImage2').naturalWidth`
+        )
+
+        if (result < 400) {
+          throw new Error('Incorrectly loaded image')
+        }
+
+        return 'result-correct'
+      }, /result-correct/)
+
+      await check(async () => {
+        const result = await browser.eval(
+          `document.getElementById('myImage3').naturalWidth`
         )
 
         if (result < 400) {
@@ -1033,7 +1067,25 @@ function runTests(mode) {
           browser,
           `http://localhost:${appPort}/_next/image?url=%2Ftest.jpg&w=828&q=75`
         )
+      ).toBe(false)
+      expect(
+        await hasImageMatchingUrl(
+          browser,
+          `http://localhost:${appPort}/_next/image?url=%2Ftest.png&w=828&q=75`
+        )
+      ).toBe(true)
+      expect(
+        await hasImageMatchingUrl(
+          browser,
+          `http://localhost:${appPort}/_next/image?url=%2Ftest.svg&w=828&q=75`
+        )
       ).toBe(true)
+      expect(
+        await hasImageMatchingUrl(
+          browser,
+          `http://localhost:${appPort}/_next/image?url=%2Ftest.webp&w=828&q=75`
+        )
+      ).toBe(false)
     } finally {
       if (browser) {
         await browser.close()