Layout is stuck in phase1 if items change before forced updated renders

[wjdwndud0114]

Describe the bug Dug into the code a bit https://github.com/jaredLunde/masonic/blob/main/src/use-masonry.tsx#L84 So needsFreshBatch sets the items into "phase1" state if true and triggers a force re-render through https://github.com/jaredLunde/masonic/blob/main/src/use-masonry.tsx#L187

However, on the very next render, if the items change and there are less items than before: needsFreshBatch remains true again for the new items -> this does not trigger a re-render through the useEffect L187. What results is an empty grid since all the items are stuck in phase1 style (hidden).

Note that this occurs even with a new positioner passed in. The dependency list provided to usePositioner is changed to reset the position cache.

Expected behavior Next render happens and the cells are visible

For some context, I have a loading state with 12 items. However, if there's a cache hit, the loading state is only true for a single render. The next render, items are changed into actual data which can be less than 12 items.

I think the solution here is to modify the logic that triggers the forced update?

[V3RON]

I'm experiencing same thing and results of my investigation are equal. needsFreshBatch stays true between renders and effect is not called, therefore tiles are stuck in phase one.

Reproduction: https://codesandbox.io/s/sharp-sid-1wnt11?file=/src/App.js

I also noticed that if all tiles have same height and itemHeightEstimate is accurate then it doesn't happen. It can be observed by changing itemHeightEstimate to around 20 in the reproduction sandbox. It's probably, because there is no need for second batch immediately after the first one, therefore flag has time to update itself to false first.

Removing dependency array from the effect fixes this issue in attached sandbox, but produces update loop in useMasonry test suite. On the other hand, adding positioner as a dependency seems to pass both jest and manual tests.

[jaredLunde]

Haven't encountered, but PRs are always welcome : )

[V3RON]

Hey @wjdwndud0114, could you confirm adding positioner to the dependency array of forceUpdate effect resolves your issue too? I want to be sure it's exactly the same case before opening a pull request.

[wjdwndud0114]

I'll test that soon, but that makes sense since positioner is updated when the masonry layout should be updated.

Currently, I found a workaround by putting the side effects inside useMemo calls instead of useEffect calls to avoid the single loading render state.

[wjdwndud0114]

Hi, sorry got busy, but I made a replication: https://codesandbox.io/s/usemasonry-example-forked-nj1ri3?file=/src/index.js

In this example, note that only 2 items show, but if you scroll, the rest of the items load. Now change line 21 to useMemo instead of useEffect and it will fix the issue.

[maierson]

I'm running into the same problem while trying to create a masonry component that scrolls within a parent and shows different sets of images according to route. The image data is retrieved on each first route access and then cached locally in Recoil.

Navigating to each route the first time shows all items as expected but returning to a previously loaded route shows only a small subset of the images or none at all (depending on the size of the data). Only items that fit fully into the visible part of the masonry component appear.

[github-actions[bot]]

:tada: This issue has been resolved in version 3.6.5 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[maierson]

I can confirm this solves it for us. Thank you @jaredLunde for the quick response and a really great library.

There is one issue remaining for us that is probably related (not a deal breaker) and has to do with the infinite loader functionality.

Setup:

• we have an overscanBy:2 on the useMasonry hook.
• we have a maybeLoadMore hook with a minimumBatchSize:20 and threshold:20

Actions:

• navigate to route 1 which has hundreds of images available to the infinite scroll to load
• the display fills with images. I can see the maybeLoadMore hook triggers to load enough images to fill the display
• navigate to route 2 which has less than 10 images - all images load correctly
• return to route 1. At this point all the image data for this first display are cached locally

Result:

• only the first page of images shows in the display
• scrolling causes the images on the masonry "page" below (data already in the local cache) to appear but does NOT trigger maybeLoadMore hook (nothing gets loaded further down on scroll)

Fix:

• changing to overscanBy:3 fixes the issue (everything behaves as expected - the extra overscroll restores the maybeLoadMore hook to trigger).

Note that the height of the parent component holding the embedded masonry is large enough to show more than one page of images (ie more than 20 images as set in the minimumBatchSize:20 of themaybeLoadMore hook) . It's not a stopper for us at the moment but thought I'd let you know about it since it might afford some fine tuning.

[jaredLunde]

Thanks for the follow-up @maierson - I will look into that