Lazy loading causes ExpressionChangedAfterItHasBeenCheckedError (NG-ZORRO?)

[lppedd]

Even with this workaround, it seems that lazy loading a component doesn't work. At least that happens when using the NG-ZORRO Tab's lazy loading mechanism.

this.subscription = this.formGroup.valueChanges
  .pipe(
    // this is required otherwise an `ExpressionChangedAfterItHasBeenCheckedError` will happen
    // this is due to the fact that parent component will define a given state for the form that might
    // be changed once the children are being initialized
    delay(0),

ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'ng-pristine: true'. Current value: 'ng-pristine: false'.

Usage example:

<div [formGroup]="formGroup">
  <input [formControlName]="formControlNames.name">

  <div [formArrayName]="formControlNames.families">
    <nz-tabset nzTabPosition="top" nzSize="small" nzType="card">
      <nz-tab *ngFor="let c of families.controls; index as i" [nzTitle]="c.value.name">
        <ng-template nz-tab>  <!--  This -->
          <app-family [formControlName]="i"></app-family>
        </ng-template>
      </nz-tab>
    </nz-tabset>
  </div>
</div>

I'm trying to understand, but with no luck 'til now.

[maxime1992]

Do you think you might manage to make a minimal repro of the issue on Stackblitz?

[maxime1992]

Also I doubt that it's related to lazy loading but ExpressionChangedAfterItHasBeenCheckedError are hard to track, hopefully might be easier with Ivy (? 😸)

[lppedd]

@maxime1992 I've created a new repository here https://github.com/lppedd/ngx-sub-form-test
Just clone and npm install. It is built on this repository, I just modified the provided example and added your FormArray fix.

Hope Ivy will make debugging Angular Core easier too! Switching from debugging Java code to Javascript code has been a major struggle for me.

[zakhenry]

@maxime1992 I wonder if this might actually be related to the thing I was talking about regarding not destroying the form item controls when the external value updates? Something to check perhaps.

[lppedd]

@maxime1992 @zakhenry Calling for detectChanges seems to fix the problem.

public writeValue(obj: any): void {
  if (obj) {
    if (!!this.formGroup) {
      this.handleFormArrayControls(obj);
      this.formGroup.setValue(this.transformToFormGroup(obj), {
        emitEvent: false,
      });
      this.formGroup.markAsPristine();
      this.ref.detectChanges();
    }
  } else {
    // @todo clear form?
  }
}

I wonder if this is an optimal solution at all.
And it seems this is valid only for development mode, because apparently Angular performs an additional change detection cycle to ensure everything runs smoothly.

From tick documentation

In development mode, tick() also performs a second change detection cycle to ensure that no further changes are detected. If additional changes are picked up during this second cycle, bindings in the app have side-effects that cannot be resolved in a single change detection pass. In this case, Angular throws an error, since an Angular application can only have one change detection pass during which all change detection must complete.

This would require a fix anyway, however.

The problem I see with having to use the ChangeDetectorRef is that it has to be injected via the extending classes. So all the components would have to explicitly require that, which is really ugly.

This

if (this.onChange) {
  this.onChange(null);
}

is also problematic with ng-template and arrays, as when the component gets destroyed the FormArray's FormControl's value is set to null. Not good.
I'd say it's not really the responsibility of the library to set the value to null.

[maxime1992]

Hey @lppedd 👋

I took some time today to dig into that issue, thanks for the repro because it helped a lot.

Every time there's an error ExpressionChangedAfterItHasBeenCheckedError it's really hard to debug, trying to figure it out because it probably shows a flaw in the architecture. If you want to read a deep dive why that mechanism exists you can read that article.

What I found is related to that part of the lib:

delay(0),
tap(changes => {
  // ...
  this.onChange(this.transformFromFormGroup(changes));
  // ...
}

Notice here that we've got the delay(0) (CF comment above why).

BUT, on that line

// this is required to correctly initialize the form value
this.onChange(this.transformFromFormGroup(this.fg.value));

we do not have an equivalent of delay(0). So when the component is being loaded, we do warn the parent that the value has changed. All of that happening in the same tick, and thus after the second pass of CD, one of the values into the form might have changed (pristine in that case).

I believe the fix is simply to wrap that line into a setTimeout (tried and it work on your demo).
I'll raise a MR to discuss about that and if that's the best option (because I'd be glad not to have any setTimeout nor delay.

---

Side notes:

Few things I've noticed from your repro and that might be improved:

1 - app.component.html:
Instead of passing page like that:

<app-page [page]="page"></app-page>

embrasse the lib and make a form on the top level component:

const defaultPages: Page = {
  families: [
    {
      name: 'Family 1',
      values: [
        {
          id: '1',
          value: 'Value 1',
        },
        {
          id: '2',
          value: 'Value 2',
        },
      ],
    },
  ],
};

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss'],
})
export class AppComponent extends NgxSubFormComponent<{ page: Page }> {
  formControls = {
    page: new FormControl(defaultPages),
  };
}

and then HTML becomes:

<form [formGroup]="formGroup">
  <app-page [formControlName]="formControlNames.page"></app-page>
</form>

2 - page.component.ts:

Update from:

@Component({
  selector: 'app-page',
  templateUrl: './page.component.html',
  styleUrls: ['./page.component.scss']
})
export class PageComponent extends NgxSubFormComponent<{ wrapper: Page }> {
  @Input()
  public set page(page: Page) {
    this.formGroup.setValue({
      wrapper: page
    })
  }

  protected formControls: Controls<{ wrapper: Page }> = {
    wrapper: new FormControl()
  };
}

To:

@Component({
  selector: 'app-page',
  templateUrl: './page.component.html',
  styleUrls: ['./page.component.scss'],
  providers: subformComponentProviders(PageComponent),
})
export class PageComponent extends NgxSubFormRemapComponent<Page, { wrapper: Page }> {
  protected formControls: Controls<{ wrapper: Page }> = {
    wrapper: new FormControl(),
  };

  protected transformToFormGroup(obj: Page): { wrapper: Page } {
    return { wrapper: obj };
  }

  protected transformFromFormGroup(formValue: { wrapper: Page }): Page {
    return formValue.wrapper;
  }
}

That kind of use case is exactly what NgxSubFormRemapComponent is for 😺
You can have a different form object than your original structure.

[lppedd]

@maxime1992 thanks for the advice!

Regarding the error, wouldn't be better using, somehow, the ChangeDetectorRef?
It would be the most appropriate solution imho.

[maxime1992]

Regarding the error, wouldn't be better using, somehow, the ChangeDetectorRef?

I think it's one of those (rare) cases where setTimeout actually makes sense.
We do not want consumers to be aware of that issue nor to have to inject ChangeDetectorRef and pass it to super. That'd be heavy.

Putting a setTimeout will update the Zone, and when the action actually happens, Angular will run another change detection cycle.

So I think we end up with the same result, but a simpler API when using setTimeout (even if, I agree, it does not feel great).

Also, if I was going to use ChangeDetectorRef I'd do it by calling markForCheck, not detecChanges.
See this SO post. I'm not even sure why the following worked for you

Calling for detectChanges seems to fix the problem

I guess it'd in the case where the top component is also extending NgxSubFormComponent and thus it'd be high enough to have all the sub props checked. But as I said, I'd use markForCheck which would mark the property as needing a check on the next CD (from the top of the tree). BUT, it'd happen only on the next CD, so it might stay not up to date for a while?

That's why I think in that case setTimeout is making more sense.

It would be the most appropriate solution imho

Can you explain why?

[lppedd]

@maxime1992 just because setTimeout feels a bit out-of-place.
But I understand that it's actually simpler thant injecting ChangeDetectorRef.
And ultimately, I agree with you.

I'm also not an Angular expert so my opinions might be wrong.

---

For the last part of my message, about

if (this.onChange) {
  this.onChange(null);
}

What do you think? Should I create another issue?

[maxime1992]

Oups. Missed that part 🤐

Should I create another issue?

Yes please, let's keep this thread focused on the error :)

[maxime1992]

🎉 This issue has been resolved in version 2.1.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀