Skip to content

Commit 4ca24e3

Browse files
authored
Improve horizontal layout scroll performance by removing React sync flushing (#286) and release version major
1 parent b2db331 commit 4ca24e3

9 files changed

Lines changed: 171 additions & 119 deletions

File tree

examples/perf-baselines.ci.json

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
"totalTime": 777.05
1313
},
1414
"tests/table/perf/flashing/default:should be able to collapse and expand nodes with no error": {
15-
"scriptingTime": 433.34,
16-
"renderingTime": 161.39,
17-
"paintingTime": 7.77,
18-
"totalTime": 681.6
15+
"scriptingTime": 202.25,
16+
"renderingTime": 68.28,
17+
"paintingTime": 6.67,
18+
"totalTime": 456.38
1919
},
2020
"tests/table/props/column/column-change:works correctly": {
2121
"scriptingTime": 77.4,
@@ -36,16 +36,16 @@
3636
"totalTime": 407.06
3737
},
3838
"tests/table/props/data/basic-update:via API should not rerender header": {
39-
"scriptingTime": 22.74,
40-
"renderingTime": 2.54,
41-
"paintingTime": 2.12,
42-
"totalTime": 282.04
39+
"scriptingTime": 10.79,
40+
"renderingTime": 1.51,
41+
"paintingTime": 0.22,
42+
"totalTime": 166.63
4343
},
4444
"tests/table/props/data/editing/column-editor:should use custom editor when configured": {
45-
"scriptingTime": 181.66,
46-
"renderingTime": 33.02,
47-
"paintingTime": 6.63,
48-
"totalTime": 692.85
45+
"scriptingTime": 108.94,
46+
"renderingTime": 22.22,
47+
"paintingTime": 3,
48+
"totalTime": 584.08
4949
},
5050
"tests/table/props/data/editing/persistEdit:should not persist changes to the id column": {
5151
"scriptingTime": 88.33,
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import {
2+
InfiniteTable,
3+
DataSource,
4+
type InfiniteTablePropColumns,
5+
} from '@infinite-table/infinite-react';
6+
7+
import * as React from 'react';
8+
import { useState } from 'react';
9+
import { Developer, dataSource } from './horiz-layout-data';
10+
11+
const columns: InfiniteTablePropColumns<Developer> = {
12+
id: {
13+
field: 'id',
14+
type: 'number',
15+
/*xdefaultWidth: 80,*/ renderValue: ({ value }) => value - 1,
16+
style: (_options) => {
17+
return {
18+
// background : options.rowInfo.
19+
};
20+
},
21+
},
22+
preferredLanguage: { field: 'preferredLanguage' /*xdefaultWidth: 110 */ },
23+
// age: { field: 'age' /*xdefaultWidth: 70 */ },
24+
// salary: {
25+
// field: 'salary',
26+
// type: 'number',
27+
// /*xdefaultWidth: 100,*/
28+
// },
29+
};
30+
31+
export function getRandomInt(min: number, max: number) {
32+
return Math.floor(Math.random() * (max - min + 1) + min);
33+
}
34+
35+
const domProps = { style: { height: '30vh', width: '90vw' } };
36+
// const domProps = { style: { height: '30vh', width: 300 } };
37+
38+
// dataSource.length = 12;
39+
40+
export default function App() {
41+
const [wrapRowsHorizontally, setWrapRowsHorizontally] = useState(true);
42+
return (
43+
<>
44+
<button
45+
onClick={() => {
46+
setWrapRowsHorizontally(!wrapRowsHorizontally);
47+
}}
48+
>
49+
toggle
50+
</button>
51+
<DataSource<Developer>
52+
primaryKey="id"
53+
data={dataSource}
54+
key={`${wrapRowsHorizontally}`}
55+
>
56+
<InfiniteTable<Developer>
57+
wrapRowsHorizontally={wrapRowsHorizontally}
58+
rowHeight={50}
59+
domProps={domProps}
60+
columns={columns}
61+
columnDefaultWidth={150}
62+
onCellClick={({ rowIndex, colIndex }) => {
63+
console.log('clicked', rowIndex, colIndex);
64+
}}
65+
/>
66+
</DataSource>
67+
</>
68+
);
69+
}

examples/src/pages/tests/horizontal-layout/test.page.tsx

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,31 @@ const columns: InfiniteTablePropColumns<Developer> = {
2828
field: 'id',
2929
type: 'number',
3030
defaultWidth: 100,
31+
style: {
32+
background: 'rgba(255, 99, 71, 0.4)',
33+
},
3134
},
3235
canDesign: {
3336
field: 'canDesign',
3437
defaultWidth: 200,
38+
style: {
39+
background: 'rgba(211, 119, 171, 0.3)',
40+
},
3541
},
3642
salary: {
3743
field: 'salary',
3844
type: 'number',
3945
defaultWidth: 300,
46+
style: {
47+
background: 'rgba(55, 99, 171, 0.5)',
48+
},
49+
},
50+
firstName: {
51+
field: 'firstName',
52+
style: {
53+
background: 'rgb(111 255 72 / 48%)',
54+
},
4055
},
41-
// firstName: {
42-
// field: 'firstName',
43-
// },
4456
// age: {
4557
// field: 'age',
4658
// type: 'number',
@@ -53,10 +65,10 @@ const columns: InfiniteTablePropColumns<Developer> = {
5365

5466
const domProps = {
5567
// style: { height: 420 /*30px header, 420 body*/, width: 230 },
56-
style: { height: '50vh' /*30px header, 420 body*/, width: '80vw' },
68+
style: { height: '50vh' /*30px header, 420 body*/, width: '100vw' },
5769
};
5870

59-
const data = Array.from({ length: 100 }, (_, i) => ({
71+
const data = Array.from({ length: 1000 }, (_, i) => ({
6072
id: i,
6173
preferredLanguage: `Lang ${i}`,
6274
age: i * 10,

examples/src/pages/tests/table/utils/GridCellManager.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const createCell = (node?: string): GridCellInterface<any> => {
1010
update: (newNode: string) => {
1111
node = newNode;
1212
},
13+
setPendingAfterCommitWork: () => {},
1314
getElement: () => null,
1415
getNode: () => node,
1516
destroy: () => {},

source/src/components/HeadlessTable/GridCellInterface.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,7 @@ import { Renderable } from '../types/Renderable';
22

33
export interface GridCellInterface<T_ADDITIONAL_CELL_INFO = any> {
44
debugId: string;
5-
update(
6-
content: Renderable,
7-
additionalInfo?: T_ADDITIONAL_CELL_INFO,
8-
scrollingObjectParam?: {
9-
scrolling: boolean;
10-
},
11-
): void;
5+
update(content: Renderable, additionalInfo?: T_ADDITIONAL_CELL_INFO): void;
126
getElement(): HTMLElement | null;
137
getNode(): Renderable;
148
destroy(): void;
@@ -18,4 +12,5 @@ export interface GridCellInterface<T_ADDITIONAL_CELL_INFO = any> {
1812
getAdditionalInfo(): T_ADDITIONAL_CELL_INFO | undefined;
1913
isMounted(): boolean;
2014
ref: React.RefCallback<HTMLElement | undefined>;
15+
setPendingAfterCommitWork(fn: (() => void) | null): void;
2116
}

source/src/components/HeadlessTable/GridCellManager.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,6 @@ export class GridCellManager<T_ADDITIONAL_CELL_INFO> extends Logger {
185185
cell: GridCellInterface<T_ADDITIONAL_CELL_INFO>,
186186
cellPos: CellPos,
187187
additionalInfo?: T_ADDITIONAL_CELL_INFO,
188-
scrollingObjectParam?: {
189-
scrolling: boolean;
190-
isHorizontalLayout: boolean;
191-
},
192188
) {
193189
const currentCellAtPos = this.getCellAt(cellPos);
194190

@@ -203,7 +199,7 @@ export class GridCellManager<T_ADDITIONAL_CELL_INFO> extends Logger {
203199

204200
this.setCellPositionInMatrix(cell, cellPos);
205201

206-
cell.update(node, additionalInfo, scrollingObjectParam);
202+
cell.update(node, additionalInfo);
207203

208204
return cell;
209205
}

source/src/components/HeadlessTable/GridCellPoolForReact.tsx

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class GridCellForReact<T_ADDITIONAL_CELL_INFO = any>
2626

2727
private mounted: boolean = false;
2828

29-
private IS_UPDATED_WHILE_SCROLLING_IN_HORIZONTAL_LAYOUT = false;
29+
private pendingAfterCommitWork: (() => void) | null = null;
3030

3131
private mountSubscription =
3232
buildSubscriptionCallback<GridCellInterface<T_ADDITIONAL_CELL_INFO>>();
@@ -46,7 +46,7 @@ class GridCellForReact<T_ADDITIONAL_CELL_INFO = any>
4646
key={key}
4747
name={key}
4848
updater={this.updater}
49-
shouldFlushSync={this.isUpdatedWhileScrolling}
49+
afterCommit={this.afterCommit}
5050
/>
5151
);
5252

@@ -61,8 +61,16 @@ class GridCellForReact<T_ADDITIONAL_CELL_INFO = any>
6161
};
6262
}
6363

64-
isUpdatedWhileScrolling = () => {
65-
return this.IS_UPDATED_WHILE_SCROLLING_IN_HORIZONTAL_LAYOUT;
64+
setPendingAfterCommitWork(fn: (() => void) | null) {
65+
this.pendingAfterCommitWork = fn;
66+
}
67+
68+
private afterCommit = () => {
69+
if (this.pendingAfterCommitWork) {
70+
const work = this.pendingAfterCommitWork;
71+
this.pendingAfterCommitWork = null;
72+
work();
73+
}
6674
};
6775

6876
isMounted() {
@@ -83,6 +91,7 @@ class GridCellForReact<T_ADDITIONAL_CELL_INFO = any>
8391
this.mountSubscription.destroy();
8492
this.updater.destroy();
8593

94+
this.pendingAfterCommitWork = null;
8695
this.ref = emptyFn;
8796
this.element = null;
8897
this.node = null;
@@ -92,18 +101,7 @@ class GridCellForReact<T_ADDITIONAL_CELL_INFO = any>
92101
return this.node;
93102
}
94103

95-
update(
96-
content: Renderable,
97-
additionalInfo?: T_ADDITIONAL_CELL_INFO,
98-
scrollingObjectParam?: {
99-
scrolling: boolean;
100-
isHorizontalLayout: boolean;
101-
},
102-
): void {
103-
this.IS_UPDATED_WHILE_SCROLLING_IN_HORIZONTAL_LAYOUT = scrollingObjectParam
104-
? scrollingObjectParam.scrolling &&
105-
scrollingObjectParam.isHorizontalLayout
106-
: false;
104+
update(content: Renderable, additionalInfo?: T_ADDITIONAL_CELL_INFO): void {
107105
this.updater(content);
108106
this.cellInfo = additionalInfo;
109107
}

source/src/components/HeadlessTable/ReactHeadlessTableRenderer.tsx

Lines changed: 43 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,6 @@ export const columnOffsetAtIndexWhileReordering = stripVar(
4242
InternalVars.columnOffsetAtIndexWhileReordering,
4343
);
4444

45-
const SCROLLING_OBJECT_PARAM_FLYWEIGHT = {
46-
scrolling: false,
47-
isHorizontalLayout: false,
48-
};
49-
5045
export class GridRenderer extends Logger {
5146
protected brain: MatrixBrain;
5247

@@ -1381,15 +1376,11 @@ export class GridRenderer extends Logger {
13811376

13821377
return;
13831378
}
1384-
SCROLLING_OBJECT_PARAM_FLYWEIGHT.scrolling = this.scrolling;
1385-
SCROLLING_OBJECT_PARAM_FLYWEIGHT.isHorizontalLayout = isHorizontalLayout;
1386-
13871379
this.cellManager.renderNodeAtCell(
13881380
renderedNode,
13891381
cell,
13901382
[rowIndex, colIndex],
13911383
cellAdditionalInfo,
1392-
SCROLLING_OBJECT_PARAM_FLYWEIGHT,
13931384
);
13941385

13951386
this.updateElementPosition(cell, { hidden, rowspan, colspan });
@@ -1501,45 +1492,53 @@ export class GridRenderer extends Logger {
15011492
const { x, y } = itemPosition;
15021493

15031494
if (itemElement) {
1504-
this.updateHoverClassNamesForRow(rowIndex);
1505-
1506-
// itemElement.style.gridColumn = `${colIndex} / span 1`;
1507-
// itemElement.style.gridRow = `${rowIndex} / span 1`;
1508-
1509-
// (itemElement.dataset as any).elementIndex = elementIndex;
1510-
const realCoords = this.getCellRealCoordinates(rowIndex, colIndex);
1511-
(itemElement.dataset as any).rowIndex = realCoords.rowIndex;
1495+
const applyPosition = () => {
1496+
this.updateHoverClassNamesForRow(rowIndex);
1497+
// itemElement.style.gridColumn = `${colIndex} / span 1`;
1498+
// itemElement.style.gridRow = `${rowIndex} / span 1`;
1499+
1500+
// (itemElement.dataset as any).elementIndex = elementIndex;
1501+
const realCoords = this.getCellRealCoordinates(rowIndex, colIndex);
1502+
(itemElement.dataset as any).rowIndex = realCoords.rowIndex;
1503+
1504+
(itemElement.dataset as any).colIndex = realCoords.colIndex;
1505+
1506+
if (ITEM_POSITION_WITH_TRANSFORM) {
1507+
this.setTransform(itemElement, rowIndex, colIndex, { x, y }, null);
1508+
1509+
itemElement.style.willChange = 'transform';
1510+
itemElement.style.backfaceVisibility = 'hidden';
1511+
// need to set it to auto
1512+
// in case some fixed cells are reused
1513+
// as the fixed cells had a zIndex
1514+
const hidden = options
1515+
? options.hidden
1516+
: !!this.isCellCovered(rowIndex, colIndex);
1517+
1518+
const zIndex = hidden
1519+
? '-1'
1520+
: // #updatezindex - we need to allow elements use their own zIndex, so we
1521+
// resort to allowing them to have it as a data-z-index attribute
1522+
itemElement.dataset.zIndex || 'auto';
15121523

1513-
(itemElement.dataset as any).colIndex = realCoords.colIndex;
1514-
1515-
if (ITEM_POSITION_WITH_TRANSFORM) {
1516-
this.setTransform(itemElement, rowIndex, colIndex, { x, y }, null);
1517-
1518-
itemElement.style.willChange = 'transform';
1519-
itemElement.style.backfaceVisibility = 'hidden';
1520-
// need to set it to auto
1521-
// in case some fixed cells are reused
1522-
// as the fixed cells had a zIndex
1523-
const hidden = options
1524-
? options.hidden
1525-
: !!this.isCellCovered(rowIndex, colIndex);
1526-
1527-
const zIndex = hidden
1528-
? '-1'
1529-
: // #updatezindex - we need to allow elements use their own zIndex, so we
1530-
// resort to allowing them to have it as a data-z-index attribute
1531-
itemElement.dataset.zIndex || 'auto';
1532-
1533-
//@ts-ignore
1534-
if (itemElement.__zIndex !== zIndex) {
15351524
//@ts-ignore
1536-
itemElement.__zIndex = zIndex;
1537-
itemElement.style.zIndex = zIndex;
1525+
if (itemElement.__zIndex !== zIndex) {
1526+
//@ts-ignore
1527+
itemElement.__zIndex = zIndex;
1528+
itemElement.style.zIndex = zIndex;
1529+
}
1530+
} else {
1531+
itemElement.style.display = '';
1532+
itemElement.style.left = `${x}px`;
1533+
itemElement.style.top = `${y}px`;
15381534
}
1535+
};
1536+
1537+
// if (this.scrolling && this.brain.isHorizontalLayoutBrain) {
1538+
if (this.scrolling && this.brain.isHorizontalLayoutBrain) {
1539+
cell.setPendingAfterCommitWork(applyPosition);
15391540
} else {
1540-
itemElement.style.display = '';
1541-
itemElement.style.left = `${x}px`;
1542-
itemElement.style.top = `${y}px`;
1541+
applyPosition();
15431542
}
15441543
}
15451544
};

0 commit comments

Comments
 (0)