Fix AutoHeight on last list item

[admirsaheta]

Description

Fixes (issue #)
#1868

This PR resolves the issue where ItemSeparatorComponent in grid layouts ( numColumns > 1 ) causes all items in a row to stretch to match the height of the tallest item, including separator height. The fix ensures that separators are positioned outside the main item container to prevent interference with grid layout calculations.

Changes Made:

1. ViewHolder Component Refactoring ( src/recyclerview/ViewHolder.tsx ):

• Separator Positioning Fix : Modified separator rendering to position it outside the main item container using absolute positioning
• Code Organization : Extracted style creation into dedicated utility functions ( createContainerStyle , createSeparatorStyle )
• Type Safety Improvements : Enhanced TypeScript types with proper type imports and better type annotations
• Code Quality : Improved variable naming, added clear comments, and organized code into logical sections
  Key Technical Changes:

// Before: Separator rendered inside same container as item content
<CompatContainer>
  {children}
  {separator}
</CompatContainer>

// After: Separator positioned independently to avoid layout interference
<>
  <ContainerComponent>
    {itemContent}
  </ContainerComponent>
  {separatorElement && (
    <CompatView style={separatorStyle}>
      {separatorElement}
    </CompatView>
  )}
</>

Reviewers’ hat-rack 🎩

Focus Areas for Review:

• Verify that separators are correctly positioned in both horizontal and vertical grid layouts

• Test with various ItemSeparatorComponent implementations to ensure no visual regressions

• Confirm that the refactored code maintains all existing functionality

• Check TypeScript compilation and type safety improvements

• Validate that grid items no longer stretch unnecessarily due to separator height
  Testing Scenarios:

• Grid layout with numColumns={2} and custom separator components

• Items with varying content heights in the same row

• Horizontal and vertical list orientations

• Different separator designs (tall, short, with margins)

Screenshots or videos (if needed)

Before Fix:

• Grid items in the same row would all stretch to match the height of the tallest item + separator height

• Items without separators (last in row) would have unnecessary empty space
  After Fix:

• Each grid item maintains its natural content height

• Separators are positioned independently without affecting item dimensions

• Clean, consistent grid layout regardless of separator presence

Test configuration:

<FlashList
  data={items}
  numColumns={2}
  ItemSeparatorComponent={CustomSeparator}
  renderItem={({ item }) => <ItemComponent item={item} />}
/>

The fix ensures that ItemSeparatorComponent works seamlessly with grid layouts while maintaining the expected visual separation between items.

[naqvitalha]

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?

[admirsaheta]

@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.

[naqvitalha]

@admirsaheta Really appreciate the PR, can you find a way to modify the separator status without scanning all the items? We also need to ensure that we don't end up re-rendering too many items on pagination. If you can solve for these constraints, we can merge it. Feel free to ask if you have any questions or you want to bounce ideas.

[admirsaheta]

Hey @naqvitalha — thanks again for the feedback, really appreciate the input 🙌

I think this is the right direction to take, but of course if you have a better approach in mind I’d love to hear it.

I’ve replaced the expensive updateAllWidths() method (which previously iterated through all items) with a lazy evaluation approach. Now, width calculations are deferred until they’re actually needed during layout computation. The updated updateAllWidths() no longer performs immediate calculations — it simply marks layouts as needing recalculation. This reduces complexity from O(n) to O(1) for width updates.

I also enhanced estimateLayout() and recomputeLayouts() so that widths are computed only when necessary, ensuring minimal overhead. This shifts complexity from always being O(n) when layout parameters change to O(k), where k is the number of items actually being laid out. If you check the new performance tests, you’ll see significant improvements for large lists, especially with pagination and dynamic updates.

On the micro-optimization side: I experimented with caching this.layout.length. This eliminates repeated property access in tight loops, which gives a small but measurable performance gain and reduces variability in execution time. It’s especially beneficial in frequently called methods like processAndReturnTallestItemInRow and recomputeLayouts.

Since the layouts array length remains constant during an individual layout calculation, this cache doesn’t introduce stability risks. The cache is also scoped to single method calls, never across operations where the array might change — so it stays safe while delivering the performance benefits in those critical hot paths.

[naqvitalha]

@admirsaheta Apologies for the delay! I've been busy but I'll get to this later this week.

[admirsaheta]

@naqvitalha can we take a look at this once more please, tysm 👍