From 2b0189b9649b58dc93ac79ad2bf709fd7bc8f117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Tue, 22 Apr 2025 07:06:07 -0700 Subject: [PATCH] Skip cloning Fragments in `ListEmptyComponent` to avoid `onLayout` warning (#50833) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Fixes https://github.com/facebook/react-native/issues/50817 Using a fragment is very common when rendering elements. We are cloning and adding `onLayout` always to the ListEmptyComponent element, but this would seem to work only when `View` is used for this as a wrapper in this prop. To prevent this unnecessary warning, I think we can easily check whether it is a fragment or not before cloning and adding the extra props – this adds backwards compatibility for those that don't need to use `onLayout`. ## Changelog: [GENERAL] [FIXED] - Skip cloning Fragments in ListEmptyComponent to avoid onLayout warning Pull Request resolved: https://github.com/facebook/react-native/pull/50833 Test Plan: Use the code snippet from the linked issue to verify that the warning is not thrown anymore when using a Fragment. Reviewed By: javache Differential Revision: D73421503 Pulled By: rshest fbshipit-source-id: 0da4a38130601943e4704589ac275eba39767191 --- .../Lists/VirtualizedList.js | 36 +++++--- .../Lists/__tests__/VirtualizedList-test.js | 89 +++++++++++++++++++ 2 files changed, 114 insertions(+), 11 deletions(-) diff --git a/packages/virtualized-lists/Lists/VirtualizedList.js b/packages/virtualized-lists/Lists/VirtualizedList.js index e5b0ba48600..bd29ced5c6f 100644 --- a/packages/virtualized-lists/Lists/VirtualizedList.js +++ b/packages/virtualized-lists/Lists/VirtualizedList.js @@ -891,6 +891,30 @@ class VirtualizedList extends StateSafePureComponent< return key; } + _renderEmptyComponent( + element: ExactReactElement_DEPRECATED, + inversionStyle: StyleProp, + ): React.Node { + // $FlowFixMe[prop-missing] React.Element internal inspection + const isFragment = element.type === React.Fragment; + + if (isFragment) { + return element; + } + + return React.cloneElement(element, { + onLayout: (event: LayoutChangeEvent) => { + this._onLayoutEmpty(event); + // $FlowFixMe[prop-missing] React.Element internal inspection + if (element.props.onLayout) { + element.props.onLayout(event); + } + }, + // $FlowFixMe[prop-missing] React.Element internal inspection + style: StyleSheet.compose(inversionStyle, element.props.style), + }); + } + render(): React.Node { this._checkProps(this.props); const {ListEmptyComponent, ListFooterComponent, ListHeaderComponent} = @@ -956,17 +980,7 @@ class VirtualizedList extends StateSafePureComponent< - {React.cloneElement(element, { - onLayout: (event: LayoutChangeEvent) => { - this._onLayoutEmpty(event); - // $FlowFixMe[prop-missing] React.Element internal inspection - if (element.props.onLayout) { - element.props.onLayout(event); - } - }, - // $FlowFixMe[prop-missing] React.Element internal inspection - style: StyleSheet.compose(inversionStyle, element.props.style), - })} + {this._renderEmptyComponent(element, inversionStyle)} , ); } diff --git a/packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js b/packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js index 1a4f67dc312..d0dae16b5d0 100644 --- a/packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js +++ b/packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js @@ -337,6 +337,95 @@ describe('VirtualizedList', () => { }); }); + it('empty component returns the original element if it is a React.Fragment', () => { + const listRef = React.createRef(); + const fragment = ( + +
Test
+
+ ); + + act(() => { + create( + data[index]} + getItemCount={data => data.length} + renderItem={({item}) => } + />, + ); + }); + + const result = listRef.current._renderEmptyComponent(fragment, null); + expect(result).toBe(fragment); + }); + + it('empty component clones the element and adds onLayout and style props if not a Fragment', () => { + const listRef = React.createRef(); + const element =
Test
; + const inversionStyle = {transform: [{scaleY: -1}]}; + + act(() => { + create( + data[index]} + getItemCount={data => data.length} + renderItem={({item}) => } + />, + ); + }); + + const result = listRef.current._renderEmptyComponent( + element, + inversionStyle, + ); + + // The result should be a cloned element with additional props + expect(result).not.toBe(element); + expect(result.props).toEqual( + expect.objectContaining({ + onLayout: expect.any(Function), + style: inversionStyle, + }), + ); + }); + + it('empty component preserves original onLayout handler if present', () => { + const listRef = React.createRef(); + const originalOnLayout = jest.fn(); + const element =
Test
; + const inversionStyle = {transform: [{scaleY: -1}]}; + + act(() => { + create( + data[index]} + getItemCount={data => data.length} + renderItem={({item}) => } + />, + ); + }); + + const result = listRef.current._renderEmptyComponent( + element, + inversionStyle, + ); + + // Call the onLayout handler + result.props.onLayout({nativeEvent: {layout: {width: 100, height: 100}}}); + + // Both the original and the new onLayout should be called + expect(originalOnLayout).toHaveBeenCalled(); + }); + it('returns the viewableItems correctly in the onViewableItemsChanged callback after changing the data', async () => { const ITEM_HEIGHT = 800; let data = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}];