primefaces / primeng

The Most Complete Angular UI Component Library
https://primeng.org
Other
10.45k stars 4.6k forks source link

p-dialog: ExpressionChangedAfterItHasBeenCheckedError with attr.aria-labelledby #13636

Closed psarno closed 1 year ago

psarno commented 1 year ago

Describe the bug

I know some work is being done here on accessability, and I saw a related issue but not with the p-dialog component.

We are on the latest PrimeNG version (16.3.1).

The exception is:

ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value for 'attr.aria-labelledby': 'pn_id_70_header'. Current value: 'pn_id_73_header'. Expression location: _Dialog component

Call Stack:

at throwErrorIfNoChangesMode (core.mjs:11010:11)
at bindingUpdated (core.mjs:14251:17)
at ɵɵattribute (core.mjs:14295:9)
at Dialog_div_0_div_1_Template (primeng-dialog.mjs:357:8)
at ReactiveLViewConsumer.runInContext (core.mjs:11103:13)

Line in primeng-dialog.mjs triggering this:

image

Dialog defined as:

<p-dialog
  [(visible)]="showUpdateMultiPointerDialog"
  [modal]="true"
>

Also - Can we get an updated reproducer template that isn't on Angular 14 and PrimeNG 14?

I am unable to update the template in stackblitz to Angular 16/PrimeNG 16, no matter what I try I end up with:

Error in /turbo_modules/primeng@16.3.1/fesm2022/primeng-tieredmenu.mjs (137:252) Cannot access 'TieredMenu' before initialization

Environment

Angular 16.2.2 PrimeNG 16.3.1

Reproducer

No response

Angular version

16.2.2

PrimeNG version

16.3.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.16.0

Browser(s)

No response

Steps to reproduce the behavior

  1. Create a p-dialog component
  2. Set it's [(visible)] property to a boolean flag
  3. Set the boolean to true to show the dialog

Expected behavior

There is no ExpressionChangedAfterItHasBeenCheckedError

SoyDiego commented 1 year ago

I think I did a PR about that. I will check again.

Edit ⚠️ I found it. https://github.com/primefaces/primeng/pull/13538. But now I have a doubt, your problem is in Panel o Dialog? Because you said Panel in your message but Dialog in the title and share a Stackblitz please

Thanks

psarno commented 1 year ago

@SoyDiego I appreciate the help.

It's in the dialog component, as per the stack trace:

at Dialog_div_0_div_1_Template (primeng-dialog.mjs:357:8)

And error:

Expression` location: _Dialog component

I see the PR was merged. Did that go out with 16.3.1?

If so then an issue persists here.

I am unable to do a Stackblitz, see the bottom of my post, there is a link there ... can't get it to work with 16.

SoyDiego commented 1 year ago

Yes, was merged. It's very weird. Tomorrow I will check again and I will update here.

SoyDiego commented 1 year ago

Now I saw the problem. My PR fixed in dynamic dialog and this is dialog. They are different. I will try to solve again tomorrow.

Thanks for report the issue

SoyDiego commented 1 year ago

Hi @psarno I tried to replicate your problem but I cannot see errors and anything.

My code

image

image

Testing

testing dialog.

For that maybe is good idea if you can replicate with Stackblitz.

Thanks again

psarno commented 1 year ago

@SoyDiego It seems to be the exact same issue as was occuring in #13497 just in a different component.

It may be difficult to reproduce, I am not sure what the underlying cause is but it's probably no coincidence that we have the same error in two similar components on the same exact statement (with the attr.aria-labelledby).

Sometimes these things are not always immediately reproducible and we have to go off crash reports and put in defensive coding or fixes simply based off reports that include the call stack, such as this one does.

Since this looks to be literally the same problem, with the same code, just in a different component, why not apply the same fix here to be safe? It was needed in one, why would it not be needed in both?

image

And as I asked in the original post, is there any reason we don't have an updated Stackblitz repro template that uses the latest PrimeNG 16? I can not get 16 to work on StackBlitz, I am unsure why it throws this error.

SoyDiego commented 1 year ago

@SoyDiego It seems to be the exact same issue as was occuring in #13497 just in a different component.

It may be difficult to reproduce, I am not sure what the underlying cause is but it's probably no coincidence that we have the same error in two similar components on the same exact statement (with the attr.aria-labelledby).

Sometimes these things are not always immediately reproducible and we have to go off crash reports and put in defensive coding or fixes simply based off reports that include the call stack, such as this one does.

Since this looks to be literally the same problem, with the same code, just in a different component, why not apply the same fix here to be safe? It was needed in one, why would it not be needed in both?

image

And as I asked in the original post, is there any reason we don't have an updated Stackblitz repro template that uses the latest PrimeNG 16? I can not get 16 to work on StackBlitz, I am unsure why it throws this error.

@psarno I will create a new PR adding the same implementations because you are right, It's the same problem but I don't know why can't replicated. And I'm not decide this things and I don't know why Stackblitz is not working. I'm not part of PrimeNG Team, only a contributor here trying to help, ok? Also I don't know If PrimeNG Team will approve it, because again, I tested and I cannot replicated the problem, and you are the only user with that problem. In that issue (#13497), a lot of problem were reporting.

psarno commented 1 year ago

The error is occuring in the <p-dialog> component as shown by the call stack, but our actual repro scenario is a component setup such as the following:

<p-dialog>
   <ng-template pTemplate="body">
      <p-autoComplete></p-autoComplete>
    </ng-template>
</p-dialog>

That is overly simplified but it shows the control that causes this error to occur.

When we type a character into the <p-autoComplete>, it immediatly throws the ExpressionChangedAfterItHasBeenCheckedError. Note however, that this is not occuring in the <p-autoComplete>, even though that triggers it to occur ... it is occuring in the <p-dialog> as per the call stack.

psarno commented 1 year ago

@SoyDiego Understood that you're just a contributor, you are doing a great work with the PRs.

I am also just leaving these notes here so the PrimeNG team can see the logic behind what is going on, if this is ever merged.

@cagataycivici Is there any chance we can get an officially updated Stackblitz template for PrimeNG from your team to version 16? I don't know who to contact about that. The current one is on 14. I have tried to get it working with 16 but run into one particular error I haven't figured out.

SoyDiego commented 1 year ago

The error is occuring in the <p-dialog> component as shown by the call stack, but our actual repro scenario is a component setup such as the following:

<p-dialog>
   <ng-template pTemplate="body">
      <p-autoComplete></p-autoComplete>
    </ng-template>
</p-dialog>

That is overly simplified but it shows the control that causes this error to occur.

When we type a character into the <p-autoComplete>, it immediatly throws the ExpressionChangedAfterItHasBeenCheckedError. Note however, that this is not occuring in the <p-autoComplete>, even though that triggers it to occur ... it is occuring in the <p-dialog> as per the call stack.

I tried with this example too and I couldn't see the problem. Anyway, I have created a PR here: https://github.com/primefaces/primeng/pull/13644 applying the same solution as dynamycdialog component (https://github.com/primefaces/primeng/pull/13538). Will see If PrimeNG Team approve it or not.

jncalderon commented 1 year ago

@psarno @cetincakiroglu hi guys, I see that this fix become on 16.4.2 release, how I can get before?

cetincakiroglu commented 1 year ago

Hi @psarno,

You can't get before, we'll release it today!