fix(forms): clear parent when removing controls (angular#29517)

When a control is removed from a FormGroup, or replaced by another control, that control's parent
wasn't cleared. Apart from leaving the parent reference on the removed control, this also made it
impossible for the control to react on its removal in an overridden setParent.

This change introduces an unregisterControl method, symmetric to registerControl, which is called
to clear a control's parent (set to null) when it's removed or replaced.

PR Close angular#29517

packages/forms/src/model.ts

@@ -151,8 +151,8 @@ export abstract class AbstractControl {
   // TODO(issue/24571): remove '!'.
   _updateOn !: FormHooks;
 
-  // TODO(issue/24571): remove '!'.
-  private _parent !: FormGroup | FormArray;
+  private _parent: FormGroup|FormArray|null = null;
+
   private _asyncValidationSubscription: any;
 
   /**
@@ -178,7 +178,7 @@ export abstract class AbstractControl {
   /**
    * The parent control.
    */
-  get parent(): FormGroup|FormArray { return this._parent; }
+  get parent(): FormGroup|FormArray|null { return this._parent; }
 
   /**
    * The validation status of the control. There are four possible
@@ -560,7 +560,7 @@ export abstract class AbstractControl {
   /**
    * @param parent Sets the parent of the control
    */
-  setParent(parent: FormGroup|FormArray): void { this._parent = parent; }
+  setParent(parent: FormGroup|FormArray|null): void { this._parent = parent; }
 
   /**
    * Sets the value of the control. Abstract method (implemented in sub-classes).
@@ -870,8 +870,8 @@ export abstract class AbstractControl {
    * @internal
    */
   private _parentMarkedDirty(onlySelf?: boolean): boolean {
-    const parentDirty = this._parent && this._parent.dirty;
-    return !onlySelf && parentDirty && !this._parent._anyControlsDirty();
+    if (!this._parent) return false;
+    return !onlySelf && this._parent.dirty && !this._parent._anyControlsDirty();
   }
 }
 
@@ -1274,6 +1274,24 @@ export class FormGroup extends AbstractControl {
     return control;
   }
 
+  /**
+   * Unregisters a control from the group's list of controls.
+   *
+   * This method does not update the value or validity of the control.
+   * Use {@link FormGroup#removeControl removeControl} instead.
+   *
+   * @param name The control name to unregister from the collection
+   */
+  unregisterControl(name: string): AbstractControl {
+    const control = this.controls[name];
+    if (control) {
+      control._registerOnCollectionChange(() => {});
+      control.setParent(null);
+    }
+    delete this.controls[name];
+    return control;
+  }
+
   /**
    * Add a control to this group.
    *
@@ -1294,8 +1312,7 @@ export class FormGroup extends AbstractControl {
    * @param name The control name to remove from the collection
    */
   removeControl(name: string): void {
-    if (this.controls[name]) this.controls[name]._registerOnCollectionChange(() => {});
-    delete (this.controls[name]);
+    this.unregisterControl(name);
     this.updateValueAndValidity();
     this._onCollectionChange();
   }
@@ -1307,8 +1324,7 @@ export class FormGroup extends AbstractControl {
    * @param control Provides the control for the given name
    */
   setControl(name: string, control: AbstractControl): void {
-    if (this.controls[name]) this.controls[name]._registerOnCollectionChange(() => {});
-    delete (this.controls[name]);
+    this.unregisterControl(name);
     if (control) this.registerControl(name, control);
     this.updateValueAndValidity();
     this._onCollectionChange();

packages/forms/test/form_group_spec.ts

@@ -159,6 +159,15 @@ import {of } from 'rxjs';
         expect(g.valid).toBe(false);
       });
 
+      it('should update parent of added control', () => {
+        const g = new FormGroup({});
+        const c = new FormControl();
+
+        g.addControl('c', c);
+
+        expect(c.parent).toBe(g);
+      });
+
       it('should update value and validity when control is removed', () => {
         const g = new FormGroup(
             {'one': new FormControl('1'), 'two': new FormControl('2', Validators.minLength(10))});
@@ -170,6 +179,15 @@ import {of } from 'rxjs';
         expect(g.value).toEqual({'one': '1'});
         expect(g.valid).toBe(true);
       });
+
+      it('should update parent of removed control', () => {
+        const c = new FormControl();
+        const g = new FormGroup({c: c});
+
+        g.removeControl('c');
+
+        expect(c.parent).toBe(null);
+      });
     });
 
     describe('dirty', () => {