Skip to content

Commit

Permalink
fix(use-positioner): re-initialize positioner instance before render
Browse files Browse the repository at this point in the history
fix #12, thank you to @khmm12 for a detailed explanation and fix
  • Loading branch information
jaredLunde committed Sep 9, 2020
1 parent 59de2b1 commit fbaff55
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/__snapshots__/index.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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;"
Expand Down
31 changes: 27 additions & 4 deletions src/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down
52 changes: 29 additions & 23 deletions src/use-positioner.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react'
import useLayoutEffect from '@react-hook/passive-layout-effect'
import {createIntervalTree} from './interval-tree'

/**
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit fbaff55

Please sign in to comment.