ReactiveForms: FormGroup.removeControl doesn't call control.setParent(null)

[mbinic]

🐞 bug report

Affected Package

@angular/forms

Is this a regression?

Most probably no.

Description

The FormGroup class' removeControl method does not call setParent(null) for the target control. This appears to be an inconsistency in the implementation.

Our expectation is that removeControl would call control.setParent(null), to clear the control's parent once detached. This would also make it symmetric with addControl.

Use case

We have a dynamic form implementation where a class extending FormGroup is dynamically attaching/detaching child groups/controls using addControl/removeControl. The controls can react to their inclusion/exclusion in their setParent method (to e.g. subscribe to a parent's attachedState observable).

Since we're extending FormGroup this is easy to work around by just overriding removeControl, but IMHO the inconsistency in the framework should be fixed.

🔬 Minimal Reproduction

The call is missing from: https://github.com/angular/angular/blob/master/packages/forms/src/model.ts#L1298 .

🔥 Exception or Error

None.

🌍 Your Environment

Angular Version: 7.2.0

Angular CLI: 7.3.0
Node: 10.15.3
OS: win32 x64

Anything else relevant?

No.

[mbinic]

Trying to fix this, I just noticed that setControl, which replaces a control, doesn't call setParent(null) on the old control either: https://github.com/angular/angular/blob/master/packages/forms/src/model.ts#L1310.

Also found a reference to issue #24571 on the _parent field declaration, which I think can be initialized to null, since it would initially be undefined even though that's not listed as one of its possible types; and also cause setParent(undefined) somehow feels wrong...
I come from the C# world though, so please feel free to correct me if something like that feels more at home here. In the meantime I'll make the change locally, add tests and see if anything blows up :)

[Airblader]

@mbinic Sounds all fine to me.

[mbinic]

Sorry @Airblader, I don't understand - are you saying that the original behavior is fine (i.e. no change is needed), or that assigning undefined to parent is fine, or that the changes my PR introduces are fine? Not all three can be fine at the same time :)

[Airblader]

That your PR changes sound fine :-)