Skip to content

Commit

Permalink
perf: prefer .forEach over for...in and for...of (#1281)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ghiscoding
ghiscoding authored Dec 22, 2023
1 parent f6dc75c commit 9cc6941
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 143 deletions.
17 changes: 8 additions & 9 deletions packages/common/src/core/slickCore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,11 @@ export class SlickEventData<ArgType = any> {
// when we already have an event, we want to keep some of the event properties
// looping through some props is the only way to keep and sync these properties to the returned EventData
if (event) {
const eventProps = [
[
'altKey', 'ctrlKey', 'metaKey', 'shiftKey', 'key', 'keyCode',
'clientX', 'clientY', 'offsetX', 'offsetY', 'pageX', 'pageY',
'bubbles', 'type', 'which', 'x', 'y'
];
for (const key of eventProps) {
(this as any)[key] = event[key as keyof Event];
}
].forEach(key => (this as any)[key] = event[key as keyof Event]);
}
this.target = this.nativeEvent ? this.nativeEvent.target : undefined;
}
Expand Down Expand Up @@ -660,10 +657,12 @@ export class Utils {
}

public static applyDefaults(targetObj: any, srcObj: any) {
for (const key in srcObj) {
if (srcObj.hasOwnProperty(key) && !targetObj.hasOwnProperty(key)) {
targetObj[key] = srcObj[key];
}
if (typeof srcObj === 'object') {
Object.keys(srcObj).forEach(key => {
if (srcObj.hasOwnProperty(key) && !targetObj.hasOwnProperty(key)) {
targetObj[key] = srcObj[key];
}
});
}
}

Expand Down
26 changes: 13 additions & 13 deletions packages/common/src/core/slickDataview.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1557,11 +1557,13 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD

const storeCellCssStyles = (hash: CssStyleHash) => {
hashById = {};
for (const row in hash) {
if (hash) {
const id = this.rows[row as any][this.idProperty as keyof TData];
hashById[id] = hash[row];
}
if (typeof hash === 'object') {
Object.keys(hash).forEach(row => {
if (hash) {
const id = this.rows[row as any][this.idProperty as keyof TData];
hashById[id] = hash[row];
}
});
}
};

Expand All @@ -1570,18 +1572,16 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
storeCellCssStyles(grid.getCellCssStyles(key));

const update = () => {
if (hashById) {
if (typeof hashById === 'object') {
inHandler = true;
this.ensureRowsByIdCache();
const newHash: CssStyleHash = {};
for (const id in hashById) {
if (hashById) {
const row = this.rowsById?.[id];
if (isDefined(row)) {
newHash[row as number] = hashById[id];
}
Object.keys(hashById).forEach(id => {
const row = this.rowsById?.[id];
if (isDefined(row)) {
newHash[row as number] = hashById[id];
}
}
});
grid.setCellCssStyles(key, newHash);
inHandler = false;
}
Expand Down
132 changes: 75 additions & 57 deletions packages/common/src/core/slickGrid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
/** handles "display:none" on container or container parents, related to issue: https://github.com/6pac/SlickGrid/issues/568 */
cacheCssForHiddenInit() {
this._hiddenParents = Utils.parents(this._container, ':hidden') as HTMLElement[];
for (const el of this._hiddenParents) {
this._hiddenParents.forEach(el => {
const old: Partial<CSSStyleDeclaration> = {};
for (const name in this.cssShow) {
if (this.cssShow) {
Expand All @@ -907,20 +907,22 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
}
}
this.oldProps.push(old);
}
});
}

restoreCssFromHiddenInit() {
// finish handle display:none on container or container parents
// - put values back the way they were
let i = 0;
for (const el of this._hiddenParents) {
const old = this.oldProps[i++];
for (const name in this.cssShow) {
if (this.cssShow) {
el.style[name as CSSStyleDeclarationWritable] = (old as any)[name];
if (this._hiddenParents) {
this._hiddenParents.forEach(el => {
const old = this.oldProps[i++];
for (const name in this.cssShow) {
if (this.cssShow) {
el.style[name as CSSStyleDeclarationWritable] = (old as any)[name];
}
}
}
});
}
}

Expand Down Expand Up @@ -2384,9 +2386,9 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e

const sheet = this._style.sheet;
if (sheet) {
for (const rule of rules) {
rules.forEach(rule => {
sheet.insertRule(rule);
}
});

for (let i = 0; i < this.columns.length; i++) {
if (!this.columns[i] || this.columns[i].hidden) { continue; }
Expand Down Expand Up @@ -3485,11 +3487,11 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
cellDiv.setAttribute('title', toolTipText);

if (m.hasOwnProperty('cellAttrs') && m.cellAttrs instanceof Object) {
for (const key in m.cellAttrs) {
Object.keys(m.cellAttrs).forEach(key => {
if (m.cellAttrs.hasOwnProperty(key)) {
cellDiv.setAttribute(key, m.cellAttrs[key]);
}
}
});
}

// if there is a corresponding row (if not, this is the Add New row or this data hasn't been loaded yet)
Expand Down Expand Up @@ -3546,12 +3548,18 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
if (this.currentEditor) {
this.makeActiveCellNormal();
}
for (const row in this.rowsCache) {
if (this.rowsCache) {
this.removeRowFromCache(+row);
}

if (typeof this.rowsCache === 'object') {
Object.keys(this.rowsCache).forEach(row => {
if (this.rowsCache) {
this.removeRowFromCache(+row);
}
});
}

if (this._options.enableAsyncPostRenderCleanup) {
this.startPostProcessingCleanup();
}
if (this._options.enableAsyncPostRenderCleanup) { this.startPostProcessingCleanup(); }
}

/**
Expand Down Expand Up @@ -3588,16 +3596,18 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
this.postProcessgroupId++;

// store and detach node for later async cleanup
for (const columnIdx in postProcessedRow) {
if (postProcessedRow.hasOwnProperty(columnIdx)) {
this.postProcessedCleanupQueue.push({
actionType: 'C',
groupId: this.postProcessgroupId,
node: cacheEntry.cellNodesByColumnIdx[+columnIdx],
columnIdx: +columnIdx,
rowIdx
});
}
if (typeof postProcessedRow === 'object') {
Object.keys(postProcessedRow).forEach(columnIdx => {
if (postProcessedRow.hasOwnProperty(columnIdx)) {
this.postProcessedCleanupQueue.push({
actionType: 'C',
groupId: this.postProcessgroupId,
node: cacheEntry.cellNodesByColumnIdx[+columnIdx],
columnIdx: +columnIdx,
rowIdx
});
}
});
}

if (!cacheEntry.rowNode) {
Expand Down Expand Up @@ -3920,11 +3930,15 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
// remove the rows that are now outside of the data range
// this helps avoid redundant calls to .removeRow() when the size of the data decreased by thousands of rows
const r1 = dataLength - 1;
for (const i in this.rowsCache) {
if (Number(i) > r1) {
this.removeRowFromCache(+i);
}
if (typeof this.rowsCache === 'object') {
Object.keys(this.rowsCache).forEach(row => {
const cachedRow = +row;
if (cachedRow > r1) {
this.removeRowFromCache(cachedRow);
}
});
}

if (this._options.enableAsyncPostRenderCleanup) {
this.startPostProcessingCleanup();
}
Expand Down Expand Up @@ -4306,22 +4320,24 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e

protected invalidatePostProcessingResults(row: number) {
// change status of columns to be re-rendered
for (const columnIdx in this.postProcessedRows[row]) {
if (this.postProcessedRows[row].hasOwnProperty(columnIdx)) {
this.postProcessedRows[row][columnIdx] = 'C';
}
if (typeof this.postProcessedRows[row] === 'object') {
Object.keys(this.postProcessedRows[row]).forEach(columnIdx => {
if (this.postProcessedRows[row].hasOwnProperty(columnIdx)) {
this.postProcessedRows[row][columnIdx] = 'C';
}
});
}
this.postProcessFromRow = Math.min(this.postProcessFromRow as number, row);
this.postProcessToRow = Math.max(this.postProcessToRow as number, row);
this.startPostProcessing();
}

protected updateRowPositions() {
for (const row in this.rowsCache) {
if (this.rowsCache) {
if (this.rowsCache && typeof this.rowsCache === 'object') {
Object.keys(this.rowsCache).forEach(row => {
const rowNumber = row ? parseInt(row, 10) : 0;
Utils.setStyleSize(this.rowsCache[rowNumber].rowNode![0], 'top', this.getRowTop(rowNumber));
}
});
}
}

Expand Down Expand Up @@ -4631,33 +4647,35 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
let columnId: number | string;
let addedRowHash;
let removedRowHash;
for (const row in this.rowsCache) {
if (this.rowsCache) {
removedRowHash = removedHash?.[row];
addedRowHash = addedHash?.[row];

if (removedRowHash) {
for (columnId in removedRowHash) {
if (!addedRowHash || removedRowHash[columnId] !== addedRowHash[columnId]) {
node = this.getCellNode(+row, this.getColumnIndex(columnId));
if (node) {
node.classList.remove(removedRowHash[columnId]);
if (typeof this.rowsCache === 'object') {
Object.keys(this.rowsCache).forEach(row => {
if (this.rowsCache) {
removedRowHash = removedHash?.[row];
addedRowHash = addedHash?.[row];

if (removedRowHash) {
for (columnId in removedRowHash) {
if (!addedRowHash || removedRowHash[columnId] !== addedRowHash[columnId]) {
node = this.getCellNode(+row, this.getColumnIndex(columnId));
if (node) {
node.classList.remove(removedRowHash[columnId]);
}
}
}
}
}

if (addedRowHash) {
for (columnId in addedRowHash) {
if (!removedRowHash || removedRowHash[columnId] !== addedRowHash[columnId]) {
node = this.getCellNode(+row, this.getColumnIndex(columnId));
if (node) {
node.classList.add(addedRowHash[columnId]);
if (addedRowHash) {
for (columnId in addedRowHash) {
if (!removedRowHash || removedRowHash[columnId] !== addedRowHash[columnId]) {
node = this.getCellNode(+row, this.getColumnIndex(columnId));
if (node) {
node.classList.add(addedRowHash[columnId]);
}
}
}
}
}
}
});
}
}

Expand Down
10 changes: 6 additions & 4 deletions packages/common/src/extensions/slickCheckboxSelectColumn.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,12 @@ export class SlickCheckboxSelectColumn<T = any> {
removeList.push(row);
}
}
for (const selectedRow in this._selectedRowsLookup) {
if (selectedRow !== undefined) {
this._grid.invalidateRow(+selectedRow);
}
if (typeof this._selectedRowsLookup === 'object') {
Object.keys(this._selectedRowsLookup).forEach(selectedRow => {
if (selectedRow !== undefined) {
this._grid.invalidateRow(+selectedRow);
}
});
}

this._selectedRowsLookup = lookup;
Expand Down
6 changes: 2 additions & 4 deletions packages/common/src/extensions/slickRowSelectionModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,9 @@ export class SlickRowSelectionModel implements SelectionModel {
}

protected rowsToRanges(rows: number[]) {
const ranges = [];
const ranges: SlickRange[] = [];
const lastCell = this._grid.getColumns().length - 1;
for (const row of rows) {
ranges.push(new SlickRange(row, 0, row, lastCell));
}
rows.forEach(row => ranges.push(new SlickRange(row, 0, row, lastCell)));
return ranges;
}
}
8 changes: 4 additions & 4 deletions packages/common/src/filters/nativeSelectFilter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,13 @@ export class NativeSelectFilter implements Filter {

// collection could be an Array of Strings OR Objects
if (collection.every(x => typeof x === 'string')) {
for (const option of collection) {
collection.forEach(option => {
selectElm.appendChild(
createDomElement('option', { value: option, label: option, textContent: option })
);
}
});
} else {
for (const option of collection) {
collection.forEach(option => {
if (!option || (option[labelName] === undefined && option.labelKey === undefined)) {
throw new Error(`A collection with value/label (or value/labelKey when using Locale) is required to populate the Native Select Filter list, for example:: { filter: model: Filters.select, collection: [ { value: '1', label: 'One' } ]')`);
}
Expand All @@ -176,7 +176,7 @@ export class NativeSelectFilter implements Filter {
selectElm.appendChild(
createDomElement('option', { value: option[valueName], textContent: textLabel })
);
}
});
}

return selectElm;
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/services/collection.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ export class CollectionService<T = any> {
if (Array.isArray(filterByOptions)) {
filteredCollection = (filterResultBy === FilterMultiplePassType.merge) ? [] : [...collection];

for (const filter of filterByOptions) {
filterByOptions.forEach(filter => {
if (filterResultBy === FilterMultiplePassType.merge) {
const filteredPass = this.singleFilterCollection(collection, filter);
filteredCollection = uniqueArray([...filteredCollection, ...filteredPass]);
} else {
filteredCollection = this.singleFilterCollection(filteredCollection, filter);
}
}
});
} else {
filteredCollection = this.singleFilterCollection(collection, filterByOptions);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/services/domUtilities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function buildMsSelectCollectionList(type: 'editor' | 'filter', collectio
// collection could be an Array of Strings OR Objects
if (Array.isArray(collection)) {
if (collection.every((x: any) => typeof x === 'number' || typeof x === 'string')) {
for (const option of collection) {
collection.forEach(option => {
const selectOption: OptionRowData = { text: option, value: option };
if (type === 'filter' && Array.isArray(searchTerms)) {
selectOption.selected = (searchTerms.findIndex(term => term === option) >= 0); // when filter search term is found then select it in dropdown
Expand All @@ -57,7 +57,7 @@ export function buildMsSelectCollectionList(type: 'editor' | 'filter', collectio
if ((selectOption.selected && isMultiSelect) || (selectOption.selected && !isMultiSelect && option !== '')) {
hasFoundSearchTerm = true;
}
}
});
} else {
// array of objects will require a label/value pair unless a customStructure is passed
collection.forEach((option: SelectOption) => {
Expand Down
Loading

0 comments on commit 9cc6941

Please sign in to comment.