regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component" #40521

satanTime · 2021-01-22T16:59:55Z

🐞 bug report

Affected Package

@angular/core/testing v11.1.0

Is this a regression?

Yes, the previous version in which this bug was not present was: v11.0.9

Description

tests for reactive forms without value accessor fail in clean up.

🔬 Minimal Reproduction

https://codesandbox.io/s/twilight-thunder-tbl3d?file=/src/test.spec.ts (open console)
change angular to 11.0.9 to see that error isn't present: https://take.ms/wuPcy

🔥 Exception or Error


Error during cleanup of component

which is caused by


Cannot read property 'registerOnChange' of null at cleanUpControl

🌍 Your Environment

Angular Version:



     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/


Angular CLI: 11.1.0
Node: 12.20.1
OS: darwin x64

Angular: 11.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: Yes

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1101.0
@angular-devkit/build-angular   0.1101.0
@angular-devkit/core            11.1.0
@angular-devkit/schematics      11.1.0
@schematics/angular             11.1.0
@schematics/update              0.1101.0
rxjs                            6.6.3
typescript                      4.0.5

The text was updated successfully, but these errors were encountered:

angular/angular#40521

JoostK · 2021-01-22T17:25:14Z

Here's the full stacktrace:

TypeError: Cannot read property 'registerOnChange' of null
    at cleanUpControl (https://tbl3d.csb.app/node_modules/@angular/forms/fesm2015/forms.js:1664:21)
    at FormGroupDirective.removeControl (https://tbl3d.csb.app/node_modules/@angular/forms/fesm2015/forms.js:3189:5)
    at FormControlName.ngOnDestroy (https://tbl3d.csb.app/node_modules/@angular/forms/fesm2015/forms.js:3494:26)
    at callProviderLifecycles (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:16098:14)
    at callElementProvidersLifecycles (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:16069:7)
    at callLifecycleHooksChildrenFirst (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:16059:21)
    at destroyView (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:20501:3)
    at callViewAction (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:20609:7)
    at execComponentViewsAction (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:20546:7)
    at destroyView (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:20500:3)
    at callWithDebugContext (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:21134:23)
    at Object.debugDestroyView [as destroyView] (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:20899:10)
    at ViewRef_.destroy (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:15584:14)
    at ComponentRef_.destroy (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/core.js:15433:19)
    at ComponentFixture.destroy (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/testing.js:308:25)
    at eval (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/testing.js:1602:17)
    at Array.forEach (<anonymous>)
    at TestBedViewEngine.resetTestingModule (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/testing.js:1600:26)
    at Function.resetTestingModule (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/testing.js:1509:29)
    at UserContext.eval (https://tbl3d.csb.app/node_modules/@angular/core/fesm2015/testing.js:1913:13)
    at ZoneDelegate.invoke (https://tbl3d.csb.app/node_modules/zone.js/fesm2015/zone.js:368:26)
    at ProxyZoneSpec.onInvoke (eval at z (https://codesandbox.io/static/js/sandbox.d6312f59f.js:1:98947), <anonymous>:300:43)
    at ZoneDelegate.invoke (https://tbl3d.csb.app/node_modules/zone.js/fesm2015/zone.js:367:52)
    at Zone.run (https://tbl3d.csb.app/node_modules/zone.js/fesm2015/zone.js:130:43)
    at runInTestZone (eval at z (https://codesandbox.io/static/js/sandbox.d6312f59f.js:1:98947), <anonymous>:580:38)
    at UserContext.eval (eval at z (https://codesandbox.io/static/js/sandbox.d6312f59f.js:1:98947), <anonymous>:595:24)
    at ZoneQueueRunner.attempt (https://tbl3d.csb.app/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:7063:44)
    at ZoneQueueRunner.QueueRunner.run (https://tbl3d.csb.app/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:7104:25)
    at runNext (https://tbl3d.csb.app/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:7023:18)
    at next (https://tbl3d.csb.app/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:7030:11)
    at eval (https://tbl3d.csb.app/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:6924:9)
    at ZoneDelegate.invokeTask (https://tbl3d.csb.app/node_modules/zone.js/fesm2015/zone.js:402:31)
    at Zone.runTask (https://tbl3d.csb.app/node_modules/zone.js/fesm2015/zone.js:174:47)
    at drainMicroTaskQueue (https://tbl3d.csb.app/node_modules/zone.js/fesm2015/zone.js:578:35)

Notice how this uses ViewEngine, not Ivy, but that may not be relevant.

PR angular#39235 introduced additional cleanup logic for form controls and directives. The cleanup logic relies on the presence of ControlValueAccessor instances on FormControlName and FormControl directives. In general these fields are present and there are also checks to make sure that the mentioned directive instances are created with CVAs. However some scenarios (primarily tests) may invoke the logic in a way that the directive instance would not be fully initialized, thus causing CVA to be absent. As a result, the cleanup logic fails while trying to call some methods on associated CVA instances. This commit updates the cleanup logic to take into account the situation when CVA is not present. Fixes angular#40521.

AndrewKushnir · 2021-01-22T19:22:27Z

Hi @satanTime, thanks for reporting the issue.

PR #39235 (changes were released in v11.1.0) introduced additional cleanup logic for form controls and directives. This cleanup logic relies on the fact that FormControlName and FormControl directives are fully initialized, thus having the valueAccessor field defined as an instance of corresponding CVA (Control Value Accessor). There is a logic in selectValueAccessor function that makes sure that this is the case and throws an error otherwise. However in your test there is a try-catch (in a form of expect(...).toThrow(...)) which ignores that error, thus directive instance ended up being not fully initialized.

This is unlikely to happen in real scenarios (i.e. in an app on a page), but some specific use-cases in tests might cause the error to happen. So I've created a PR #40526 to make the cleanup logic more resilient to this type of scenarios.

Thank you.

satanTime · 2021-01-22T19:38:17Z

Hi.

Totally agree. We have a lib for testing which contains a couple of tests to ensure that it fails in the same way as angular does, but not sure that someone would do something similar in a real project.

Thank you and happy coding!

akram1905 · 2021-01-25T16:33:05Z

Hi,
It actually broke our app on production!
[Partially it was our fault we didn't lock Angular's version on package.json]

We use a directive to add dynamic Form controls, then in ngOnDestroy we call removeControl which caused the error.

satanTime · 2021-01-25T16:36:18Z

Ah, here we go, my condolences.

akram1905 · 2021-01-25T16:54:38Z

Thank you for your kindness and sympathy.
I’m deeply moved by your comforting words.
I greatly appreciate your precious gifts that you are thinking to send me since as you know words won't be enough to condolence me.

AndrewKushnir · 2021-01-25T17:38:09Z

Hi @akram1905, thanks for providing additional information.

The mentioned PR #40526 is in the merge queue and the fix will be available in v11.1.1 later this week.

We use a directive to add dynamic Form controls, then in ngOnDestroy we call removeControl which caused the error.

The code that was there before in cleanUpControl function always expected a CVA instance to be present on a directive and there is also a code in selectValueAccessor that throws if it's not the case. I'm wondering if you could share a minimal repro of your use-case (via Stackblitz)? I'd like to make sure we have test coverage for it and/or add more tests if needed to prevent regressions in the future. Note: this issue will be closed automatically once #40526 is merged, but you could still leave a comment with additional details in this thread.

Thank you.

…#40526) PR #39235 introduced additional cleanup logic for form controls and directives. The cleanup logic relies on the presence of ControlValueAccessor instances on FormControlName and FormControl directives. In general these fields are present and there are also checks to make sure that the mentioned directive instances are created with CVAs. However some scenarios (primarily tests) may invoke the logic in a way that the directive instance would not be fully initialized, thus causing CVA to be absent. As a result, the cleanup logic fails while trying to call some methods on associated CVA instances. This commit updates the cleanup logic to take into account the situation when CVA is not present. Fixes #40521. PR Close #40526

angular-automatic-lock-bot · 2021-02-25T16:45:57Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

satanTime changed the title ~~regression "Cannot read property 'registerOnChange' of null at cleanUpControl"~~ regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component" Jan 22, 2021

satanTime changed the title ~~regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component"~~ regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / Error during cleanup of component Jan 22, 2021

satanTime changed the title ~~regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / Error during cleanup of component~~ regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component" Jan 22, 2021

satanTime added a commit to satanTime/ng-mocks that referenced this issue Jan 22, 2021

test: pending tests because of 11.1.0 regression

a9466eb

angular/angular#40521

JoostK added the area: forms label Jan 22, 2021

ngbot bot added this to the needsTriage milestone Jan 22, 2021

petebacondarwin assigned AndrewKushnir Jan 22, 2021

AndrewKushnir added P2 The issue is important to a large percentage of users, with a workaround state: confirmed type: bug/fix labels Jan 22, 2021

ngbot bot modified the milestones: needsTriage, Backlog Jan 22, 2021

AndrewKushnir mentioned this issue Jan 22, 2021

fix(forms): properly cleanup in cases when FormControlName has no CVA #40526

Closed

3 tasks

AndrewKushnir added P4 A relatively minor issue that is not relevant to core functions state: has PR and removed P2 The issue is important to a large percentage of users, with a workaround labels Jan 22, 2021

jessicajaniuk closed this as completed in 4d66185 Jan 25, 2021

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component" #40521

regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component" #40521

satanTime commented Jan 22, 2021 •

edited

JoostK commented Jan 22, 2021

AndrewKushnir commented Jan 22, 2021

satanTime commented Jan 22, 2021

akram1905 commented Jan 25, 2021

satanTime commented Jan 25, 2021

akram1905 commented Jan 25, 2021

AndrewKushnir commented Jan 25, 2021

angular-automatic-lock-bot bot commented Feb 25, 2021

regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component" #40521

regression "Cannot read property 'registerOnChange' of null at cleanUpControl" / "Error during cleanup of component" #40521

Comments

satanTime commented Jan 22, 2021 • edited

🐞 bug report

Affected Package

Is this a regression?

Description

🔬 Minimal Reproduction

🔥 Exception or Error

🌍 Your Environment

JoostK commented Jan 22, 2021

AndrewKushnir commented Jan 22, 2021

satanTime commented Jan 22, 2021

akram1905 commented Jan 25, 2021

satanTime commented Jan 25, 2021

akram1905 commented Jan 25, 2021

AndrewKushnir commented Jan 25, 2021

angular-automatic-lock-bot bot commented Feb 25, 2021

satanTime commented Jan 22, 2021 •

edited