Skip to content

Commit 2b0189b

Browse files
mateoguzmanafacebook-github-bot
authored andcommitted
Skip cloning Fragments in ListEmptyComponent to avoid onLayout warning (facebook#50833)
Summary: Fixes facebook#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: facebook#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
1 parent 04348e9 commit 2b0189b

File tree

2 files changed

+114
-11
lines changed

2 files changed

+114
-11
lines changed

packages/virtualized-lists/Lists/VirtualizedList.js

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,30 @@ class VirtualizedList extends StateSafePureComponent<
891891
return key;
892892
}
893893

894+
_renderEmptyComponent(
895+
element: ExactReactElement_DEPRECATED<any>,
896+
inversionStyle: StyleProp<ViewStyle>,
897+
): React.Node {
898+
// $FlowFixMe[prop-missing] React.Element internal inspection
899+
const isFragment = element.type === React.Fragment;
900+
901+
if (isFragment) {
902+
return element;
903+
}
904+
905+
return React.cloneElement(element, {
906+
onLayout: (event: LayoutChangeEvent) => {
907+
this._onLayoutEmpty(event);
908+
// $FlowFixMe[prop-missing] React.Element internal inspection
909+
if (element.props.onLayout) {
910+
element.props.onLayout(event);
911+
}
912+
},
913+
// $FlowFixMe[prop-missing] React.Element internal inspection
914+
style: StyleSheet.compose(inversionStyle, element.props.style),
915+
});
916+
}
917+
894918
render(): React.Node {
895919
this._checkProps(this.props);
896920
const {ListEmptyComponent, ListFooterComponent, ListHeaderComponent} =
@@ -956,17 +980,7 @@ class VirtualizedList extends StateSafePureComponent<
956980
<VirtualizedListCellContextProvider
957981
cellKey={this._getCellKey() + '-empty'}
958982
key="$empty">
959-
{React.cloneElement(element, {
960-
onLayout: (event: LayoutChangeEvent) => {
961-
this._onLayoutEmpty(event);
962-
// $FlowFixMe[prop-missing] React.Element internal inspection
963-
if (element.props.onLayout) {
964-
element.props.onLayout(event);
965-
}
966-
},
967-
// $FlowFixMe[prop-missing] React.Element internal inspection
968-
style: StyleSheet.compose(inversionStyle, element.props.style),
969-
})}
983+
{this._renderEmptyComponent(element, inversionStyle)}
970984
</VirtualizedListCellContextProvider>,
971985
);
972986
}

packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,95 @@ describe('VirtualizedList', () => {
337337
});
338338
});
339339

340+
it('empty component returns the original element if it is a React.Fragment', () => {
341+
const listRef = React.createRef();
342+
const fragment = (
343+
<React.Fragment>
344+
<div>Test</div>
345+
</React.Fragment>
346+
);
347+
348+
act(() => {
349+
create(
350+
<VirtualizedList
351+
ref={listRef}
352+
data={[]}
353+
ListEmptyComponent={fragment}
354+
getItem={(data, index) => data[index]}
355+
getItemCount={data => data.length}
356+
renderItem={({item}) => <item value={item.key} />}
357+
/>,
358+
);
359+
});
360+
361+
const result = listRef.current._renderEmptyComponent(fragment, null);
362+
expect(result).toBe(fragment);
363+
});
364+
365+
it('empty component clones the element and adds onLayout and style props if not a Fragment', () => {
366+
const listRef = React.createRef();
367+
const element = <div>Test</div>;
368+
const inversionStyle = {transform: [{scaleY: -1}]};
369+
370+
act(() => {
371+
create(
372+
<VirtualizedList
373+
ref={listRef}
374+
data={[]}
375+
ListEmptyComponent={element}
376+
getItem={(data, index) => data[index]}
377+
getItemCount={data => data.length}
378+
renderItem={({item}) => <item value={item.key} />}
379+
/>,
380+
);
381+
});
382+
383+
const result = listRef.current._renderEmptyComponent(
384+
element,
385+
inversionStyle,
386+
);
387+
388+
// The result should be a cloned element with additional props
389+
expect(result).not.toBe(element);
390+
expect(result.props).toEqual(
391+
expect.objectContaining({
392+
onLayout: expect.any(Function),
393+
style: inversionStyle,
394+
}),
395+
);
396+
});
397+
398+
it('empty component preserves original onLayout handler if present', () => {
399+
const listRef = React.createRef();
400+
const originalOnLayout = jest.fn();
401+
const element = <div onLayout={originalOnLayout}>Test</div>;
402+
const inversionStyle = {transform: [{scaleY: -1}]};
403+
404+
act(() => {
405+
create(
406+
<VirtualizedList
407+
ref={listRef}
408+
data={[]}
409+
ListEmptyComponent={element}
410+
getItem={(data, index) => data[index]}
411+
getItemCount={data => data.length}
412+
renderItem={({item}) => <item value={item.key} />}
413+
/>,
414+
);
415+
});
416+
417+
const result = listRef.current._renderEmptyComponent(
418+
element,
419+
inversionStyle,
420+
);
421+
422+
// Call the onLayout handler
423+
result.props.onLayout({nativeEvent: {layout: {width: 100, height: 100}}});
424+
425+
// Both the original and the new onLayout should be called
426+
expect(originalOnLayout).toHaveBeenCalled();
427+
});
428+
340429
it('returns the viewableItems correctly in the onViewableItemsChanged callback after changing the data', async () => {
341430
const ITEM_HEIGHT = 800;
342431
let data = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}];

0 commit comments

Comments
 (0)