Primeng: Loading indicator fails with Lazy DataTable

[rlexa]

I'm submitting a ... (check one with "x")

[x] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primeng/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=35

Current behavior
This is a repost of #3293 and it is definitely not a duplicate of #3041 as it doesn't have anything to do with the virtual height settings.
Using datatable with virtual scrolling and lazy loading and subsequently setting data inside of onLazyLoad which is then used in templates causes to the Angular 4.x. expression changed exception. This is a problem for dev builds and karma unit testing.
Hint: currently circumventing by embedding the onLazyLoad code in a setTimeout call.

Expected behavior
onLazyLoad should be called outside of change detection context i.e. it should be possible to set data in onLazyLoad without resorting to timeouts etc.

Minimal reproduction of the problem with instructions
Set the data actually being used in dataTable in onLazyLoad e.g. data = [1,2,3,4] and total = data.length and use it in the template e.g. {{ total }}. In dev build it should cause the exception and it should work alright when embedded in setTimeout.

What is the motivation / use case for changing the behavior?
DataTabe with lazy loading is very useful, but broken tests prevent it from being tested properly.

Please tell us about your environment:
Windows 10, npm, ng console.

• Angular version: 4.2.5

• PrimeNG version: 4.1.0-rc.3

• Browser: Chrome

• Language: TypeScript 2.3

• Node (for AoT issues): node --version = v6.10.3

[RebelSyntax]

I experienced the same issue too. Was plaguing me. What I ended up realizing (not sure if this is bug or feature of angular) is that change detection was looking at my child component the p-datatable and seeing the value as undefined. Since I used square bracket databinding to set the datatable value in the template, my tests were failing. To be clear, I didn't experience this issue outside of unit testing, but tests failing here killed my code coverage requirement.

Long story short, what I ended up doing was getting hold of the child datatable object with @ViewChild(dt) datatable:DataTable. Then, inside of my lazy load call back, I set the datatable.value property to my lazy loaded object. Voila', worked like a charm. My dev and prod build still worked, and seemed snappier to boot. Also code coverage requirement is complete.

If I have time, I will set up a minimal repro. Like I said, not sure if this is a bug in primeng or just a feature of angular that suffered my inability to use the square bracket template syntax as expected.

I also wonder if there is a bug in angular that prevented that expression has changed error message from firing outside of unit test.

[StevenLiekens]

Long story short: onLazyLoad should be triggered after AfterViewInit or else you likely end up seeing ExpressionChangedAfterItHasBeenCheckedError in development.

@RebelSyntax You should see this error in development builds but never in production builds. Another possible cause is that your unit tests run synchronously while your real code runs asynchronously. Only synchronous code can result in this error.

[RebelSyntax]

@StevenLiekens Funny thing, when I had this problem, I didn't see it even in dev builds. Only came up in testing when trying to mock data held in p-datatable [value]= attribute.

I just grabbed a reference to the @ViewChild of the datatable in my component using a template variable and then at the end of my lazy load call chain (after lazy paging, lazy filtering, lazy sorting customizations) I set datatable.value to my new object array returned by my lazy chain functions. No more expression has changed problems in ng test.

[StevenLiekens]

@RebelSyntax did you read my edit?

I have a good guess of why you had that problem: your unit tests probably use in-memory test data while your real code goes out to a web service to fetch data. One runs synchronously and the other is asynchronous.

Let me know if my assumptions are wrong.

[RebelSyntax]

@StevenLiekens you are right, that is something that I definitely looked at in my test cases. So it could just be my misunderstanding of the data flow in the testbed. That said, I am not mocking the backend call, just using Observables to control what data is presented to the component at various points in testing.

Being aware of the async issue you mentioned in advanced and not wanting to wire up mock backend, I tried timeouts, tick, and async observeables. I assume I just did not get it right if the case is as you say an async issue in the test (which makes perfect sense as well).

Now I feel more compelled to wire up the mock backend and revert my component change that "fixed" the test to see if I have positive results.

I feel like I have a good handle on the component data flow and how angular handles change detection as well as the lifecycle hooks. I definitely realized right away that lazy load needs to happen after AfterViewInit. As I mentioned, I did not cross this error in dev or prod builds, it's just a problem for me in testing.

I feel this still may be some buggy nature in the lazy load functionality, but there is some question also in the cli discussion that this may be some bug in angular that used to exist that hid the problem. Once that bug was resolved angular supposedly started acting like it should with not suppressing this error.

It's definitely an issue that needs more documentation for both testing and dev/prod builds.

I am perfectly fine with grabbing reference to 3rd party child component (p-datatable) and manipulating the object inside of the component code rather than the template syntax. I do it anyways with my custom filters/sorts/paging etc. I am comfortable in my understanding of the view encapsulation that occurs with angular components/child components and templates/css.

The solution I came up with just feels like better code and definitely became more testable without having to wire up a much more verbose test harness.

Still worth playing around with to gain deeper insight into the primeng plugin and how interaction occurs with angular.

Thanks for you comments!

[cagataycivici]

Good news, as the DataTable knows when data is loading and loaded, it handles loading property itself, you don't need to use loading property externally, fixes the problem as well. So with next version please remove the loading prop in a lazy datatable and it'll just work.

[andrei-ivanov]

I can't wait to test the next release, it was driving me crazy and I was unable to upgrade Angular because of it :-)

[machfalcon]

Looks like there's an issue with this fix if the data doesn't change when the lazy load event is fired. In our usage, we are ignoring the lazy load event on horizontal scroll since we are loading the full row up front and not lazy loading any column data. However, since handleDataChange isn't called since we aren't changing the data, the table still displays the loading spinner and we can't set it back to not loading unless we do some workaround such as:

this.data = [...this.data];

[ciukstar]

How to deal with following example:

<p-dataTable [value]="items" [loading]="loading" loadingIcon="fa-spinner" [lazy]="true" 
                     [emptyMessage]="loading ? 'Please wait...' : 'No records found'">

"it handles loading property itself" - Now I cannot use loding property in emptyMessage expression.

[ctrl-brk]

What does "it handles loading property itself" mean? If I don't specify loading property there's no spinner or any visual effect. And if I do I have "expression changed..." error. I need to change loading property within setTimeout to get rid of this error.

[andrei-ivanov]

I think the fix for this issue was reverted later and probably now it triggers the error again :-(

[carmu]

I fixed this, using settime():
///html
[lazy]="true" (onLazyLoad)="filter($event)" >

///ts
filter(event: LazyLoadEvent) {
setTimeout(() => this.getPedidos(), 0);
}

getPedidos(){
this.loading = true;
this.pedidosService.filterPedidos(this.pedidoFilter).subscribe(result => {
this.pedidos = resultado.registros;
this.loading = false;
},
() => {
this.error = "No se pudieron recuperar los pedidos"
this.loading = false;
})
}