fix: don't clear the inView state if we get a node (#337)

* fix: don't clear the inView state if we get a node #308

* fix: use a useEffect to trigger the cleanup

Author: thebuilder

src/__tests__/hooks.test.js

@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useCallback } from 'react'
 import { render } from '@testing-library/react'
 import { useInView } from '../useInView'
 import { intersectionMockInstance, mockAllIsIntersecting } from '../test-utils'
@@ -83,3 +83,75 @@ test('inView should be false when component is unmounted', () => {
   rerender(<HookComponent unmount />)
   getByText('false')
 })
+
+const SwitchHookComponent = ({ options, toggle, unmount }) => {
+  const [ref, inView] = useInView(options)
+  return (
+    <>
+      <div
+        data-testid="item-1"
+        data-inview={!toggle && inView}
+        ref={!toggle && !unmount ? ref : undefined}
+      />
+      <div
+        data-testid="item-2"
+        data-inview={!!toggle && inView}
+        ref={toggle && !unmount ? ref : undefined}
+      />
+    </>
+  )
+}
+
+/**
+ * This is a test for the case where people move the ref around (please don't)
+ */
+test('should handle ref removed', () => {
+  const { rerender, getByTestId } = render(<SwitchHookComponent />)
+  mockAllIsIntersecting(true)
+
+  const item1 = getByTestId('item-1')
+  const item2 = getByTestId('item-2')
+
+  // Item1 should be inView
+  expect(item1.getAttribute('data-inview')).toBe('true')
+  expect(item2.getAttribute('data-inview')).toBe('false')
+
+  rerender(<SwitchHookComponent toggle />)
+  mockAllIsIntersecting(true)
+
+  // Item2 should be inView
+  expect(item1.getAttribute('data-inview')).toBe('false')
+  expect(item2.getAttribute('data-inview')).toBe('true')
+
+  rerender(<SwitchHookComponent unmount />)
+
+  // Nothing should be inView
+  expect(item1.getAttribute('data-inview')).toBe('false')
+  expect(item2.getAttribute('data-inview')).toBe('false')
+
+  // Add the ref back
+  rerender(<SwitchHookComponent />)
+  mockAllIsIntersecting(true)
+  expect(item1.getAttribute('data-inview')).toBe('true')
+  expect(item2.getAttribute('data-inview')).toBe('false')
+})
+
+const MergeRefsComponent = ({ options }) => {
+  const [inViewRef, inView] = useInView(options)
+  const setRef = useCallback(
+    (node) => {
+      inViewRef(node)
+    },
+    [inViewRef],
+  )
+
+  return <div data-testid="inview" data-inview={inView} ref={setRef} />
+}
+
+test('should handle ref merged', () => {
+  const { rerender, getByTestId } = render(<MergeRefsComponent />)
+  mockAllIsIntersecting(true)
+  rerender(<MergeRefsComponent />)
+
+  expect(getByTestId('inview').getAttribute('data-inview')).toBe('true')
+})

src/useInView.tsx

@@ -2,6 +2,7 @@
 import * as React from 'react'
 import { observe, unobserve } from './intersection'
 import { InViewHookResponse, IntersectionOptions } from './index'
+import { useEffect } from 'react'
 
 type State = {
   inView: boolean
@@ -23,13 +24,8 @@ export function useInView(
     (node) => {
       if (ref.current) {
         unobserve(ref.current)
-
-        if (!options.triggerOnce) {
-          // Reset the state, unless the hook is set to only `triggerOnce`
-          // In that case, resetting the state would trigger another update.
-          setState(initialState)
-        }
       }
+
       if (node) {
         observe(
           node,
@@ -45,11 +41,19 @@ export function useInView(
         )
       }
 
-      // Store a reference to the node
+      // Store a reference to the node, so we can unobserve it later
       ref.current = node
     },
     [options.threshold, options.root, options.rootMargin, options.triggerOnce],
   )
 
+  useEffect(() => {
+    if (!ref.current && state !== initialState && !options.triggerOnce) {
+      // If we don't have a ref, then reset the state (unless the hook is set to only `triggerOnce`)
+      // This ensures we correctly reflect the current state - If you aren't observing anything, then nothing is inView
+      setState(initialState)
+    }
+  })
+
   return [setRef, state.inView, state.entry]
 }