Unclear error message for badly configured custom form controls

[mwaibel-go]

Which @angular/* package(s) are relevant/releated to the feature request?

forms

Description

I was hitting the following error when implementing a custom form control:

ERROR Error: Value accessor was not provided as an array for form control with unspecified name attribute
    at _throwError (forms.mjs:1752)
    at selectValueAccessor (forms.mjs:1782)
    at new FormControlName (forms.mjs:5743)
    at NodeInjectorFactory.FormControlName_Factory [as factory] (forms.mjs:5827)
    at getNodeInjectable (core.mjs:3556)
    at searchTokensOnInjector (core.mjs:3493)
    at getOrCreateInjectable (core.mjs:3437)
    at ɵɵdirectiveInject (core.mjs:14720)
    at ɵɵinject (core.mjs:4778)
    at NodeInjectorFactory.factory (core.mjs:11563)

It was very unclear to me what this error message meant or how to fix it. My form controls all had names and formControlNames.

Only by uncommenting parts of my form could I even find the offending form control, since the error doesn’t specify what it was trying to do when the error occured.

Googling the error only brought up the source code for the test regarding this error, but there’s no explanation why an error is thrown there, either.

The actual error was that my form control was badly configured, i.e. it provided the NG_VALUE_ACCESSOR like this:

  providers: [
    { provide: NG_VALUE_ACCESSOR, useExisting: MyControl},
  ]

instead of this

  providers: [
    { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => MyControl), multi: true },
  ]

Maybe related? Maybe related: #3011

Here’s a reproduction: https://stackblitz.com/edit/angular-ivy-e2wj4z?file=src/app/app.component.ts

Proposed solution

Since I still don’t know the actual cause of the error, I can’t propose a solution to a clearer error message. It would be nice to know the class or form control something was trying to process when the error happens.

Andrew Kushnir proposed a solution:

I think we can improve the error message by including a note like this:

Value accessor was not provided as an array for form control with unspecified name attribute.
Please make sure that the `NG_VALUE_ACCESSOR` token is configured as a multi provider (with the `multi: true` property).

I think there might also be an opportunity to improve the logic around getting the attribute name (so that there is a real name vs "unspecified attribute").

Alternatives considered

—

[AndrewKushnir]

Hi @mwaibel-go, the mentioned error was caused by the missing multi: true in your code:

{ provide: NG_VALUE_ACCESSOR, useExisting: MyControl}

which makes it a regular provider (vs a multi provider that the code expects). Here is the code location:

angular/packages/forms/src/directives/shared.ts

Lines 317 to 318 in 8ebc946

	if (!Array.isArray(valueAccessors) && (typeof ngDevMode === 'undefined' || ngDevMode))
	_throwError(dir, 'Value accessor was not provided as an array for form control with');

I think we can improve the error message by including a note like this:

Value accessor was not provided as an array for form control with unspecified name attribute.
Please make sure that the `NG_VALUE_ACCESSOR` token is configured as a multi provider (with the `multi: true` property).

I think there might also be an opportunity to improve the logic around getting the attribute name (so that there is a real name vs "unspecified attribute"). Could you please create a simple example of your structure using Stackblitz, so that we can look at the logic that calculates the name and see if there is a way to improve it?

[mwaibel-go]

Thank you for responding. Yes, I can certainly provide a repro. It might take until the weekend, though.

Sorry for replying so late. I did not get a notification for your response.

[bleistift-zwei]

Here is a reproduction from my alt account: https://stackblitz.com/edit/angular-ivy-e2wj4z?file=src/app/app.component.ts

[ameryousuf]

@AndrewKushnir Can I work on this issue?

[AndrewKushnir]

@ameryousuf sure, please let me know if you have any questions. Thank you.

[ameryousuf]

@AndrewKushnir I changed the error message as you suggested, and opened the PR.

However, regarding improving the logic around getting the attribute name (so that there is a real name vs "unspecified attribute"), do you have any idea how to improve it?

Since the selectValueAccessor function is being called from the constructors of NgModel, FormControlDirective, and FormControlName directives, where the name property doesn't have a value yet.

Besides, we can't move the call of the selectValueAccessor function to the lifecycle hooks.

[AndrewKushnir]

@ameryousuf thanks for creating the PR and sorry for the delay in answering.

If I understand correctly, the problem happens with the NgModel only (for the FormControlDirective and FormControlName) we should have the necessary info in the error message). If so, we can add a special check in the _throwError function like this:

--- a/packages/forms/src/directives/shared.ts
+++ b/packages/forms/src/directives/shared.ts
@@ -15,6 +15,7 @@ import {ControlContainer} from './control_container';
 import {BuiltInControlValueAccessor, ControlValueAccessor} from './control_value_accessor';
 import {DefaultValueAccessor} from './default_value_accessor';
 import {NgControl} from './ng_control';
+import {NgModel} from './ng_model';
 import {FormArrayName} from './reactive_directives/form_group_name';
 import {ngModelWarning} from './reactive_errors';
 import {AsyncValidatorFn, Validator, ValidatorFn} from './validators';
@@ -278,6 +279,8 @@ function _throwError(dir: AbstractControlDirective, message: string): void {
     messageEnd = `path: '${dir.path!.join(' -> ')}'`;
   } else if (dir.path![0]) {
     messageEnd = `name: '${dir.path}'`;
+  } else if ((dir instanceof NgModel) && !!dir.name) {
+    messageEnd = `name: '${dir.name}'`;
   } else {
     messageEnd = 'unspecified name attribute';
   }

My proposal is to proceed with PR #45192 without this change and create a followup PR (which would also require some tests). Please let me know if any additional information is needed.

Thank you.

[ameryousuf]

@AndrewKushnir Thanks for your reply.

Actually, it's related to the FormControlName and NgModel directives which contain the name as @Input.

Why? Because the selectValueAccessor function is being called from the constructor of these directives, where the @Input('name') is not being initialized yet, and will be always undefined in this stage.

So, your suggestion won't work in this case.

We can resolve it by calling the selectValueAccessor function from ngOnChanges once, instead of constructor:

ngOnChanges(changes: SimpleChanges): void {
  if (!this.valueAccessor) {
    // Initializing the `valueAccessor` here, gives as the opportunity to access the @Input('name') value from the `selectValueAccessor` function.
    this.valueAccessor = selectValueAccessor(this, this.valueAccessors);
  }
  // ....
}

I tested the above change and it didn't break any unit tests.

Regarding the FormControlDirective, it doesn't contain an @Input('name'), so, we can't resolve its name within the selectValueAccessor function.

What do you suggest, please?
Thank you.

[AndrewKushnir]

@ameryousuf thanks for the comment. You are right, the inputs wouldn't be available until the first ngOnChanges call. I think it might be a bit too risky to move the valueAccessor calculation to the ngOnChanges hook as there might be some code that would rely on it being available sooner than that... I think we should put this change on hold for now and we can come back to that once we do bigger refactoring in the future.

[ameryousuf]

Okay, thanks @AndrewKushnir.

Please let me know if you need any help in the future. Thanks!

[akushnarov]

Dear @dylhunn and @AndrewKushnir, I was looking at the "good first issue" list and I found this issue. I'd like to contribute, but first I wanted to check with you. I see a PR 45192 which was closed for inactivity, should I take it as a base? What would you suggest to the beginner? :)

[ashide2729]

looks like the pr is already closed after committing the changes, maybe the issue is still open because of the second problem that is yet to be addressed which is this
and if the issue is already fixed maybe we can close the issue as it confusing for other people who are trying to contirbute

[AndrewKushnir]

Sorry about the confusion. I think the error message was improved in #45192, so we can close this ticket. Thank you.

[ashide2729]

thank you for the quick response

[angular-automatic-lock-bot]

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.}