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

bug(mat-table): MatTable with observable as data source should trigger change detection after update #24483

Open
1 task
preda7or opened this issue Feb 25, 2022 · 9 comments
Labels
area: cdk/table P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@preda7or
Copy link

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

MatTable with an observable as data source does update viewRefs, but does not trigger a detection change (markForCheck).
Which can cause the view not to update (until a change detection is triggered by some other event)

Reproduction

Steps to reproduce:

  1. create component with OnPush change detection strategy
  2. create an observable as the data source of material table

https://stackblitz.com/edit/components-issue-5pt19s

Expected Behavior

MatTable with observable as data source should mark itself for checking (markForCheck) when viewRefs were updated.

Actual Behavior

No change detection triggered.

Environment

  • Angular: 13.2.4
  • CDK/Material: 13.2.4
  • Browser(s): Chrome 98.0.4758.102
  • Operating System (e.g. Windows, macOS, Ubuntu): Windows 10
@preda7or preda7or added the needs triage This issue needs to be triaged by the team label Feb 25, 2022
@crisbeto
Copy link
Member

Your example is using runOutsideAngular, maybe that's the root cause?

@preda7or
Copy link
Author

preda7or commented Feb 28, 2022

@crisbeto
Ah sorry, that just stayed in by accident. I took it out, you can it is still not working.

I checked the CDK source, there is nothing to trigger change detection.

That is why the observable source also important, no data binding change is happening, so if internally the table does not mark itself for check, then nothing will.

@zarend
Copy link
Contributor

zarend commented Feb 28, 2022

Hello,

It seems like the issue is that the trackBy function is tracking by the id, which remains constant. This can be fixed by tracking by the value which changes each time the timer emits.

  trackByFn(index: number, item: { id: number; value: number }) {
    return item.value;
  }

I'll go ahead and close this because MatTable appears to be works as expected when trackByFn returns unique values.

Best,
Zach

@zarend zarend closed this as completed Feb 28, 2022
@preda7or
Copy link
Author

preda7or commented Mar 1, 2022

I thought this trackBy works the same way as the ngFor, meaning that it is used to identity check (if DOM element needs to be moved, created, deleted). If identity matches then bindings are updated, but DOM hierarchy does not change.

@preda7or
Copy link
Author

preda7or commented Mar 1, 2022

@zarend from the updated example (https://stackblitz.com/edit/components-issue-5pt19s) it can clearly be seen, that tracking by value destroys and recreates the rows and their component tree (see the console logs).

So changing the trackBy is not the solution (Track by function is meant to provide identity anyway),
whereas running a markForCheck on the containing component results in the correct behavior: updating bindings.

Please reopen the bug.

@crisbeto crisbeto reopened this Mar 1, 2022
@preda7or
Copy link
Author

preda7or commented Mar 1, 2022

The solution is probably only one line of code, that - if the data source is an observable - runs a markForCheck on the table.
(There may be other cases when it is not strictly necessary, eg. there is no diff, maybe without onpush it also works, etc.)

@andrewseguin
Copy link
Contributor

Thanks for the reproduction showing the issue - something does seem off because I would have expected the table to update on each push on the datasource stream.

@andrewseguin andrewseguin added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent area: cdk/table and removed needs triage This issue needs to be triaged by the team labels Apr 13, 2022
@preda7or
Copy link
Author

preda7or commented Apr 14, 2022

It is very simple:

  • as the input of the mat-table is not changing,
  • and we have set OnPush strategy,

changedetection will not be triggered and
even if triggered by the container component, it will not run on the table.

@ChristofferGersen
Copy link

This also prevents the rendering of components inside a MatNoDataRow, even without any trackBy function. I have updated the example to show this: https://stackblitz.com/edit/components-issue-dpw4qc

As a workaround I currently call markForCheck inside the constructor of components that I plan to use inside a MatNoDataRow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cdk/table P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

5 participants