From fbaff55b29a1cddad5437d7f76f69a5213a5a452 Mon Sep 17 00:00:00 2001
From: Jared Lunde <jared.lunde@gmail.com>
Date: Wed, 9 Sep 2020 09:20:59 -0600
Subject: [PATCH] fix(use-positioner): re-initialize positioner instance before
 render

fix #12, thank you to @khmm12 for a detailed explanation and fix
---
 src/__snapshots__/index.test.tsx.snap |  2 +-
 src/index.test.tsx                    | 31 +++++++++++++---
 src/use-positioner.ts                 | 52 +++++++++++++++------------
 3 files changed, 57 insertions(+), 28 deletions(-)

diff --git a/src/__snapshots__/index.test.tsx.snap b/src/__snapshots__/index.test.tsx.snap
index 3e1dce5..38e4f46 100644
--- a/src/__snapshots__/index.test.tsx.snap
+++ b/src/__snapshots__/index.test.tsx.snap
@@ -46,7 +46,7 @@ exports[`<Masonry> should update when the size of the window changes: Should dis
     </div>
     <div
       role="griditem"
-      style="top: 400px; left: 0px; width: 100px; writing-mode: horizontal-tb; position: absolute;"
+      style="width: 100px; position: absolute; writing-mode: horizontal-tb; top: 400px; left: 0px;"
     >
       <div
         style="width: 100%; height: 400px;"
diff --git a/src/index.test.tsx b/src/index.test.tsx
index 5b90675..cdc71a7 100644
--- a/src/index.test.tsx
+++ b/src/index.test.tsx
@@ -379,6 +379,22 @@ describe('usePositioner()', () => {
     result.current.update([1, 204])
     expect(result.current.shortestColumn()).toBe(804)
   })
+
+  it('should create a new positioner when deps change', () => {
+    const {result, rerender} = renderHook(
+      ({deps, ...props}) => usePositioner(props, deps),
+      {
+        initialProps: {width: 1280, columnCount: 1, deps: [1]},
+      }
+    )
+
+    const initialPositioner = result.current
+    rerender({width: 1280, columnCount: 1, deps: [1]})
+    expect(result.current).toBe(initialPositioner)
+
+    rerender({width: 1280, columnCount: 1, deps: [2]})
+    expect(result.current).not.toBe(initialPositioner)
+  })
 })
 
 describe('useContainerPosition()', () => {
@@ -828,12 +844,15 @@ const FakeCard = ({data: {height}, index}): React.ReactElement => (
 // Simulate scroll events
 const scrollEvent = document.createEvent('Event')
 scrollEvent.initEvent('scroll', true, true)
-const scrollTo = (value): void => {
+const setScroll = (value): void => {
   Object.defineProperty(window, 'scrollY', {value, configurable: true})
+}
+const scrollTo = (value): void => {
+  setScroll(value)
   window.dispatchEvent(scrollEvent)
 }
 const resetScroll = (): void => {
-  scrollTo(0)
+  setScroll(0)
 }
 
 // Simulate window resize event
@@ -842,7 +861,7 @@ resizeEvent.initEvent('resize', true, true)
 const orientationEvent = document.createEvent('Event')
 orientationEvent.initEvent('orientationchange', true, true)
 
-const resizeTo = (width, height) => {
+const setWindowSize = (width, height) => {
   Object.defineProperty(document.documentElement, 'clientWidth', {
     value: width,
     configurable: true,
@@ -851,11 +870,15 @@ const resizeTo = (width, height) => {
     value: height,
     configurable: true,
   })
+}
+
+const resizeTo = (width, height) => {
+  setWindowSize(width, height)
   window.dispatchEvent(resizeEvent)
 }
 
 const resetSize = () => {
-  resizeTo(1280, 720)
+  setWindowSize(1280, 720)
 }
 
 // performance.now mock
diff --git a/src/use-positioner.ts b/src/use-positioner.ts
index dd36017..ad6a16a 100644
--- a/src/use-positioner.ts
+++ b/src/use-positioner.ts
@@ -1,5 +1,4 @@
 import * as React from 'react'
-import useLayoutEffect from '@react-hook/passive-layout-effect'
 import {createIntervalTree} from './interval-tree'
 
 /**
@@ -34,33 +33,40 @@ export function usePositioner(
       columnGutter
     )
   }
-  const [positioner, setPositioner] = React.useState<Positioner>(initPositioner)
-  const didMount = React.useRef(0)
+  // eslint-disable-next-line prefer-const
+  let [positioner, setPositioner] = React.useState<Positioner>(initPositioner)
+  const prevDeps = React.useRef(deps)
+  const opts = [width, columnWidth, columnGutter, columnCount]
+  const prevOpts = React.useRef(opts)
+  const optsChanged = !opts.every((item, i) => prevOpts.current[i] === item)
 
-  // Create a new positioner when the dependencies change
-  useLayoutEffect(() => {
-    if (didMount.current) setPositioner(initPositioner())
-    didMount.current = 1
-    // eslint-disable-next-line
-  }, deps)
+  if (typeof process !== 'undefined' && process.env.NODE_ENV !== 'production') {
+    if (deps.length !== prevDeps.current.length) {
+      throw new Error(
+        'usePositioner(): The length of your dependencies array changed.'
+      )
+    }
+  }
 
-  // Updates the item positions any time a prop potentially affecting their
-  // size changes
-  useLayoutEffect(() => {
-    if (didMount.current) {
-      const cacheSize = positioner.size()
-      const nextPositioner = initPositioner()
-      let index = 0
+  // Create a new positioner when the dependencies or sizes change
+  // Thanks to https://github.com/khmm12 for pointing this out
+  // https://github.com/jaredLunde/masonic/pull/41
+  if (optsChanged || !deps.every((item, i) => prevDeps.current[i] === item)) {
+    const prevPositioner = positioner
+    positioner = initPositioner()
+    prevDeps.current = deps
+    prevOpts.current = opts
 
-      for (; index < cacheSize; index++) {
-        const pos = positioner.get(index)
-        nextPositioner.set(index, pos !== void 0 ? pos.height : 0)
+    if (optsChanged) {
+      const cacheSize = prevPositioner.size()
+      for (let index = 0; index < cacheSize; index++) {
+        const pos = prevPositioner.get(index)
+        positioner.set(index, pos !== void 0 ? pos.height : 0)
       }
-
-      setPositioner(nextPositioner)
     }
-    // eslint-disable-next-line
-  }, [width, columnWidth, columnGutter, columnCount])
+
+    setPositioner(positioner)
+  }
 
   return positioner
 }