Skip to content
Open
110 changes: 70 additions & 40 deletions src/recyclerview/ViewHolder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,17 @@
* The component is memoized to prevent unnecessary re-renders and includes layout comparison logic.
*/

import { LayoutChangeEvent } from "react-native";
import type { LayoutChangeEvent, ViewStyle } from "react-native";
import React, {
RefObject,
type RefObject,
useCallback,
useLayoutEffect,
useMemo,
useRef,
} from "react";

import { FlashListProps, RenderTarget } from "../FlashListProps";

import { RVDimension, RVLayout } from "./layout-managers/LayoutManager";
import type { FlashListProps, RenderTarget } from "../FlashListProps";
import type { RVDimension, RVLayout } from "./layout-managers/LayoutManager";
import { CompatView } from "./components/CompatView";

/**
Expand Down Expand Up @@ -49,12 +48,43 @@ export interface ViewHolderProps<TItem> {
onSizeChanged?: (index: number, size: RVDimension) => void;
}

/**
* Creates the main container style for the ViewHolder
*/
const createContainerStyle = (
layout: RVLayout,
horizontal: boolean = false,
target: RenderTarget
): ViewStyle => ({
flexDirection: horizontal ? "row" : "column",
position: target === "StickyHeader" ? "relative" : "absolute",
width: layout.enforcedWidth ? layout.width : undefined,
height: layout.enforcedHeight ? layout.height : undefined,
minHeight: layout.minHeight,
minWidth: layout.minWidth,
maxHeight: layout.maxHeight,
maxWidth: layout.maxWidth,
left: layout.x,
top: layout.y,
});

/**
* Creates the separator positioning style
*/
const createSeparatorStyle = (
layout: RVLayout,
horizontal: boolean = false
): ViewStyle => ({
position: "absolute",
left: horizontal ? layout.x + layout.width : layout.x,
top: horizontal ? layout.y : layout.y + layout.height,
});

/**
* Internal ViewHolder component that handles the actual rendering of list items
* @template TItem - The type of item being rendered in the list
*/
const ViewHolderInternal = <TItem,>(props: ViewHolderProps<TItem>) => {
// create ref for View
const viewRef = useRef<CompatView>(null);
const {
index,
Expand All @@ -68,9 +98,10 @@ const ViewHolderInternal = <TItem,>(props: ViewHolderProps<TItem>) => {
CellRendererComponent,
ItemSeparatorComponent,
trailingItem,
horizontal,
horizontal = false,
} = props;

// Manage ref lifecycle
useLayoutEffect(() => {
refHolder.set(index, viewRef);
return () => {
Expand All @@ -80,55 +111,54 @@ const ViewHolderInternal = <TItem,>(props: ViewHolderProps<TItem>) => {
};
}, [index, refHolder]);

const onLayout = useCallback(
// Handle layout changes
const handleLayout = useCallback(
(event: LayoutChangeEvent) => {
onSizeChanged?.(index, event.nativeEvent.layout);
},
[index, onSizeChanged]
);

const separator = useMemo(() => {
return ItemSeparatorComponent && trailingItem !== undefined ? (
// Render separator component if needed
const separatorElement = useMemo(() => {
if (!ItemSeparatorComponent || trailingItem === undefined) {
return null;
}
return (
<ItemSeparatorComponent leadingItem={item} trailingItem={trailingItem} />
) : null;
);
}, [ItemSeparatorComponent, item, trailingItem]);

// console.log("ViewHolder re-render", index);

const children = useMemo(() => {
// Render main item content
const itemContent = useMemo(() => {
return renderItem?.({ item, index, extraData, target }) ?? null;
// TODO: Test more thoroughly
// We don't really to re-render the children when the index changes
// Note: We intentionally exclude index from dependencies to avoid unnecessary re-renders
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [item, extraData, target, renderItem]);

const style = {
flexDirection: horizontal ? "row" : "column",
position: target === "StickyHeader" ? "relative" : "absolute",
width: layout.enforcedWidth ? layout.width : undefined,
height: layout.enforcedHeight ? layout.height : undefined,
minHeight: layout.minHeight,
minWidth: layout.minWidth,
maxHeight: layout.maxHeight,
maxWidth: layout.maxWidth,
left: layout.x,
top: layout.y,
} as const;
// Determine container component
const ContainerComponent = (CellRendererComponent ?? CompatView) as React.ComponentType<any>;

// TODO: Fix this type issue
const CompatContainer = (CellRendererComponent ??
CompatView) as unknown as any;
// Create styles
const containerStyle = createContainerStyle(layout, horizontal ?? false, target);
const separatorStyle = separatorElement ? createSeparatorStyle(layout, horizontal ?? false) : undefined;

return (
<CompatContainer
ref={viewRef}
onLayout={onLayout}
style={style}
index={index}
>
{children}
{separator}
</CompatContainer>
<>
<ContainerComponent
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the separator out means that we won't include its size in measurements which can break the entire layout. I think the right solution is to skip separators on the last row.

It's a little challenging because we don't want the view holders to know which layout manager is being used. Layout manager is the only class that really knows whether there should be a separator or not. Can you explore this approach?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naqvitalha You’re right, moving separators out of the layout would have messed up measurements and created inconsistencies. Instead, I went with a simpler approach: just skip separators on the last row. This keeps the layout clean without breaking separation of concerns.

How it works

  • Layout Manager smarts
    The GridLayoutManager.ts now figures out which items should skip separators:
// Inside recomputeLayouts
if (endIndex === this.layouts.length - 1) {
  this.processAndReturnTallestItemInRow(endIndex);
  this.layouts.forEach(l => (l.skipSeparator = false));
  this.markLastRowItemsToSkipSeparators(endIndex);
}
  • ViewHolder stays dumb (on purpose)
    ViewHolder.tsx doesn’t need to know anything about grid logic — it just respects skipSeparator:
const shouldRenderSeparator =
  !layout.skipSeparator &&
  trailingItem !== undefined &&
  ItemSeparatorComponent !== undefined;
  • Dynamic updates handled
    When items get added/removed, all flags are reset and the new last row is re-marked.
    → This keeps separator behavior consistent no matter how the grid changes.

Tests

  • Grid layouts (2x2, 3x3, uneven rows)
  • Dynamic item add/remove
  • Edge cases (single item, empty grid)
  • ViewHolder rendering logic

This keeps separators under control without leaking layout logic into the ViewHolder, and it works smoothly even as items change dynamically.

ref={viewRef}
onLayout={handleLayout}
style={containerStyle}
index={index}
>
{itemContent}
</ContainerComponent>
{separatorElement && (
<CompatView style={separatorStyle}>
{separatorElement}
</CompatView>
)}
</>
);
};

Expand Down