refactor: tweak some internals for optimization (#196)

* refactor: tweak some internals for some minor optimization

* test: improve testing

Author: thebuilder

package.json

@@ -128,7 +128,7 @@
     "@typescript-eslint/parser": "^1.4.2",
     "babel-core": "^7.0.0-bridge.0",
     "babel-eslint": "10.0.1",
-    "babel-jest": "^24.0.0",
+    "babel-jest": "^24.4.0",
     "babel-loader": "^8.0.5",
     "concurrently": "4.1.0",
     "coveralls": "^3.0.3",
@@ -140,7 +140,7 @@
     "eslint-plugin-react": "7.x",
     "husky": "^1.3.1",
     "intersection-observer": "^0.5.1",
-    "jest": "^24.0.0",
+    "jest": "^24.4.0",
     "jest-dom": "^3.1.1",
     "lint-staged": "^8.1.5",
     "npm-run-all": "^4.1.5",

src/InView.tsx

@@ -93,8 +93,13 @@ export class InView extends React.Component<
   }
 
   handleChange = (inView: boolean, entry: IntersectionObserverEntry) => {
-    this.setState({ inView, entry })
+    // Only trigger a state update if inView has changed.
+    // This prevents an unnecessary extra state update during mount, when the element stats outside the viewport
+    if (inView !== this.state.inView || inView) {
+      this.setState({ inView, entry })
+    }
     if (this.props.onChange) {
+      // If the user is actively listening for onChange, always trigger it
       this.props.onChange(inView, entry)
     }
   }

src/__tests__/intersection.test.js

@@ -78,6 +78,17 @@ it('should observe with unique rootId', () => {
     observer: expect.any(Object),
   })
 })
+it('should observe with rootMargin', () => {
+  const cb = jest.fn()
+  const root = document.createElement('div')
+  const instance = observe(el, cb, { root, rootMargin: '0px 0px 0px 0px' })
+
+  expect(instance).toMatchObject({
+    inView: false,
+    observerId: '1_0_0px 0px 0px 0px',
+    observer: expect.any(Object),
+  })
+})
 
 it('should unobserve', () => {
   observe(el, jest.fn())
@@ -94,6 +105,18 @@ it('should keep observer when unobserve with multiple elements', () => {
   unobserve(el)
 })
 
+it('should remove unused roots', () => {
+  const el2 = { el: 'htmlElement2' }
+  const el3 = { el: 'htmlElement3' }
+  const root = document.createElement('div')
+  observe(el, jest.fn(), { root, threshold: 0 })
+  observe(el2, jest.fn(), { root, threshold: 0.2 })
+  observe(el3, jest.fn(), { threshold: 0.4 })
+  unobserve(el)
+  unobserve(el2)
+  unobserve(el3)
+})
+
 it('should trigger onChange with ratio 0', () => {
   const cb = jest.fn()
   const instance = observe(el, cb)
@@ -260,3 +283,31 @@ it('should trigger clear visible when going back to 0 with array threshold', ()
 
   expect(instance.inView).toBe(false)
 })
+
+it('should use threshold if not included in IntersectionObserver instance', () => {
+  const cb = jest.fn()
+  IntersectionObserver.mockImplementationOnce(() => ({
+    observe: jest.fn(),
+    unobserve: jest.fn(),
+    disconnect: jest.fn(),
+  }))
+
+  const instance = observe(el, cb, { threshold: 0 })
+  expect(instance).toMatchObject({
+    thresholds: [0],
+  })
+})
+
+it('should use array threshold if not included in IntersectionObserver instance', () => {
+  const cb = jest.fn()
+  IntersectionObserver.mockImplementationOnce(() => ({
+    observe: jest.fn(),
+    unobserve: jest.fn(),
+    disconnect: jest.fn(),
+  }))
+
+  const instance = observe(el, cb, { threshold: [0] })
+  expect(instance).toMatchObject({
+    thresholds: [0],
+  })
+})

src/intersection.ts

@@ -112,16 +112,19 @@ export function unobserve(element: Element | null) {
     let rootObserved = false
     /* istanbul ignore else  */
     if (observerId) {
-      INSTANCE_MAP.forEach((item, key) => {
-        if (item && key !== element) {
-          /* istanbul ignore else  */
-          if (item.observerId === observerId) itemsLeft = true
-          /* istanbul ignore else  */
-          if (item.observer.root === root) rootObserved = true
+      for (let [key, item] of INSTANCE_MAP) {
+        if (key !== element) {
+          if (item.observerId === observerId) {
+            itemsLeft = true
+            rootObserved = true
+            break
+          }
+          if (item.observer.root === root) {
+            rootObserved = true
+          }
         }
-      })
+      }
     }
-
     if (!rootObserved && root) ROOT_IDS.delete(root)
     if (observer && !itemsLeft) {
       // No more elements to observe for threshold, disconnect observer

src/useInView.tsx

@@ -16,23 +16,26 @@ export function useInView(options: IntersectionOptions = {}): HookResponse {
 
   React.useEffect(
     () => {
-      if (ref) {
-        observe(
-          ref,
-          (inView, intersection) => {
+      if (!ref) return
+      observe(
+        ref,
+        (inView, intersection) => {
+          // Only trigger a state update if inView has changed.
+          // This prevents an unnecessary extra state update during mount, when the element stats outside the viewport
+          if (inView !== state.inView || inView) {
             setState({ inView, entry: intersection })
 
             if (inView && options.triggerOnce) {
               // If it should only trigger once, unobserve the element after it's inView
               unobserve(ref)
             }
-          },
-          options,
-        )
-      }
+          }
+        },
+        options,
+      )
 
       return () => {
-        if (ref) unobserve(ref)
+        unobserve(ref)
       }
     },
     [