Skip to content

Commit 3aa4894

Browse files
LodinLodin
authored
Merge pull request #44 from Lodin/feat/id-as-key
Use node ID as react element key
2 parents b52d315 + 4cd30e0 commit 3aa4894

7 files changed

+180
-71
lines changed

‎README.md

+5-1
Original file line numberDiff line numberDiff line change
@@ -647,4 +647,8 @@ const Node = ({data: {isLeaf, name}, isOpen, style, setOpen}) => (
647647
<div>{name}</div>
648648
</div>
649649
);
650-
```
650+
```
651+
652+
### 5. Migrate all your IDs to string
653+
654+
Using node IDs as keys should improve React rendering performance. However, it means that you won't be able to use `Symbol` as IDs anymore. You should move all your IDs to be strings instead of symbols.

‎__tests__/FixedSizeTree.spec.tsx

+37-9
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,21 @@ describe('FixedSizeTree', () => {
272272
).toEqual(['foo-1', 'foo-2', 'foo-5', 'foo-6', 'foo-7']);
273273
});
274274

275+
it('provides a itemKey function to FixedSizeList', () => {
276+
const itemKey = component.find(FixedSizeList).prop('itemKey');
277+
expect(itemKey).not.toBeUndefined();
278+
279+
expect(component.find(Row).map((node) => node.key())).toEqual([
280+
'foo-1',
281+
'foo-2',
282+
'foo-3',
283+
'foo-4',
284+
'foo-5',
285+
'foo-6',
286+
'foo-7',
287+
]);
288+
});
289+
275290
describe('placeholder', () => {
276291
const testRICTimeout = 16;
277292
let unmockRIC: () => void;
@@ -486,23 +501,34 @@ describe('FixedSizeTree', () => {
486501
},
487502
},
488503
'foo-5': true,
504+
// This action means nothing because "foo-6" is leaf and has no
505+
// children. But it sill will set `isOpen` to true.
489506
'foo-6': true,
490507
});
508+
component.update(); // Update the wrapper to get the latest changes
491509

492510
const receivedRecords = extractReceivedRecords(
493511
component.find(FixedSizeList),
494512
);
513+
expect(
514+
receivedRecords.reduce<Record<string, boolean>>(
515+
(acc, {data: {id}, isOpen}) => {
516+
acc[id] = isOpen;
495517

496-
expect(receivedRecords.map(({isOpen}) => isOpen)).toEqual([
497-
true,
498-
false,
499-
false,
500-
false,
518+
return acc;
519+
},
520+
{},
521+
),
522+
).toEqual({
523+
'foo-1': true,
524+
// Since `foo-2` is closed, `foo-3` and `foo-4` do not even appear in
525+
// the list of received records.
526+
'foo-2': false,
501527
// The `foo-5` and `foo-6` nodes are opened manually in recomputeTree
502-
true,
503-
true,
504-
false,
505-
]);
528+
'foo-5': true,
529+
'foo-6': true,
530+
'foo-7': false,
531+
});
506532
});
507533

508534
it('does nothing if opennessState is not an object', async () => {
@@ -512,6 +538,7 @@ describe('FixedSizeTree', () => {
512538

513539
// @ts-expect-error: Test for non-typescript code.
514540
await treeInstance.recomputeTree('4');
541+
component.update(); // Update the wrapper to get the latest changes
515542

516543
expect(extractReceivedRecords(component.find(FixedSizeList))).toEqual(
517544
originalRecords,
@@ -526,6 +553,7 @@ describe('FixedSizeTree', () => {
526553
await treeInstance.recomputeTree({
527554
'foo-42': false,
528555
});
556+
component.update(); // Update the wrapper to get the latest changes
529557

530558
expect(extractReceivedRecords(component.find(FixedSizeList))).toEqual(
531559
originalRecords,

‎__tests__/VariableSizeTree.spec.tsx

+37-9
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,21 @@ describe('VariableSizeTree', () => {
293293
).toEqual(['foo-1', 'foo-2', 'foo-5', 'foo-6', 'foo-7']);
294294
});
295295

296+
it('provides a itemKey function to VariableSizeList', () => {
297+
const itemKey = component.find(VariableSizeList).prop('itemKey');
298+
expect(itemKey).not.toBeUndefined();
299+
300+
expect(component.find(Row).map((node) => node.key())).toEqual([
301+
'foo-1',
302+
'foo-2',
303+
'foo-3',
304+
'foo-4',
305+
'foo-5',
306+
'foo-6',
307+
'foo-7',
308+
]);
309+
});
310+
296311
describe('placeholder', () => {
297312
const testRICTimeout = 16;
298313
let unmockRIC: () => void;
@@ -515,23 +530,34 @@ describe('VariableSizeTree', () => {
515530
},
516531
},
517532
'foo-5': true,
533+
// This action means nothing because "foo-6" is leaf and has no
534+
// children. But it sill will set `isOpen` to true.
518535
'foo-6': true,
519536
});
537+
component.update(); // Update the wrapper to get the latest changes
520538

521539
const receivedRecords = extractReceivedRecords(
522540
component.find(VariableSizeList),
523541
);
542+
expect(
543+
receivedRecords.reduce<Record<string, boolean>>(
544+
(acc, {data: {id}, isOpen}) => {
545+
acc[id] = isOpen;
524546

525-
expect(receivedRecords.map(({isOpen}) => isOpen)).toEqual([
526-
true,
527-
false,
528-
false,
529-
false,
547+
return acc;
548+
},
549+
{},
550+
),
551+
).toEqual({
552+
'foo-1': true,
553+
// Since `foo-2` is closed, `foo-3` and `foo-4` do not even appear in
554+
// the list of received records.
555+
'foo-2': false,
530556
// The `foo-5` and `foo-6` nodes are opened manually in recomputeTree
531-
true,
532-
true,
533-
false,
534-
]);
557+
'foo-5': true,
558+
'foo-6': true,
559+
'foo-7': false,
560+
});
535561
});
536562

537563
it('does nothing if opennessState is not an object', async () => {
@@ -541,6 +567,7 @@ describe('VariableSizeTree', () => {
541567

542568
// @ts-expect-error: Test for non-typescript code.
543569
await treeInstance.recomputeTree('4');
570+
component.update(); // Update the wrapper to get the latest changes
544571

545572
expect(
546573
extractReceivedRecords(component.find(VariableSizeList)),
@@ -555,6 +582,7 @@ describe('VariableSizeTree', () => {
555582
await treeInstance.recomputeTree({
556583
'foo-42': false,
557584
});
585+
component.update(); // Update the wrapper to get the latest changes
558586

559587
expect(
560588
extractReceivedRecords(component.find(VariableSizeList)),

‎src/FixedSizeTree.tsx

+6-7
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import Tree, {
77
TreeState,
88
NodePublicState,
99
} from './Tree';
10-
import {createBasicRecord} from './utils';
10+
import {createBasicRecord, getIdByIndex} from './utils';
1111

1212
export type FixedSizeNodeData = NodeData;
1313

@@ -32,8 +32,8 @@ const computeTree = createTreeComputer<
3232
FixedSizeTreeProps<FixedSizeNodeData>,
3333
FixedSizeTreeState<FixedSizeNodeData>
3434
>({
35-
createRecord(data, {recomputeTree}, parent, previousRecord) {
36-
const record = createBasicRecord(
35+
createRecord: (data, {recomputeTree}, parent, previousRecord) =>
36+
createBasicRecord(
3737
{
3838
data,
3939
isOpen: previousRecord
@@ -45,10 +45,7 @@ const computeTree = createTreeComputer<
4545
}),
4646
},
4747
parent,
48-
);
49-
50-
return record;
51-
},
48+
),
5249
});
5350

5451
export class FixedSizeTree<
@@ -88,6 +85,8 @@ export class FixedSizeTree<
8885
itemCount={order!.length}
8986
itemData={this.getItemData()}
9087
ref={this.list}
88+
// eslint-disable-next-line @typescript-eslint/unbound-method
89+
itemKey={getIdByIndex}
9190
>
9291
{rowComponent!}
9392
</FixedSizeList>

0 commit comments

Comments
 (0)