fix(forms): ensure OnPush ancestors are marked dirty when the promise…

… resolves (#44886) Currently, `ngModel` calls` setValue` after the `resolvedPromise` is resolved. The promise is resolved _after_ the child template executes. The change detection is run but `OnPush` views are not updated because they are not marked as dirty. PR Close #44886
angular · Jan 31, 2022 · 1aebbf8 · 1aebbf8
1 parent 250dc40
commit 1aebbf8
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 7 deletions.
diff --git a/goldens/public-api/forms/forms.md b/goldens/public-api/forms/forms.md
@@ -5,6 +5,7 @@
 ```ts
 
 import { AfterViewInit } from '@angular/core';
+import { ChangeDetectorRef } from '@angular/core';
 import { ElementRef } from '@angular/core';
 import { EventEmitter } from '@angular/core';
 import * as i0 from '@angular/core';
@@ -204,10 +205,8 @@ export class DefaultValueAccessor extends BaseControlValueAccessor implements Co
 // @public
 export class EmailValidator extends AbstractValidatorDirective {
     email: boolean | string;
-
     // (undocumented)
     enabled(input: boolean): boolean;
-
     // (undocumented)
     static ɵdir: i0.ɵɵDirectiveDeclaration<EmailValidator, "[email][formControlName],[email][formControl],[email][ngModel]", never, { "email": "email"; }, {}, never>;
     // (undocumented)
@@ -574,7 +573,7 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
 
 // @public
 export class NgModel extends NgControl implements OnChanges, OnDestroy {
-    constructor(parent: ControlContainer, validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[]);
+    constructor(parent: ControlContainer, validators: (Validator | ValidatorFn)[], asyncValidators: (AsyncValidator | AsyncValidatorFn)[], valueAccessors: ControlValueAccessor[], _changeDetectorRef?: ChangeDetectorRef | null | undefined);
     // (undocumented)
     readonly control: FormControl;
     get formDirective(): any;
@@ -599,7 +598,7 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {
     // (undocumented)
     static ɵdir: i0.ɵɵDirectiveDeclaration<NgModel, "[ngModel]:not([formControlName]):not([formControl])", ["ngModel"], { "name": "name"; "isDisabled": "disabled"; "model": "ngModel"; "options": "ngModelOptions"; }, { "update": "ngModelChange"; }, never>;
     // (undocumented)
-    static ɵfac: i0.ɵɵFactoryDeclaration<NgModel, [{ optional: true; host: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; self: true; }]>;
+    static ɵfac: i0.ɵɵFactoryDeclaration<NgModel, [{ optional: true; host: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; self: true; }, { optional: true; }]>;
 }
 
 // @public

diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
@@ -83,6 +83,9 @@
   {
     "name": "ChangeDetectionStrategy"
   },
+  {
+    "name": "ChangeDetectorRef"
+  },
   {
     "name": "CheckboxControlValueAccessor"
   },
@@ -1055,6 +1058,9 @@
   {
     "name": "injectArgs"
   },
+  {
+    "name": "injectChangeDetectorRef"
+  },
   {
     "name": "injectElementRef"
   },

diff --git a/packages/forms/src/directives/ng_model.ts b/packages/forms/src/directives/ng_model.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Directive, EventEmitter, forwardRef, Host, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges} from '@angular/core';
+import {ChangeDetectorRef, Directive, EventEmitter, forwardRef, Host, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges} from '@angular/core';
 
 import {FormControl, FormHooks} from '../model';
 import {NG_ASYNC_VALIDATORS, NG_VALIDATORS} from '../validators';
@@ -208,7 +208,8 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {
       @Optional() @Self() @Inject(NG_VALIDATORS) validators: (Validator|ValidatorFn)[],
       @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators:
           (AsyncValidator|AsyncValidatorFn)[],
-      @Optional() @Self() @Inject(NG_VALUE_ACCESSOR) valueAccessors: ControlValueAccessor[]) {
+      @Optional() @Self() @Inject(NG_VALUE_ACCESSOR) valueAccessors: ControlValueAccessor[],
+      @Optional() @Inject(ChangeDetectorRef) private _changeDetectorRef?: ChangeDetectorRef|null) {
     super();
     this._parent = parent;
     this._setValidators(validators);
@@ -313,6 +314,7 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {
   private _updateValue(value: any): void {
     resolvedPromise.then(() => {
       this.control.setValue(value, {emitViewToModelChange: false});
+      this._changeDetectorRef?.markForCheck();
     });
   }
 
@@ -327,6 +329,8 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {
       } else if (!isDisabled && this.control.disabled) {
         this.control.enable();
       }
+
+      this._changeDetectorRef?.markForCheck();
     });
   }
 }
diff --git a/packages/forms/test/value_accessor_integration_spec.ts b/packages/forms/test/value_accessor_integration_spec.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, EventEmitter, Input, Output, Type, ViewChild} from '@angular/core';
+import {ChangeDetectionStrategy, ChangeDetectorRef, Component, Directive, EventEmitter, Input, Output, Type, ViewChild} from '@angular/core';
 import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing';
 import {AbstractControl, ControlValueAccessor, FormControl, FormGroup, FormsModule, NG_VALIDATORS, NG_VALUE_ACCESSOR, NgControl, NgForm, NgModel, ReactiveFormsModule, Validators} from '@angular/forms';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
@@ -1099,6 +1099,66 @@ import {dispatchEvent} from '@angular/platform-browser/testing/src/browser_util'
              });
            }));
       });
+
+      describe('`ngModel` value accessor inside an OnPush component', () => {
+        it('should run change detection and update the value', fakeAsync(async () => {
+             @Component({
+               selector: 'parent',
+               template: '<child [ngModel]="value"></child>',
+               changeDetection: ChangeDetectionStrategy.OnPush,
+             })
+             class Parent {
+               value!: string;
+
+               constructor(private ref: ChangeDetectorRef) {}
+
+               setTimeoutAndChangeValue(): void {
+                 setTimeout(() => {
+                   this.value = 'Carson';
+                   this.ref.detectChanges();
+                 }, 50);
+               }
+             }
+
+             @Component({
+               selector: 'child',
+               template: 'Value: {{ value }}',
+               providers: [{provide: NG_VALUE_ACCESSOR, useExisting: Child, multi: true}]
+             })
+             class Child implements ControlValueAccessor {
+               value!: string;
+
+               writeValue(value: string): void {
+                 this.value = value;
+               }
+
+               registerOnChange(): void {}
+
+               registerOnTouched(): void {}
+             }
+
+             const fixture = initTest(Parent, Child);
+             fixture.componentInstance.value = 'Nancy';
+             fixture.detectChanges();
+
+             await fixture.whenStable();
+             fixture.detectChanges();
+             await fixture.whenStable();
+
+             const child = fixture.debugElement.query(By.css('child'));
+             // Let's ensure that the initial value has been set, because previously
+             // it wasn't set inside an `OnPush` component.
+             expect(child.nativeElement.innerHTML).toEqual('Value: Nancy');
+
+             fixture.componentInstance.setTimeoutAndChangeValue();
+             tick(50);
+
+             fixture.detectChanges();
+             await fixture.whenStable();
+
+             expect(child.nativeElement.innerHTML).toEqual('Value: Carson');
+           }));
+      });
     });
   });
 }