Skip to content

Commit 07bac2f

Browse files
committed
refactor: convert rows to signal input
As part of this change, `_internalRows` are now a computed signal. Further optimizations are done after rowGroups are turned into a signal as well.
1 parent 49d1760 commit 07bac2f

File tree

3 files changed

+63
-110
lines changed

3 files changed

+63
-110
lines changed

projects/ngx-datatable/src/lib/components/datatable.component.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
role="rowgroup"
3535
[groupRowsBy]="groupRowsBy"
3636
[groupedRows]="groupedRows"
37-
[rows]="_internalRows"
37+
[rows]="_internalRows()"
3838
[groupExpansionDefault]="groupExpansionDefault()"
3939
[scrollbarV]="scrollbarV()"
4040
[scrollbarH]="scrollbarH()"
@@ -95,7 +95,7 @@
9595
</div>
9696
@if (footerHeight()) {
9797
<datatable-footer
98-
[rowCount]="groupedRows !== undefined ? _internalRows.length : rowCount"
98+
[rowCount]="groupedRows !== undefined ? _internalRows().length : rowCount"
9999
[groupCount]="groupedRows !== undefined ? rowCount : undefined"
100100
[pageSize]="pageSize"
101101
[offset]="offset"

projects/ngx-datatable/src/lib/components/datatable.component.spec.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,10 @@ describe('DatatableComponent', () => {
238238
sortBy({ column: 2 }, fixture);
239239
fixture.detectChanges();
240240

241-
expect(textContent({ row: 1, column: 1 }, fixture)).toContain('amet');
242-
expect(textContent({ row: 2, column: 1 }, fixture)).toContain('dolor');
243-
expect(textContent({ row: 3, column: 1 }, fixture)).toContain('ipsum');
244-
expect(textContent({ row: 4, column: 1 }, fixture)).toContain('lorem');
241+
expect(textContent({ row: 1, column: 1 }, fixture)).toContain('dolor');
242+
expect(textContent({ row: 2, column: 1 }, fixture)).toContain('ipsum');
243+
expect(textContent({ row: 3, column: 1 }, fixture)).toContain('lorem');
244+
expect(textContent({ row: 4, column: 1 }, fixture)).toContain('amet');
245245
expect(textContent({ row: 5, column: 1 }, fixture)).toContain('maecennas');
246246
expect(textContent({ row: 6, column: 1 }, fixture)).toContain('sed');
247247
expect(textContent({ row: 7, column: 1 }, fixture)).toContain('foo');
@@ -301,10 +301,10 @@ describe('DatatableComponent', () => {
301301
sortBy({ column: 2 }, fixture);
302302
fixture.detectChanges();
303303

304-
expect(textContent({ row: 1, column: 1 }, fixture)).toContain('amet');
305-
expect(textContent({ row: 2, column: 1 }, fixture)).toContain('dolor');
306-
expect(textContent({ row: 3, column: 1 }, fixture)).toContain('ipsum');
307-
expect(textContent({ row: 4, column: 1 }, fixture)).toContain('lorem');
304+
expect(textContent({ row: 1, column: 1 }, fixture)).toContain('dolor');
305+
expect(textContent({ row: 2, column: 1 }, fixture)).toContain('ipsum');
306+
expect(textContent({ row: 3, column: 1 }, fixture)).toContain('lorem');
307+
expect(textContent({ row: 4, column: 1 }, fixture)).toContain('amet');
308308
expect(textContent({ row: 5, column: 1 }, fixture)).toContain('maecennas');
309309
expect(textContent({ row: 6, column: 1 }, fixture)).toContain('sed');
310310
expect(textContent({ row: 7, column: 1 }, fixture)).toContain('foo');

projects/ngx-datatable/src/lib/components/datatable.component.ts

Lines changed: 53 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
ChangeDetectionStrategy,
55
ChangeDetectorRef,
66
Component,
7+
computed,
78
ContentChild,
89
contentChildren,
910
DoCheck,
@@ -117,23 +118,7 @@ export class DatatableComponent<TRow extends Row = any>
117118
/**
118119
* Rows that are displayed in the table.
119120
*/
120-
@Input() set rows(val: (TRow | undefined)[] | null | undefined) {
121-
this._rows = val ?? [];
122-
if (val) {
123-
// This will ensure that datatable detects changes on doing like this rows = [...rows];
124-
if (val.length) {
125-
this.rowDiffer.diff([]);
126-
}
127-
this._internalRows = [...val];
128-
}
129-
}
130-
131-
/**
132-
* Gets the rows.
133-
*/
134-
get rows(): (TRow | undefined)[] {
135-
return this._rows;
136-
}
121+
readonly rows = input<(TRow | undefined)[] | null | undefined>();
137122

138123
/**
139124
* This attribute allows the user to set the name of the column to group the data with
@@ -143,7 +128,7 @@ export class DatatableComponent<TRow extends Row = any>
143128
this._groupRowsBy = val;
144129
if (this._groupRowsBy) {
145130
// creates a new array with the data grouped
146-
this.groupedRows = this.groupArrayBy(this._rows, this._groupRowsBy);
131+
this.groupedRows = this.groupArrayBy(this.rows() ?? [], this._groupRowsBy);
147132
}
148133
}
149134
}
@@ -670,15 +655,15 @@ export class DatatableComponent<TRow extends Row = any>
670655
*/
671656
get allRowsSelected(): boolean {
672657
const selected = this.selected();
673-
let allRowsSelected = this.rows && selected && selected.length === this.rows.length;
658+
let allRowsSelected = this.rows() && selected && selected.length === this.rows()!.length;
674659

675660
if (this.bodyComponent && this.selectAllRowsOnPage()) {
676661
const indexes = this.bodyComponent.indexes;
677662
const rowsOnPage = indexes().last - indexes().first;
678663
allRowsSelected = selected.length === rowsOnPage;
679664
}
680665

681-
return selected && this.rows?.length !== 0 && allRowsSelected;
666+
return !!(selected && this.rows()?.length !== 0 && allRowsSelected);
682667
}
683668

684669
element = inject<ElementRef<HTMLElement>>(ElementRef).nativeElement;
@@ -687,12 +672,36 @@ export class DatatableComponent<TRow extends Row = any>
687672
bodyHeight!: number;
688673
rowCount = 0;
689674
rowDiffer: IterableDiffer<TRow | undefined> = inject(IterableDiffers).find([]).create();
675+
/** This counter is increased, when the rowDiffer detects a change. This will cause an update of _internalRows. */
676+
private readonly _rowDiffCount = signal(0);
690677

691678
_offsetX = 0;
692679
_offset = 0;
693-
_rows: (TRow | undefined)[] = [];
694680
_groupRowsBy?: keyof TRow;
695-
_internalRows: (TRow | undefined)[] = [];
681+
readonly _internalRows = computed(() => {
682+
this._rowDiffCount(); // to trigger recalculation when row differ detects a change
683+
let rows = this.rows()?.slice() ?? [];
684+
685+
const sorts = this.sorts();
686+
if (sorts.length && !this.externalSorting()) {
687+
rows = sortRows(rows, this._internalColumns(), this.sorts());
688+
}
689+
690+
if (this.treeFromRelation() && this.treeToRelation()) {
691+
rows = groupRowsByParents(
692+
rows,
693+
optionalGetterForProp(this.treeFromRelation()),
694+
optionalGetterForProp(this.treeToRelation())
695+
);
696+
}
697+
698+
if (this.ghostLoadingIndicator() && this.scrollbarV() && !this.externalPaging()) {
699+
// in case where we don't have predefined total page length
700+
rows.push(undefined); // undefined row will render ghost cell row at the end of the page
701+
}
702+
703+
return rows;
704+
});
696705
// TODO: consider removing internal modifications of the columns.
697706
// This requires a different strategy for certain properties like width.
698707
readonly _internalColumns = linkedSignal(() =>
@@ -721,26 +730,7 @@ export class DatatableComponent<TRow extends Row = any>
721730
private readonly _rowInitDone = signal(false);
722731

723732
constructor() {
724-
// Effect to handle column template changes
725733
// TODO: This should be a computed signal.
726-
effect(() => {
727-
const columns = this.columnTemplates().map(c => c.column());
728-
// Ensure that we do not listen to other properties (yet).
729-
untracked(() => this.translateColumns(columns));
730-
});
731-
732-
// Effect to handle ghost loading indicator changes
733-
effect(() => {
734-
const ghostLoading = this.ghostLoadingIndicator();
735-
// Use untracked to only subscribe to ghostLoadingIndicator changes
736-
untracked(() => {
737-
if (ghostLoading && this.scrollbarV() && !this.externalPaging()) {
738-
// in case where we don't have predefined total page length
739-
this.rows = [...this.rows, undefined]; // undefined row will render ghost cell row at the end of the page
740-
}
741-
});
742-
});
743-
744734
// Effect to handle recalculate when limit or count changes
745735
effect(() => {
746736
// Track limit and count changes
@@ -749,6 +739,12 @@ export class DatatableComponent<TRow extends Row = any>
749739
// Recalculate without tracking other signals
750740
untracked(() => this.recalculateDims());
751741
});
742+
// Recalculates the rowCount when internal rows change
743+
// TODO: This should be a computed signal.
744+
effect(() => {
745+
this._internalRows();
746+
this.rowCount = untracked(() => this.calcRowCount());
747+
});
752748
}
753749

754750
/**
@@ -766,29 +762,21 @@ export class DatatableComponent<TRow extends Row = any>
766762
* Lifecycle hook that is called when Angular dirty checks a directive.
767763
*/
768764
ngDoCheck(): void {
769-
const rowDiffers = this.rowDiffer.diff(this.rows);
765+
const rowDiffers = this.rowDiffer.diff(this.rows());
770766
if (rowDiffers || this.disableRowCheck()) {
767+
this._rowDiffCount.update(count => count + 1);
771768
// we don't sort again when ghost loader adds a dummy row
772769
if (
773770
!this.ghostLoadingIndicator() &&
774771
!this.externalSorting() &&
775772
this._internalColumns().length
776773
) {
777774
this.sortInternalRows();
778-
} else {
779-
this._internalRows = [...this.rows];
780775
}
781776

782-
// auto group by parent on new update
783-
this._internalRows = groupRowsByParents(
784-
this._internalRows,
785-
optionalGetterForProp(this.treeFromRelation()),
786-
optionalGetterForProp(this.treeToRelation())
787-
);
788-
789777
if (this._groupRowsBy && rowDiffers) {
790778
// If a column has been specified in _groupRowsBy create a new array with the data grouped by that row
791-
this.groupedRows = this.groupArrayBy(this._rows, this._groupRowsBy);
779+
this.groupedRows = this.groupArrayBy(this.rows() ?? [], this._groupRowsBy);
792780
}
793781
if (rowDiffers) {
794782
queueMicrotask(() => {
@@ -846,18 +834,6 @@ export class DatatableComponent<TRow extends Row = any>
846834
}
847835
};
848836

849-
/**
850-
* Translates the templates to the column objects
851-
*/
852-
translateColumns(columns: TableColumn<TRow>[]) {
853-
if (columns.length) {
854-
if (!this.externalSorting() && this.rows?.length) {
855-
this.sortInternalRows();
856-
}
857-
this.cd.markForCheck();
858-
}
859-
}
860-
861837
/**
862838
* Creates a map with the data grouped by the user choice of grouping index
863839
*
@@ -1059,12 +1035,7 @@ export class DatatableComponent<TRow extends Row = any>
10591035
}
10601036

10611037
// otherwise use row length
1062-
if (this.rows) {
1063-
return this.rows.length;
1064-
}
1065-
1066-
// other empty :(
1067-
return 0;
1038+
return this.rows()?.length ?? 0;
10681039
}
10691040

10701041
/**
@@ -1074,10 +1045,8 @@ export class DatatableComponent<TRow extends Row = any>
10741045
if (!this.externalPaging()) {
10751046
if (this.groupedRows) {
10761047
return this.groupedRows.length;
1077-
} else if (this.treeFromRelation() != null && this.treeToRelation() != null) {
1078-
return this._internalRows.length;
10791048
} else {
1080-
return this.rows.length;
1049+
return this._internalRows().length;
10811050
}
10821051
}
10831052

@@ -1188,13 +1157,6 @@ export class DatatableComponent<TRow extends Row = any>
11881157
this.sortInternalRows();
11891158
}
11901159

1191-
// auto group by parent on new update
1192-
this._internalRows = groupRowsByParents(
1193-
this._internalRows,
1194-
optionalGetterForProp(this.treeFromRelation()),
1195-
optionalGetterForProp(this.treeToRelation())
1196-
);
1197-
11981160
// Always go to first page when sorting to see the newly sorted data
11991161
this.offset = 0;
12001162
this.bodyComponent.updateOffsetY(this.offset);
@@ -1221,19 +1183,23 @@ export class DatatableComponent<TRow extends Row = any>
12211183

12221184
// do the opposite here
12231185
if (!allSelected) {
1224-
this.selected.set(this._internalRows.slice(first, last).filter(row => !!row) as TRow[]);
1186+
this.selected.set(
1187+
this._internalRows()
1188+
.slice(first, last)
1189+
.filter(row => !!row) as TRow[]
1190+
);
12251191
} else {
12261192
this.selected.set([]);
12271193
}
12281194
} else {
12291195
let relevantRows: TRow[];
12301196
const disableRowCheckFn = this.disableRowCheck();
12311197
if (disableRowCheckFn) {
1232-
relevantRows = this.rows.filter(
1198+
relevantRows = (this.rows() ?? []).filter(
12331199
(row => row && !disableRowCheckFn(row)) as (row: TRow | undefined) => row is TRow
12341200
);
12351201
} else {
1236-
relevantRows = this.rows.filter(row => !!row);
1202+
relevantRows = (this.rows() ?? []).filter(row => !!row);
12371203
}
12381204
// before we splice, chk if we currently have all selected
12391205
const allSelected = this.selected().length === relevantRows.length;
@@ -1264,7 +1230,9 @@ export class DatatableComponent<TRow extends Row = any>
12641230
const row = event.row;
12651231
// TODO: For duplicated items this will not work
12661232
const treeToRel = this.treeToRelation();
1267-
const rowIndex = this._rows.findIndex(r => r && r[treeToRel!] === event.row[treeToRel!]);
1233+
const rowIndex = (this.rows() ?? []).findIndex(
1234+
r => r && r[treeToRel!] === event.row[treeToRel!]
1235+
);
12681236
this.treeAction.emit({ row, rowIndex });
12691237
}
12701238

@@ -1273,32 +1241,17 @@ export class DatatableComponent<TRow extends Row = any>
12731241
}
12741242

12751243
private sortInternalRows(): void {
1276-
// if there are no sort criteria we reset the rows with original rows
1277-
if (!this.sorts()?.length) {
1278-
this._internalRows = this._rows;
1279-
// if there is any tree relation then re-group rows accordingly
1280-
if (this.treeFromRelation() && this.treeToRelation()) {
1281-
this._internalRows = groupRowsByParents(
1282-
this._internalRows,
1283-
optionalGetterForProp(this.treeFromRelation()),
1284-
optionalGetterForProp(this.treeToRelation())
1285-
);
1286-
}
1287-
}
12881244
if (this.groupedRows?.length) {
12891245
const sortOnGroupHeader = this.sorts()?.find(
12901246
sortColumns => sortColumns.prop === this._groupRowsBy
12911247
);
1292-
this.groupedRows = this.groupArrayBy(this._rows, this._groupRowsBy!);
1248+
this.groupedRows = this.groupArrayBy(this.rows() ?? [], this._groupRowsBy!);
12931249
this.groupedRows = sortGroupedRows(
12941250
this.groupedRows,
12951251
this._internalColumns(),
12961252
this.sorts(),
12971253
sortOnGroupHeader
12981254
);
1299-
this._internalRows = [...this._internalRows];
1300-
} else {
1301-
this._internalRows = sortRows(this._internalRows, this._internalColumns(), this.sorts());
13021255
}
13031256
}
13041257
}

0 commit comments

Comments
 (0)