DevExpress / devextreme-angular

Angular UI and data visualization components
https://js.devexpress.com/Demos/
MIT License
558 stars 160 forks source link

DxDataGrid endless loop if [columns] is bound to component #96

Closed marcelgood closed 7 years ago

marcelgood commented 8 years ago

If binding the columns property to a property on the component that returns the array of columns, the grid goes into some kind of an endless loop and the application is unusable. This can be reproduced in your example. If I modify the DxDataGrid in the example like so:

<dx-data-grid
    [columns]='gridColumns'
    [dataSource]='customers'
    [paging]='{
        pageSize: 10
    }'
    [pager]= '{
        showPageSizeSelector: true,
        allowedPageSizes: [5, 10, 20],
        showInfo: true
    }'>
    <div *dxTemplate="let cellData = data of 'phoneCellTemplate'">
        <dx-button (onClick)="callNumber(cellData.value)" [text]="cellData.text"></dx-button>
    </div>
</dx-data-grid>

and then add a gridColumns property to AppComponent like so:

    get gridColumns() {
        return [
            "CompanyName", 
            "City", 
            "State", 
            "Fax", 
            { 
                dataField: "Phone", 
                cellTemplate: "phoneCellTemplate" 
            }
        ];
    }

then, the grid kills the entire app. You can page anymore and the page is generally messed up and Chrome uses a lot of CPU.

marcelgood commented 7 years ago

Is there an update on this and all the other reported DxDataGrid bugs?

DokaRus commented 7 years ago

Hi, The approach you've chosen makes me think that you are not on the right way. When you define columns using a getter, it returns a new array of columns each time it's called. The Angular change detection mechanism notifies the grid that its columns have been changed and as a result, the grid is rendered infinitely. I suggest that you initiate dxDataGrid with columns defined as a simple array. Alternatively, make your getter return a cached array of columns.

marcelgood commented 7 years ago

Thanks for the input. You have a good point on returning a new array every time, but that in itself shouldn't cause an endless loop. It doesn't in other scenarios. It's at most inefficient. It does indeed work if I cache the array, though.

However, this still highlights an issue in the grid. Angular 2 itself does not interpret this as a change. If it would then it would throw an exception in dev mode. Because after every round of change detection, dev mode immediately performs a second round to verify that no bindings have changed since the end of the first, as this would indicate that changes are being caused by change detection itself. Angular 2 only seems to see this as a change if the array content changes, not the array itself.

If the DxDataGrid is getting into an endless loop due to the array reference changing, then it is doing something while rendering itself that triggers another change detection or something. The act of rendering shouldn't trigger a change detection, so I think you have a bug somewhere in the grid. I'm good for now, though. It now works in my application by caching the array and only creating a new array when the columns do in fact change.

DokaRus commented 7 years ago

DxDataGrid uses setTimeout and this is the cause of this loop. Grid also supports the Angular 1, jQuery and KnockoutJS approaches, so we are not ready to get rid of timers right now. Though we will think about it, this will unlikely happen in 16.2