Skip to content

Commit e44dbc4

Browse files
authored
Add warning when keyExtractor is not defined with maintainVisibleContentPosition (#1862)
* Add warning when keyExtractor is not defined with maintainVisibleContentPosition When using maintainVisibleContentPosition (through onStartReached), keyExtractor is essential for proper item tracking. This warning helps developers identify potential issues early. * Add tests for keyExtractor warning with maintainVisibleContentPosition Tests verify that the warning is shown when onStartReached is used without keyExtractor, and that no warnings appear in other scenarios. * Fix test ts errors
1 parent d8747bb commit e44dbc4

File tree

3 files changed

+84
-0
lines changed

3 files changed

+84
-0
lines changed
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { RecyclerViewManager } from "../recyclerview/RecyclerViewManager";
2+
import { WarningMessages } from "../errors/WarningMessages";
3+
import { FlashListProps } from "../FlashListProps";
4+
5+
describe("RecyclerViewManager", () => {
6+
let consoleWarnSpy: jest.SpyInstance;
7+
8+
beforeEach(() => {
9+
consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation();
10+
});
11+
12+
afterEach(() => {
13+
consoleWarnSpy.mockRestore();
14+
});
15+
16+
describe("keyExtractor warning with maintainVisibleContentPosition", () => {
17+
const createMockProps = (overrides = {}) =>
18+
({
19+
data: [{ id: 1 }, { id: 2 }, { id: 3 }],
20+
renderItem: jest.fn(),
21+
...overrides,
22+
} as FlashListProps<unknown>);
23+
24+
const createManager = (props: FlashListProps<unknown>) => {
25+
return new RecyclerViewManager(props);
26+
};
27+
28+
it("should warn when onStartReached is defined but keyExtractor is not", () => {
29+
const props = createMockProps({
30+
onStartReached: jest.fn(),
31+
keyExtractor: undefined,
32+
});
33+
34+
createManager(props);
35+
36+
expect(consoleWarnSpy).toHaveBeenCalledWith(
37+
WarningMessages.keyExtractorNotDefinedForMVCP
38+
);
39+
});
40+
41+
it("should not warn when both onStartReached and keyExtractor are defined", () => {
42+
const props = createMockProps({
43+
onStartReached: jest.fn(),
44+
keyExtractor: (item: any) => item.id.toString(),
45+
});
46+
47+
createManager(props);
48+
49+
expect(consoleWarnSpy).not.toHaveBeenCalled();
50+
});
51+
52+
it("should not warn when onStartReached is not defined", () => {
53+
const props = createMockProps({
54+
onStartReached: undefined,
55+
keyExtractor: undefined,
56+
});
57+
58+
createManager(props);
59+
60+
expect(consoleWarnSpy).not.toHaveBeenCalled();
61+
});
62+
63+
it("should not warn when onStartReached is not defined but keyExtractor is", () => {
64+
const props = createMockProps({
65+
onStartReached: undefined,
66+
keyExtractor: (item: any) => item.id.toString(),
67+
});
68+
69+
createManager(props);
70+
71+
expect(consoleWarnSpy).not.toHaveBeenCalled();
72+
});
73+
});
74+
});

src/errors/WarningMessages.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
export const WarningMessages = {
22
keyExtractorNotDefinedForAnimation:
33
"keyExtractor is not defined. This might cause the animations to not work as expected.",
4+
keyExtractorNotDefinedForMVCP:
5+
"keyExtractor is not defined, maintainVisibleContentPosition may not work as expected.",
46
exceededMaxRendersWithoutCommit:
57
"Exceeded max renders without commit. This might mean that you have duplicate keys in your keyExtractor output or your list is nested in a ScrollView causing a lot of items to render at once. " +
68
"If it's none of those and is causing a real issue or error, consider reporing this on FlashList Github",

src/recyclerview/RecyclerViewManager.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ErrorMessages } from "../errors/ErrorMessages";
2+
import { WarningMessages } from "../errors/WarningMessages";
23

34
import ViewabilityManager from "./viewability/ViewabilityManager";
45
import { ConsecutiveNumbers } from "./helpers/ConsecutiveNumbers";
@@ -69,6 +70,7 @@ export class RecyclerViewManager<T> {
6970
props.maxItemsInRecyclePool
7071
);
7172
this.itemViewabilityManager = new ViewabilityManager<T>(this as any);
73+
this.checkPropsAndWarn();
7274
}
7375

7476
// updates render stack based on the engaged indices which are sorted. Recycles unused keys.
@@ -446,4 +448,10 @@ export class RecyclerViewManager<T> {
446448
this.propsRef.extraData
447449
);
448450
}
451+
452+
private checkPropsAndWarn() {
453+
if (this.propsRef.onStartReached && !this.propsRef.keyExtractor) {
454+
console.warn(WarningMessages.keyExtractorNotDefinedForMVCP);
455+
}
456+
}
449457
}

0 commit comments

Comments
 (0)