Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: decrease calls to setItems & grid invalidate #1363

Merged
merged 2 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe('AutocompleterEditor', () => {
gridOptionMock.translater = translateService;
gridOptionMock.enableTranslate = true;
const mockCollection = ['male', 'female'];
const promise = new Promise(resolve => resolve(mockCollection));
const promise = Promise.resolve(mockCollection);
(mockColumn.internalColumnEditor as ColumnEditor).collection = null as any;
(mockColumn.internalColumnEditor as ColumnEditor).collectionAsync = promise;

Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/editors/__tests__/selectEditor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ describe('SelectEditor', () => {

it('should initialize the editor with element being disabled in the DOM when passing a collectionAsync and an empty collection property', () => {
const mockCollection = ['male', 'female'];
const promise = new Promise(resolve => resolve(mockCollection));
const promise = Promise.resolve(mockCollection);
(mockColumn.internalColumnEditor as ColumnEditor).collection = null as any;
(mockColumn.internalColumnEditor as ColumnEditor).collectionAsync = promise;
gridOptionMock.translater = translateService;
Expand Down
4 changes: 1 addition & 3 deletions packages/common/src/services/__tests__/grid.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1673,13 +1673,11 @@ describe('Grid Service', () => {
describe('renderGrid method', () => {
it('should invalidate the grid and call render after', () => {
const invalidateSpy = jest.spyOn(gridStub, 'invalidate');
const renderSpy = jest.spyOn(gridStub, 'render');

service.renderGrid();

expect(invalidateSpy).toHaveBeenCalled();
expect(renderSpy).toHaveBeenCalled();
expect(gridStub.invalidate).toHaveBeenCalledBefore(gridStub.render as any);
expect(gridStub.invalidate).toHaveBeenCalled();
});
});

Expand Down
3 changes: 1 addition & 2 deletions packages/common/src/services/grid.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,8 @@ export class GridService {

/** Re-Render the Grid */
renderGrid() {
if (this._grid && typeof this._grid.invalidate === 'function') {
if (typeof this._grid?.invalidate === 'function') {
this._grid.invalidate();
this._grid.render();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()

it('should be able to load async editors with a regular Promise', (done) => {
const mockCollection = ['male', 'female'];
const promise = new Promise(resolve => resolve(mockCollection));
const promise = Promise.resolve(mockCollection);
const mockColDefs = [{ id: 'gender', field: 'gender', editor: { model: Editors.text, collectionAsync: promise } }] as Column[];

component.columnDefinitions = mockColDefs;
Expand All @@ -730,7 +730,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()

it('should be able to load collectionAsync and expect Editor to be destroyed and re-render when receiving new collection from await', (done) => {
const mockCollection = ['male', 'female'];
const promise = new Promise(resolve => resolve(mockCollection));
const promise = Promise.resolve(mockCollection);
const mockEditor = {
disable: jest.fn(),
destroy: jest.fn(),
Expand Down Expand Up @@ -758,7 +758,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()

it('should be able to load async editors with as a Promise with content to simulate http-client', (done) => {
const mockCollection = ['male', 'female'];
const promise = new Promise(resolve => resolve({ content: mockCollection }));
const promise = Promise.resolve({ content: mockCollection });
const mockColDefs = [{ id: 'gender', field: 'gender', editor: { model: Editors.text, collectionAsync: promise } }] as Column[];

component.columnDefinitions = mockColDefs;
Expand Down Expand Up @@ -1064,6 +1064,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
const refreshSpy = jest.spyOn(component, 'refreshGridData');

const mockData = [{ firstName: 'John', lastName: 'Doe' }, { firstName: 'Jane', lastName: 'Smith' }];
jest.spyOn(mockDataView, 'getItems').mockReturnValueOnce(mockData);
component.gridOptions = {
enablePagination: true,
presets: { pagination: { pageSize: 2, pageNumber: expectedPageNumber } }
Expand Down Expand Up @@ -1529,7 +1530,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
service: mockGraphqlService2,
options: mockGraphqlOptions,
preProcess: () => jest.fn(),
process: () => new Promise(resolve => resolve({ data: { users: { nodes: [], totalCount: 100 } } })),
process: () => Promise.resolve({ data: { users: { nodes: [], totalCount: 100 } } }),
} as GraphqlServiceApi,
pagination: mockPagination,
} as unknown as GridOption;
Expand All @@ -1547,7 +1548,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
backendServiceApi: {
service: mockGraphqlService,
preProcess: () => jest.fn(),
process: () => new Promise(resolve => resolve('process resolved')),
process: () => Promise.resolve('process resolved'),
}
} as unknown as GridOption;
component.initialization(divContainer, slickEventHandler);
Expand All @@ -1564,7 +1565,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
service: mockGraphqlService,
useLocalSorting: true,
preProcess: () => jest.fn(),
process: () => new Promise(resolve => resolve('process resolved')),
process: () => Promise.resolve('process resolved'),
}
} as unknown as GridOption;
component.initialization(divContainer, slickEventHandler);
Expand Down Expand Up @@ -1594,7 +1595,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
service: mockGraphqlService,
useLocalFiltering: true,
preProcess: () => jest.fn(),
process: () => new Promise(resolve => resolve('process resolved')),
process: () => Promise.resolve('process resolved'),
}
} as unknown as GridOption;
component.initialization(divContainer, slickEventHandler);
Expand All @@ -1612,7 +1613,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
backendServiceApi: {
service: mockGraphqlService,
preProcess: () => jest.fn(),
process: () => new Promise(resolve => resolve('process resolved')),
process: () => Promise.resolve('process resolved'),
}
} as unknown as GridOption;
component.initialization(divContainer, slickEventHandler);
Expand Down Expand Up @@ -1904,7 +1905,6 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
});

it('should have custom footer with metrics when the DataView "onSetItemsCalled" event is triggered', () => {
const invalidateSpy = jest.spyOn(mockGrid, 'invalidate');
const expectation = {
startTime: expect.toBeDate(),
endTime: expect.toBeDate(),
Expand All @@ -1918,7 +1918,6 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
const footerSpy = jest.spyOn(component.slickFooter!, 'metrics', 'set');
mockDataView.onSetItemsCalled.notify({ idProperty: 'id', itemCount: 0 });

expect(invalidateSpy).toHaveBeenCalled();
expect(component.metrics).toEqual(expectation);
expect(footerSpy).toHaveBeenCalledWith(expectation);
});
Expand Down Expand Up @@ -2119,6 +2118,7 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file', initialSort: { columndId: 'file', direction: 'ASC' } } } as unknown as GridOption;
component.datasetHierarchical = mockHierarchical;
component.eventPubSubService = new EventPubSubService(divContainer);
component.isDatasetHierarchicalInitialized = true;
component.initialization(divContainer, slickEventHandler);

expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
Expand Down
43 changes: 26 additions & 17 deletions packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ export class SlickVanillaGridBundle<TData = any> {
this._eventPubSubService = pubSub;
}

set isDatasetHierarchicalInitialized(isInitialized: boolean) {
this._isDatasetHierarchicalInitialized = isInitialized;
}

get gridOptions(): GridOption {
return this._gridOptions || {} as GridOption;
}
Expand Down Expand Up @@ -382,11 +386,7 @@ export class SlickVanillaGridBundle<TData = any> {
this.universalContainerService.registerInstance('TranslaterService', this.translaterService);
this.universalContainerService.registerInstance('TreeDataService', this.treeDataService);

this.initialization(this._gridContainerElm, eventHandler);
if (!hierarchicalDataset && !this.gridOptions.backendServiceApi) {
this.setData(dataset || []);
this._currentDatasetLength = this.dataset.length;
}
this.initialization(this._gridContainerElm, eventHandler, dataset);
}

emptyGridContainerElm() {
Expand Down Expand Up @@ -471,7 +471,7 @@ export class SlickVanillaGridBundle<TData = any> {
this._registeredResources = [];
}

initialization(gridContainerElm: HTMLElement, eventHandler: SlickEventHandler) {
initialization(gridContainerElm: HTMLElement, eventHandler: SlickEventHandler, inputDataset?: TData[]) {
// when detecting a frozen grid, we'll automatically enable the mousewheel scroll handler so that we can scroll from both left/right frozen containers
if (this.gridOptions && ((this.gridOptions.frozenRow !== undefined && this.gridOptions.frozenRow >= 0) || this.gridOptions.frozenColumn !== undefined && this.gridOptions.frozenColumn >= 0) && this.gridOptions.enableMouseWheelScrollHandler === undefined) {
this.gridOptions.enableMouseWheelScrollHandler = true;
Expand Down Expand Up @@ -576,9 +576,13 @@ export class SlickVanillaGridBundle<TData = any> {
}

// load the data in the DataView (unless it's a hierarchical dataset, if so it will be loaded after the initial tree sort)
if (Array.isArray(this.dataset)) {
const initialDataset = this.gridOptions?.enableTreeData ? this.sortTreeDataset(this.dataset) : this.dataset;
this.dataView?.setItems(initialDataset, this._gridOptions.datasetIdPropertyName);
if (this.dataView) {
inputDataset = inputDataset || [];
const initialDataset = this.gridOptions?.enableTreeData ? this.sortTreeDataset(inputDataset) : inputDataset;
this.dataView.beginUpdate();
this.dataView.setItems(initialDataset, this._gridOptions.datasetIdPropertyName);
this._currentDatasetLength = inputDataset.length;
this.dataView.endUpdate();
}

// if you don't want the items that are not visible (due to being filtered out or being on a different page)
Expand All @@ -604,14 +608,14 @@ export class SlickVanillaGridBundle<TData = any> {
}
}

this.slickGrid.invalidate();

if ((this.dataView?.getLength() ?? 0) > 0) {
if (!this._isDatasetInitialized && (this._gridOptions.enableCheckboxSelector || this._gridOptions.enableRowSelection)) {
this.loadRowSelectionPresetWhenExists();
}
this.loadFilterPresetsWhenDatasetInitialized();
this._isDatasetInitialized = true;
} else {
this.displayEmptyDataWarning(true);
}

// user might want to hide the header row on page load but still have `enableFiltering: true`
Expand Down Expand Up @@ -643,6 +647,14 @@ export class SlickVanillaGridBundle<TData = any> {
// bind resize ONLY after the dataView is ready
this.bindResizeHook(this.slickGrid, this.gridOptions);

// local grid, check if we need to show the Pagination
// if so then also check if there's any presets and finally initialize the PaginationService
// a local grid with Pagination presets will potentially have a different total of items, we'll need to get it from the DataView and update our total
if (this.gridOptions?.enablePagination && this._isLocalGrid) {
this.showPagination = true;
this.loadLocalGridPagination(this.dataset);
}

// once the grid is created, we'll return its instance (we do this to return Transient Services from DI)
this._slickerGridInstances = {
// Slick Grid & DataView objects
Expand Down Expand Up @@ -963,11 +975,6 @@ export class SlickVanillaGridBundle<TData = any> {
// if so then also check if there's any presets and finally initialize the PaginationService
// a local grid with Pagination presets will potentially have a different total of items, we'll need to get it from the DataView and update our total
if (this.slickGrid && this._gridOptions) {
if (this._gridOptions.enablePagination && this._isLocalGrid) {
this.showPagination = true;
this.loadLocalGridPagination(dataset);
}

if (this._gridOptions.enableEmptyDataWarningMessage && Array.isArray(dataset)) {
const finalTotalCount = totalCount || dataset.length;
this.displayEmptyDataWarning(finalTotalCount < 1);
Expand Down Expand Up @@ -1098,7 +1105,9 @@ export class SlickVanillaGridBundle<TData = any> {
}

protected displayEmptyDataWarning(showWarning = true) {
this.slickEmptyWarning?.showEmptyDataMessage(showWarning);
if (this.gridOptions.enableEmptyDataWarningMessage) {
this.slickEmptyWarning?.showEmptyDataMessage(showWarning);
}
}

/** When data changes in the DataView, we'll refresh the metrics and/or display a warning if the dataset is empty */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ describe('Vanilla-Force-Grid-Bundle Component instantiated via Constructor', ()
const refreshSpy = jest.spyOn(component, 'refreshGridData');

const mockData = [{ firstName: 'John', lastName: 'Doe' }, { firstName: 'Jane', lastName: 'Smith' }];
jest.spyOn(mockDataView, 'getItems').mockReturnValueOnce(mockData);
component.gridOptions = {
enablePagination: true,
presets: { pagination: { pageSize: 2, pageNumber: expectedPageNumber } }
Expand Down
4 changes: 2 additions & 2 deletions test/cypress/e2e/example17.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ describe('Example 17 - Auto-Scroll with Range Selector', () => {
cy.get(`.grid17-2 [style="top: ${CELL_HEIGHT * 0}px;"]`).should('have.length', 2 * 2);
cy.get(`.grid17-1 .grid-canvas-left > [style="top: ${CELL_HEIGHT * 0}px;"]`).children().should('have.length', 2 * 2);
cy.get(`.grid17-2 .grid-canvas-left > [style="top: ${CELL_HEIGHT * 0}px;"]`).children().should('have.length', 2 * 2);
cy.get('.grid17-1 .grid-canvas-top').children().should('have.length', 3 * 2 + 1); // +1 for "Empty Data" div
cy.get('.grid17-2 .grid-canvas-top').children().should('have.length', 3 * 2 + 1);
cy.get('.grid17-1 .grid-canvas-top').children().should('have.length', 3 * 2);
cy.get('.grid17-2 .grid-canvas-top').children().should('have.length', 3 * 2);
});

function resetScrollInFrozen() {
Expand Down
2 changes: 1 addition & 1 deletion test/translateServiceStub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,6 @@ export class TranslateServiceStub implements TranslaterService {
}

use(locale: string) {
return new Promise(resolve => resolve(this._locale = locale));
return Promise.resolve(this._locale = locale);
}
}
Loading