Performance optimizations

[jtomaszewski]

Is the angular2-data-table optimized when it comes to performance?

I didn't get to see through all the code, but after a quick glimpse:

• we could add a trackBy option to *ngFor="let row of rows; let i = index;" in Body component's template
• components like BodyCell, BodyRow should propably be dependant only on Inputs and use ChangeDetectionStrategy.OnPush
• components like Body propably could also use ChangeDetectionStrategy.OnPush, but as it's currently heavily dependant on StateService, we would need to add some rxjs observables to that or something.

What do you guys think? Maybe I could help a bit with that.

[amcdnl]

@jtomaszewski - Actually, I tried some of those ....

1. trackBy gave me some odd errors when i tried it rc1, worth re-looking at for sure. You can track by the column id defined https://github.com/swimlane/angular2-data-table/blob/master/src/models/TableColumn.ts#L23.
2. I had those changed, however, that caused errors with things not updating correctly. See this https://github.com/swimlane/angular2-data-table/commit/4c0658dff6b4cf6d9b040f5c3cc56b929911f27f#diff-6c5fe72c2cab8659bc1ddbfa686ed2f9.
3. Absolutely. I've already got an issue open for https://github.com/swimlane/angular2-data-table/issues/32 support. I'd love to work with you on implementing that, my rx is not to the standard something like this requires.

[jtomaszewski]

OK, I did most of it in my master branch. Can you check it out https://github.com/jtomaszewski/angular2-data-table , and either give me notes, what should I improve, or cherry-pick / merge all the commits ? (So I don't have to create a PR for each of them ;]).

[jtomaszewski]

Hey, why did you close it? ;]

There's still some commits here that can be merged:

• jtomaszewski@8658afe
• jtomaszewski@9b87bef
• jtomaszewski@b63bc30
• jtomaszewski@16d2652

Should I do sepearte PRs for all of them?

[amcdnl]

SorryI thought you said it was closed. I will manually merge them in :)

[amcdnl]

The do check has to stay but I did the rest :)

[jtomaszewski]

Thanks for the quick merging ;)

Why does the doCheck has to stay? It's tempting to get rid of it as well.

[amcdnl]

Its for when the columns change externally ( i think lol ) and one other thing I forgot. I will dig it up though and add comments.

[amcdnl]

Closing as no longer relevant due to total onpush refactor.