Skip to content

fix(grids): getState no longer mutates live sorting/groupBy/filtering expressions #17194

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.

Sign up for GitHub

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

Merged
ChronosSF merged 10 commits into from
Apr 20, 2026
Merged

fix(grids): getState no longer mutates live sorting/groupBy/filtering expressions #17194

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8a405fa
fix(grids): prevent getState from mutating sorting and filtering expr…
viktorkombov Apr 16, 2026
37f3299
Merge branch 'master' into vkombov/fix-17105
viktorkombov Apr 16, 2026
a98469e
fix(grids): remove casting to any
viktorkombov Apr 16, 2026
f748be4
fix(*): apply requested changes from code review
viktorkombov Apr 16, 2026
7052d39
Merge branch 'vkombov/fix-17105' of https://github.com/IgniteUI/ignit…
viktorkombov Apr 16, 2026
b163dbf
fix(grid): optimize filtering state handling
viktorkombov Apr 16, 2026
49ddfc8
Apply suggestions from code review
viktorkombov Apr 17, 2026
5200534
fix(tests): fix a failing test
viktorkombov Apr 17, 2026
032ae0b
fix(tests): remove f from describe
viktorkombov Apr 17, 2026
8849f13
Merge branch 'master' into vkombov/fix-17105
ChronosSF Apr 17, 2026
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
62 changes: 34 additions & 28 deletions projects/igniteui-angular/grids/core/src/state-base.directive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ export class IgxGridStateBaseDirective {
private FEATURES = {
sorting: {
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
const sortingState = context.currGrid.sortingExpressions;
sortingState.forEach(s => {
delete s.strategy;
delete s.owner;
const sortingState = context.currGrid.sortingExpressions.map(s => {
const copy = { ...s };
delete copy.strategy;
delete copy.owner;
return copy;
});
return { sorting: sortingState };
},
Expand All @@ -154,10 +155,7 @@ export class IgxGridStateBaseDirective {
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
const filteringState = context.currGrid.filteringExpressionsTree;
if (filteringState) {
delete filteringState.owner;
for (const item of filteringState.filteringOperands) {
delete (item as IFilteringExpressionsTree).owner;
}
return { filtering: context.cloneFilteringTree(filteringState) };
}
return { filtering: filteringState };
},
Expand All @@ -169,16 +167,7 @@ export class IgxGridStateBaseDirective {
advancedFiltering: {
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
const filteringState = context.currGrid.advancedFilteringExpressionsTree;
let advancedFiltering: any;
if (filteringState) {
delete filteringState.owner;
for (const item of filteringState.filteringOperands) {
delete (item as IFilteringExpressionsTree).owner;
}
advancedFiltering = filteringState;
} else {
advancedFiltering = {};
}
const advancedFiltering: any = filteringState ? context.cloneFilteringTree(filteringState) : {};
return { advancedFiltering };
},
restoreFeatureState: (context: IgxGridStateBaseDirective, state: IFilteringExpressionsTree): void => {
Expand Down Expand Up @@ -226,21 +215,21 @@ export class IgxGridStateBaseDirective {
},
restoreFeatureState: (context: IgxGridStateBaseDirective, state: IColumnState[]): void => {
const newColumns = [];

// Helper to restore column state without auto-persisting widths
const restoreColumnState = (column: IgxColumnComponent | IgxColumnGroupComponent, colState: IColumnState) => {
// Extract width to handle it separately
const width = colState.width;
delete colState.width;

Object.assign(column, colState);

// Only restore width if it was explicitly set by the user (not undefined)
if (width !== undefined) {
column.width = width;
}
};

state.forEach((colState) => {
const hasColumnGroup = colState.columnGroup;
const hasColumnLayouts = colState.columnLayout;
Expand All @@ -257,9 +246,9 @@ export class IgxGridStateBaseDirective {
} else {
ref1.children.reset([]);
}

restoreColumnState(ref1, colState);

ref1.grid = context.currGrid;
if (colState.parent || colState.parentKey) {
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref1.parent));
Expand All @@ -277,7 +266,7 @@ export class IgxGridStateBaseDirective {
}

restoreColumnState(ref, colState);

ref.grid = context.currGrid;
if (colState.parent || colState.parentKey) {
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref.parent));
Expand All @@ -299,9 +288,10 @@ export class IgxGridStateBaseDirective {
groupBy: {
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
const grid = context.currGrid;
const groupingExpressions = grid.groupingExpressions;
groupingExpressions.forEach(expr => {
delete expr.strategy;
const groupingExpressions = grid.groupingExpressions.map(expr => {
const copy = { ...expr };
delete copy.strategy;
Comment thread
viktorkombov marked this conversation as resolved.
return copy;
});
const expansionState = grid.groupingExpansionState;
const groupsExpanded = grid.groupsExpanded;
Expand Down Expand Up @@ -677,6 +667,22 @@ export class IgxGridStateBaseDirective {
}
}

/**
* Recursively clones a filtering expression tree, stripping the `owner` property
* from each cloned node so the live tree is never mutated.
*/
private cloneFilteringTree(tree: IFilteringExpressionsTree): IFilteringExpressionsTree {
const copy: IFilteringExpressionsTree = { ...tree };
delete copy.owner;
copy.filteringOperands = tree.filteringOperands.map(item => {
if ('filteringOperands' in item) {
return this.cloneFilteringTree(item as IFilteringExpressionsTree);
}
return { ...item };
});
return copy;
}

/**
* This method builds a rehydrated IExpressionTree from a provided object.
*/
Expand Down
106 changes: 106 additions & 0 deletions projects/igniteui-angular/grids/core/src/state.directive.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,112 @@ describe('IgxGridState - input properties #grid', () => {
expect(prodIdColumn.colEnd).toBe(1);
});

it('getState should not mutate live sorting expressions (strategy/owner)', () => {
const fix = TestBed.createComponent(IgxGridStateComponent);
fix.detectChanges();
const grid = fix.componentInstance.grid;
const state = fix.componentInstance.state;

const customStrategy = DefaultSortingStrategy.instance();
const owner = {} as any;
grid.sortingExpressions = [
{ fieldName: 'ProductID', dir: SortingDirection.Asc, ignoreCase: false, strategy: customStrategy, owner }
];
fix.detectChanges();

expect(grid.sortingExpressions[0].strategy).toBe(customStrategy, 'strategy should be set before getState');
expect(grid.sortingExpressions[0].owner).toBe(owner, 'owner should be set before getState');

state.getState(false, 'sorting');

expect(grid.sortingExpressions[0].strategy).toBe(customStrategy, 'strategy should not be removed from live expressions after getState');
Comment thread
viktorkombov marked this conversation as resolved.
expect(grid.sortingExpressions[0].owner).toBe(owner, 'owner should not be removed from live expressions after getState');
});

it('getState should not mutate live groupBy expressions (strategy/owner)', () => {
const fix = TestBed.createComponent(IgxGridStateComponent);
fix.detectChanges();
Comment thread
viktorkombov marked this conversation as resolved.
const grid = fix.componentInstance.grid;
const state = fix.componentInstance.state;

const customStrategy = DefaultSortingStrategy.instance();
const owner = {} as any;
grid.groupingExpressions = [
{ fieldName: 'ProductID', dir: SortingDirection.Asc, ignoreCase: false, strategy: customStrategy, owner }
];
fix.detectChanges();

expect(grid.groupingExpressions[0].strategy).toBe(customStrategy, 'strategy should be set before getState');
expect(grid.groupingExpressions[0].owner).toBe(owner, 'owner should be set before getState');

state.getState(false, 'groupBy');

Comment thread
viktorkombov marked this conversation as resolved.
Outdated
expect(grid.groupingExpressions[0].strategy).toBe(customStrategy, 'strategy should not be removed from live groupBy expressions after getState');
expect(grid.groupingExpressions[0].owner).toBe(owner, 'owner should not be removed from live groupBy expressions after getState');
});

it('getState should not mutate live filtering expressions (owner)', () => {
const fix = TestBed.createComponent(IgxGridStateComponent);
fix.detectChanges();
const grid = fix.componentInstance.grid;
const state = fix.componentInstance.state;

const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
productFilteringTree.filteringOperands.push({
condition: IgxBooleanFilteringOperand.instance().condition('true'),
conditionName: 'true',
fieldName: 'InStock',
ignoreCase: true
});
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
filteringTree.filteringOperands.push(productFilteringTree);
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
grid.filteringExpressionsTree = filteringTree;
fix.detectChanges();

expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
.toBe('nestedOwner', 'nested owner should be set before getState');

state.getState(false, 'filtering');

expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live filtering tree after getState');
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
.toBe('nestedOwner', 'nested owner should not be removed from live filtering operand after getState');
});

it('getState should not mutate live advancedFiltering expressions (owner)', () => {
const fix = TestBed.createComponent(IgxGridStateComponent);
fix.detectChanges();
const grid = fix.componentInstance.grid;
const state = fix.componentInstance.state;

const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
productFilteringTree.filteringOperands.push({
condition: IgxBooleanFilteringOperand.instance().condition('true'),
conditionName: 'true',
fieldName: 'InStock',
ignoreCase: true
});
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
filteringTree.filteringOperands.push(productFilteringTree);
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
grid.advancedFilteringExpressionsTree = filteringTree;
fix.detectChanges();

expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
.toBe('nestedOwner', 'nested owner should be set before getState');

state.getState(false, 'advancedFiltering');

expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live advanced filtering tree after getState');
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
.toBe('nestedOwner', 'nested owner should not be removed from live advanced filtering operand after getState');
});

it('should preserve column widths when restoring state with all columns hidden', () => {
const fix = TestBed.createComponent(IgxGridStateComponent);
fix.detectChanges();
Expand Down
Loading